[PATCH 0/4] arch subsystem refcounter conversions

[PATCH 0/4] arch subsystem refcounter conversions

[Elena Reshetova]

Now when new refcount_t type and API are finally merged
(see include/linux/refcount.h), the following
patches convert various refcounters in the arch susystem from atomic_t
to refcount_t. By doing this we prevent intentional or accidental
underflows or overflows that can led to use-after-free vulnerabilities.

The below patches are fully independent and can be cherry-picked separately.
Since we convert all kernel subsystems in the same fashion, resulting
in about 300 patches, we have to group them for sending at least in some
fashion to be manageable. Please excuse the long cc list.

Elena Reshetova (4):
  s390: convert debug_info.ref_count from atomic_t to refcount_t
  x86: convert threshold_bank.cpus from atomic_t to refcount_t
  sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t
  kvm: convert kvm.users_count from atomic_t to refcount_t

 arch/s390/include/asm/debug.h        |  3 ++-
 arch/s390/kernel/debug.c             |  8 ++++----
 arch/sparc/kernel/mdesc.c            | 17 +++++++++--------
 arch/x86/include/asm/amd_nb.h        |  3 ++-
 arch/x86/kernel/cpu/mcheck/mce_amd.c |  6 +++---
 include/linux/kvm_host.h             |  3 ++-
 virt/kvm/kvm_main.c                  |  8 ++++----
 7 files changed, 26 insertions(+), 22 deletions(-)

-- 
2.7.4

[PATCH 0/4] arch subsystem refcounter conversions

[Elena Reshetova]

Now when new refcount_t type and API are finally merged
(see include/linux/refcount.h), the following
patches convert various refcounters in the arch susystem from atomic_t
to refcount_t. By doing this we prevent intentional or accidental
underflows or overflows that can led to use-after-free vulnerabilities.

The below patches are fully independent and can be cherry-picked separately.
Since we convert all kernel subsystems in the same fashion, resulting
in about 300 patches, we have to group them for sending at least in some
fashion to be manageable. Please excuse the long cc list.

Elena Reshetova (4):
  s390: convert debug_info.ref_count from atomic_t to refcount_t
  x86: convert threshold_bank.cpus from atomic_t to refcount_t
  sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t
  kvm: convert kvm.users_count from atomic_t to refcount_t

 arch/s390/include/asm/debug.h        |  3 ++-
 arch/s390/kernel/debug.c             |  8 ++++----
 arch/sparc/kernel/mdesc.c            | 17 +++++++++--------
 arch/x86/include/asm/amd_nb.h        |  3 ++-
 arch/x86/kernel/cpu/mcheck/mce_amd.c |  6 +++---
 include/linux/kvm_host.h             |  3 ++-
 virt/kvm/kvm_main.c                  |  8 ++++----
 7 files changed, 26 insertions(+), 22 deletions(-)

-- 
2.7.4

[PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Elena Reshetova]

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 arch/s390/include/asm/debug.h | 3 ++-
 arch/s390/kernel/debug.c      | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/s390/include/asm/debug.h b/arch/s390/include/asm/debug.h
index 0206c80..df7b54e 100644
--- a/arch/s390/include/asm/debug.h
+++ b/arch/s390/include/asm/debug.h
@@ -10,6 +10,7 @@
 #include <linux/spinlock.h>
 #include <linux/kernel.h>
 #include <linux/time.h>
+#include <linux/refcount.h>
 #include <uapi/asm/debug.h>
 
 #define DEBUG_MAX_LEVEL            6  /* debug levels range from 0 to 6 */
@@ -31,7 +32,7 @@ struct debug_view;
 typedef struct debug_info {	
 	struct debug_info* next;
 	struct debug_info* prev;
-	atomic_t ref_count;
+	refcount_t ref_count;
 	spinlock_t lock;			
 	int level;
 	int nr_areas;
diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
index 20a5a42..e4b9929 100644
--- a/arch/s390/kernel/debug.c
+++ b/arch/s390/kernel/debug.c
@@ -277,7 +277,7 @@ debug_info_alloc(const char *name, int pages_per_area, int nr_areas,
 	memset(rc->views, 0, DEBUG_MAX_VIEWS * sizeof(struct debug_view *));
 	memset(rc->debugfs_entries, 0 ,DEBUG_MAX_VIEWS *
 		sizeof(struct dentry*));
-	atomic_set(&(rc->ref_count), 0);
+	refcount_set(&(rc->ref_count), 0);
 
 	return rc;
 
@@ -361,7 +361,7 @@ debug_info_create(const char *name, int pages_per_area, int nr_areas,
         debug_area_last = rc;
         rc->next = NULL;
 
-	debug_info_get(rc);
+	refcount_set(&rc->ref_count, 1);
 out:
 	return rc;
 }
@@ -416,7 +416,7 @@ static void
 debug_info_get(debug_info_t * db_info)
 {
 	if (db_info)
-		atomic_inc(&db_info->ref_count);
+		refcount_inc(&db_info->ref_count);
 }
 
 /*
@@ -431,7 +431,7 @@ debug_info_put(debug_info_t *db_info)
 
 	if (!db_info)
 		return;
-	if (atomic_dec_and_test(&db_info->ref_count)) {
+	if (refcount_dec_and_test(&db_info->ref_count)) {
 		for (i = 0; i < DEBUG_MAX_VIEWS; i++) {
 			if (!db_info->views[i])
 				continue;
-- 
2.7.4

[PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Elena Reshetova]

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 arch/s390/include/asm/debug.h | 3 ++-
 arch/s390/kernel/debug.c      | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/s390/include/asm/debug.h b/arch/s390/include/asm/debug.h
index 0206c80..df7b54e 100644
--- a/arch/s390/include/asm/debug.h
+++ b/arch/s390/include/asm/debug.h
@@ -10,6 +10,7 @@
 #include <linux/spinlock.h>
 #include <linux/kernel.h>
 #include <linux/time.h>
+#include <linux/refcount.h>
 #include <uapi/asm/debug.h>
 
 #define DEBUG_MAX_LEVEL            6  /* debug levels range from 0 to 6 */
@@ -31,7 +32,7 @@ struct debug_view;
 typedef struct debug_info {	
 	struct debug_info* next;
 	struct debug_info* prev;
-	atomic_t ref_count;
+	refcount_t ref_count;
 	spinlock_t lock;			
 	int level;
 	int nr_areas;
diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
index 20a5a42..e4b9929 100644
--- a/arch/s390/kernel/debug.c
+++ b/arch/s390/kernel/debug.c
@@ -277,7 +277,7 @@ debug_info_alloc(const char *name, int pages_per_area, int nr_areas,
 	memset(rc->views, 0, DEBUG_MAX_VIEWS * sizeof(struct debug_view *));
 	memset(rc->debugfs_entries, 0 ,DEBUG_MAX_VIEWS *
 		sizeof(struct dentry*));
-	atomic_set(&(rc->ref_count), 0);
+	refcount_set(&(rc->ref_count), 0);
 
 	return rc;
 
@@ -361,7 +361,7 @@ debug_info_create(const char *name, int pages_per_area, int nr_areas,
         debug_area_last = rc;
         rc->next = NULL;
 
-	debug_info_get(rc);
+	refcount_set(&rc->ref_count, 1);
 out:
 	return rc;
 }
@@ -416,7 +416,7 @@ static void
 debug_info_get(debug_info_t * db_info)
 {
 	if (db_info)
-		atomic_inc(&db_info->ref_count);
+		refcount_inc(&db_info->ref_count);
 }
 
 /*
@@ -431,7 +431,7 @@ debug_info_put(debug_info_t *db_info)
 
 	if (!db_info)
 		return;
-	if (atomic_dec_and_test(&db_info->ref_count)) {
+	if (refcount_dec_and_test(&db_info->ref_count)) {
 		for (i = 0; i < DEBUG_MAX_VIEWS; i++) {
 			if (!db_info->views[i])
 				continue;
-- 
2.7.4

[PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Elena Reshetova]

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 arch/x86/include/asm/amd_nb.h        | 3 ++-
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index 00c88a0..da181ad 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -3,6 +3,7 @@
 
 #include <linux/ioport.h>
 #include <linux/pci.h>
+#include <linux/refcount.h>
 
 struct amd_nb_bus_dev_range {
 	u8 bus;
@@ -55,7 +56,7 @@ struct threshold_bank {
 	struct threshold_block	*blocks;
 
 	/* initialized to the number of CPUs on the node sharing this bank */
-	atomic_t		cpus;
+	refcount_t		cpus;
 };
 
 struct amd_northbridge {
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 524cc57..cfe0838 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -1202,7 +1202,7 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank)
 				goto out;
 
 			per_cpu(threshold_banks, cpu)[bank] = b;
-			atomic_inc(&b->cpus);
+			refcount_inc(&b->cpus);
 
 			err = __threshold_add_blocks(b);
 
@@ -1225,7 +1225,7 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank)
 	per_cpu(threshold_banks, cpu)[bank] = b;
 
 	if (is_shared_bank(bank)) {
-		atomic_set(&b->cpus, 1);
+		refcount_set(&b->cpus, 1);
 
 		/* nb is already initialized, see above */
 		if (nb) {
@@ -1289,7 +1289,7 @@ static void threshold_remove_bank(unsigned int cpu, int bank)
 		goto free_out;
 
 	if (is_shared_bank(bank)) {
-		if (!atomic_dec_and_test(&b->cpus)) {
+		if (!refcount_dec_and_test(&b->cpus)) {
 			__threshold_remove_blocks(b);
 			per_cpu(threshold_banks, cpu)[bank] = NULL;
 			return;
-- 
2.7.4

[PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Elena Reshetova]

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 arch/x86/include/asm/amd_nb.h        | 3 ++-
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index 00c88a0..da181ad 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -3,6 +3,7 @@
 
 #include <linux/ioport.h>
 #include <linux/pci.h>
+#include <linux/refcount.h>
 
 struct amd_nb_bus_dev_range {
 	u8 bus;
@@ -55,7 +56,7 @@ struct threshold_bank {
 	struct threshold_block	*blocks;
 
 	/* initialized to the number of CPUs on the node sharing this bank */
-	atomic_t		cpus;
+	refcount_t		cpus;
 };
 
 struct amd_northbridge {
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 524cc57..cfe0838 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -1202,7 +1202,7 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank)
 				goto out;
 
 			per_cpu(threshold_banks, cpu)[bank] = b;
-			atomic_inc(&b->cpus);
+			refcount_inc(&b->cpus);
 
 			err = __threshold_add_blocks(b);
 
@@ -1225,7 +1225,7 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank)
 	per_cpu(threshold_banks, cpu)[bank] = b;
 
 	if (is_shared_bank(bank)) {
-		atomic_set(&b->cpus, 1);
+		refcount_set(&b->cpus, 1);
 
 		/* nb is already initialized, see above */
 		if (nb) {
@@ -1289,7 +1289,7 @@ static void threshold_remove_bank(unsigned int cpu, int bank)
 		goto free_out;
 
 	if (is_shared_bank(bank)) {
-		if (!atomic_dec_and_test(&b->cpus)) {
+		if (!refcount_dec_and_test(&b->cpus)) {
 			__threshold_remove_blocks(b);
 			per_cpu(threshold_banks, cpu)[bank] = NULL;
 			return;
-- 
2.7.4

[PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[Elena Reshetova]

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 arch/sparc/kernel/mdesc.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c
index c0765bb..ac3fe0d 100644
--- a/arch/sparc/kernel/mdesc.c
+++ b/arch/sparc/kernel/mdesc.c
@@ -12,6 +12,7 @@
 #include <linux/miscdevice.h>
 #include <linux/bootmem.h>
 #include <linux/export.h>
+#include <linux/refcount.h>
 
 #include <asm/cpudata.h>
 #include <asm/hypervisor.h>
@@ -70,7 +71,7 @@ struct mdesc_handle {
 	struct list_head	list;
 	struct mdesc_mem_ops	*mops;
 	void			*self_base;
-	atomic_t		refcnt;
+	refcount_t		refcnt;
 	unsigned int		handle_size;
 	struct mdesc_hdr	mdesc;
 };
@@ -84,7 +85,7 @@ static void mdesc_handle_init(struct mdesc_handle *hp,
 	memset(hp, 0, handle_size);
 	INIT_LIST_HEAD(&hp->list);
 	hp->self_base = base;
-	atomic_set(&hp->refcnt, 1);
+	refcount_set(&hp->refcnt, 1);
 	hp->handle_size = handle_size;
 }
 
@@ -114,7 +115,7 @@ static void __init mdesc_memblock_free(struct mdesc_handle *hp)
 	unsigned int alloc_size;
 	unsigned long start;
 
-	BUG_ON(atomic_read(&hp->refcnt) != 0);
+	BUG_ON(refcount_read(&hp->refcnt) != 0);
 	BUG_ON(!list_empty(&hp->list));
 
 	alloc_size = PAGE_ALIGN(hp->handle_size);
@@ -154,7 +155,7 @@ static struct mdesc_handle *mdesc_kmalloc(unsigned int mdesc_size)
 
 static void mdesc_kfree(struct mdesc_handle *hp)
 {
-	BUG_ON(atomic_read(&hp->refcnt) != 0);
+	BUG_ON(refcount_read(&hp->refcnt) != 0);
 	BUG_ON(!list_empty(&hp->list));
 
 	kfree(hp->self_base);
@@ -193,7 +194,7 @@ struct mdesc_handle *mdesc_grab(void)
 	spin_lock_irqsave(&mdesc_lock, flags);
 	hp = cur_mdesc;
 	if (hp)
-		atomic_inc(&hp->refcnt);
+		refcount_inc(&hp->refcnt);
 	spin_unlock_irqrestore(&mdesc_lock, flags);
 
 	return hp;
@@ -205,7 +206,7 @@ void mdesc_release(struct mdesc_handle *hp)
 	unsigned long flags;
 
 	spin_lock_irqsave(&mdesc_lock, flags);
-	if (atomic_dec_and_test(&hp->refcnt)) {
+	if (refcount_dec_and_test(&hp->refcnt)) {
 		list_del_init(&hp->list);
 		hp->mops->free(hp);
 	}
@@ -344,7 +345,7 @@ void mdesc_update(void)
 	if (status != HV_EOK || real_len > len) {
 		printk(KERN_ERR "MD: mdesc reread fails with %lu\n",
 		       status);
-		atomic_dec(&hp->refcnt);
+		refcount_dec(&hp->refcnt);
 		mdesc_free(hp);
 		goto out;
 	}
@@ -357,7 +358,7 @@ void mdesc_update(void)
 	mdesc_notify_clients(orig_hp, hp);
 
 	spin_lock_irqsave(&mdesc_lock, flags);
-	if (atomic_dec_and_test(&orig_hp->refcnt))
+	if (refcount_dec_and_test(&orig_hp->refcnt))
 		mdesc_free(orig_hp);
 	else
 		list_add(&orig_hp->list, &mdesc_zombie_list);
-- 
2.7.4

[PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[Elena Reshetova]

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 arch/sparc/kernel/mdesc.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c
index c0765bb..ac3fe0d 100644
--- a/arch/sparc/kernel/mdesc.c
+++ b/arch/sparc/kernel/mdesc.c
@@ -12,6 +12,7 @@
 #include <linux/miscdevice.h>
 #include <linux/bootmem.h>
 #include <linux/export.h>
+#include <linux/refcount.h>
 
 #include <asm/cpudata.h>
 #include <asm/hypervisor.h>
@@ -70,7 +71,7 @@ struct mdesc_handle {
 	struct list_head	list;
 	struct mdesc_mem_ops	*mops;
 	void			*self_base;
-	atomic_t		refcnt;
+	refcount_t		refcnt;
 	unsigned int		handle_size;
 	struct mdesc_hdr	mdesc;
 };
@@ -84,7 +85,7 @@ static void mdesc_handle_init(struct mdesc_handle *hp,
 	memset(hp, 0, handle_size);
 	INIT_LIST_HEAD(&hp->list);
 	hp->self_base = base;
-	atomic_set(&hp->refcnt, 1);
+	refcount_set(&hp->refcnt, 1);
 	hp->handle_size = handle_size;
 }
 
@@ -114,7 +115,7 @@ static void __init mdesc_memblock_free(struct mdesc_handle *hp)
 	unsigned int alloc_size;
 	unsigned long start;
 
-	BUG_ON(atomic_read(&hp->refcnt) != 0);
+	BUG_ON(refcount_read(&hp->refcnt) != 0);
 	BUG_ON(!list_empty(&hp->list));
 
 	alloc_size = PAGE_ALIGN(hp->handle_size);
@@ -154,7 +155,7 @@ static struct mdesc_handle *mdesc_kmalloc(unsigned int mdesc_size)
 
 static void mdesc_kfree(struct mdesc_handle *hp)
 {
-	BUG_ON(atomic_read(&hp->refcnt) != 0);
+	BUG_ON(refcount_read(&hp->refcnt) != 0);
 	BUG_ON(!list_empty(&hp->list));
 
 	kfree(hp->self_base);
@@ -193,7 +194,7 @@ struct mdesc_handle *mdesc_grab(void)
 	spin_lock_irqsave(&mdesc_lock, flags);
 	hp = cur_mdesc;
 	if (hp)
-		atomic_inc(&hp->refcnt);
+		refcount_inc(&hp->refcnt);
 	spin_unlock_irqrestore(&mdesc_lock, flags);
 
 	return hp;
@@ -205,7 +206,7 @@ void mdesc_release(struct mdesc_handle *hp)
 	unsigned long flags;
 
 	spin_lock_irqsave(&mdesc_lock, flags);
-	if (atomic_dec_and_test(&hp->refcnt)) {
+	if (refcount_dec_and_test(&hp->refcnt)) {
 		list_del_init(&hp->list);
 		hp->mops->free(hp);
 	}
@@ -344,7 +345,7 @@ void mdesc_update(void)
 	if (status != HV_EOK || real_len > len) {
 		printk(KERN_ERR "MD: mdesc reread fails with %lu\n",
 		       status);
-		atomic_dec(&hp->refcnt);
+		refcount_dec(&hp->refcnt);
 		mdesc_free(hp);
 		goto out;
 	}
@@ -357,7 +358,7 @@ void mdesc_update(void)
 	mdesc_notify_clients(orig_hp, hp);
 
 	spin_lock_irqsave(&mdesc_lock, flags);
-	if (atomic_dec_and_test(&orig_hp->refcnt))
+	if (refcount_dec_and_test(&orig_hp->refcnt))
 		mdesc_free(orig_hp);
 	else
 		list_add(&orig_hp->list, &mdesc_zombie_list);
-- 
2.7.4

[PATCH 4/4] kvm: convert kvm.users_count from atomic_t to refcount_t

[Elena Reshetova]

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 include/linux/kvm_host.h | 3 ++-
 virt/kvm/kvm_main.c      | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index cda457b..7fa05a5 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -26,6 +26,7 @@
 #include <linux/context_tracking.h>
 #include <linux/irqbypass.h>
 #include <linux/swait.h>
+#include <linux/refcount.h>
 #include <asm/signal.h>
 
 #include <linux/kvm.h>
@@ -402,7 +403,7 @@ struct kvm {
 #endif
 	struct kvm_vm_stat stat;
 	struct kvm_arch arch;
-	atomic_t users_count;
+	refcount_t users_count;
 #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
 	struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
 	spinlock_t ring_lock;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index dcd1c12..6ae5775 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -622,7 +622,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
 	mutex_init(&kvm->lock);
 	mutex_init(&kvm->irq_lock);
 	mutex_init(&kvm->slots_lock);
-	atomic_set(&kvm->users_count, 1);
+	refcount_set(&kvm->users_count, 1);
 	INIT_LIST_HEAD(&kvm->devices);
 
 	r = kvm_arch_init_vm(kvm, type);
@@ -745,13 +745,13 @@ static void kvm_destroy_vm(struct kvm *kvm)
 
 void kvm_get_kvm(struct kvm *kvm)
 {
-	atomic_inc(&kvm->users_count);
+	refcount_inc(&kvm->users_count);
 }
 EXPORT_SYMBOL_GPL(kvm_get_kvm);
 
 void kvm_put_kvm(struct kvm *kvm)
 {
-	if (atomic_dec_and_test(&kvm->users_count))
+	if (refcount_dec_and_test(&kvm->users_count))
 		kvm_destroy_vm(kvm);
 }
 EXPORT_SYMBOL_GPL(kvm_put_kvm);
@@ -3640,7 +3640,7 @@ static int kvm_debugfs_open(struct inode *inode, struct file *file,
 	 * To avoid the race between open and the removal of the debugfs
 	 * directory we test against the users count.
 	 */
-	if (!atomic_add_unless(&stat_data->kvm->users_count, 1, 0))
+	if (!refcount_inc_not_zero(&stat_data->kvm->users_count))
 		return -ENOENT;
 
 	if (simple_attr_open(inode, file, get, set, fmt)) {
-- 
2.7.4

[PATCH 4/4] kvm: convert kvm.users_count from atomic_t to refcount_t

[Elena Reshetova]

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 include/linux/kvm_host.h | 3 ++-
 virt/kvm/kvm_main.c      | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index cda457b..7fa05a5 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -26,6 +26,7 @@
 #include <linux/context_tracking.h>
 #include <linux/irqbypass.h>
 #include <linux/swait.h>
+#include <linux/refcount.h>
 #include <asm/signal.h>
 
 #include <linux/kvm.h>
@@ -402,7 +403,7 @@ struct kvm {
 #endif
 	struct kvm_vm_stat stat;
 	struct kvm_arch arch;
-	atomic_t users_count;
+	refcount_t users_count;
 #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
 	struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
 	spinlock_t ring_lock;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index dcd1c12..6ae5775 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -622,7 +622,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
 	mutex_init(&kvm->lock);
 	mutex_init(&kvm->irq_lock);
 	mutex_init(&kvm->slots_lock);
-	atomic_set(&kvm->users_count, 1);
+	refcount_set(&kvm->users_count, 1);
 	INIT_LIST_HEAD(&kvm->devices);
 
 	r = kvm_arch_init_vm(kvm, type);
@@ -745,13 +745,13 @@ static void kvm_destroy_vm(struct kvm *kvm)
 
 void kvm_get_kvm(struct kvm *kvm)
 {
-	atomic_inc(&kvm->users_count);
+	refcount_inc(&kvm->users_count);
 }
 EXPORT_SYMBOL_GPL(kvm_get_kvm);
 
 void kvm_put_kvm(struct kvm *kvm)
 {
-	if (atomic_dec_and_test(&kvm->users_count))
+	if (refcount_dec_and_test(&kvm->users_count))
 		kvm_destroy_vm(kvm);
 }
 EXPORT_SYMBOL_GPL(kvm_put_kvm);
@@ -3640,7 +3640,7 @@ static int kvm_debugfs_open(struct inode *inode, struct file *file,
 	 * To avoid the race between open and the removal of the debugfs
 	 * directory we test against the users count.
 	 */
-	if (!atomic_add_unless(&stat_data->kvm->users_count, 1, 0))
+	if (!refcount_inc_not_zero(&stat_data->kvm->users_count))
 		return -ENOENT;
 
 	if (simple_attr_open(inode, file, get, set, fmt)) {
-- 
2.7.4

Re: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Borislav Petkov]

On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>

That SOB chain tells me that you wrote the patch and Hans, Kees and
David handled it in some way and the last one - David - is sending it to
me. It doesn't look like that though.

So what are you trying to express with it?

Documentation/process/submitting-patches.rst has some info on the whole
SOB deal:

11) Sign your work — the Developer's Certificate of Origin
----------------------------------------------------------
..


-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Borislav Petkov]

On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>

That SOB chain tells me that you wrote the patch and Hans, Kees and
David handled it in some way and the last one - David - is sending it to
me. It doesn't look like that though.

So what are you trying to express with it?

Documentation/process/submitting-patches.rst has some info on the whole
SOB deal:

11) Sign your work — the Developer's Certificate of Origin
----------------------------------------------------------
..


-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 4/4] kvm: convert kvm.users_count from atomic_t to refcount_t

[Paolo Bonzini]

On 20/02/2017 12:06, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>

Looks good, will apply after the merge window.

Paolo

> ---
>  include/linux/kvm_host.h | 3 ++-
>  virt/kvm/kvm_main.c      | 8 ++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index cda457b..7fa05a5 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -26,6 +26,7 @@
>  #include <linux/context_tracking.h>
>  #include <linux/irqbypass.h>
>  #include <linux/swait.h>
> +#include <linux/refcount.h>
>  #include <asm/signal.h>
>  
>  #include <linux/kvm.h>
> @@ -402,7 +403,7 @@ struct kvm {
>  #endif
>  	struct kvm_vm_stat stat;
>  	struct kvm_arch arch;
> -	atomic_t users_count;
> +	refcount_t users_count;
>  #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
>  	struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
>  	spinlock_t ring_lock;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index dcd1c12..6ae5775 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -622,7 +622,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>  	mutex_init(&kvm->lock);
>  	mutex_init(&kvm->irq_lock);
>  	mutex_init(&kvm->slots_lock);
> -	atomic_set(&kvm->users_count, 1);
> +	refcount_set(&kvm->users_count, 1);
>  	INIT_LIST_HEAD(&kvm->devices);
>  
>  	r = kvm_arch_init_vm(kvm, type);
> @@ -745,13 +745,13 @@ static void kvm_destroy_vm(struct kvm *kvm)
>  
>  void kvm_get_kvm(struct kvm *kvm)
>  {
> -	atomic_inc(&kvm->users_count);
> +	refcount_inc(&kvm->users_count);
>  }
>  EXPORT_SYMBOL_GPL(kvm_get_kvm);
>  
>  void kvm_put_kvm(struct kvm *kvm)
>  {
> -	if (atomic_dec_and_test(&kvm->users_count))
> +	if (refcount_dec_and_test(&kvm->users_count))
>  		kvm_destroy_vm(kvm);
>  }
>  EXPORT_SYMBOL_GPL(kvm_put_kvm);
> @@ -3640,7 +3640,7 @@ static int kvm_debugfs_open(struct inode *inode, struct file *file,
>  	 * To avoid the race between open and the removal of the debugfs
>  	 * directory we test against the users count.
>  	 */
> -	if (!atomic_add_unless(&stat_data->kvm->users_count, 1, 0))
> +	if (!refcount_inc_not_zero(&stat_data->kvm->users_count))
>  		return -ENOENT;
>  
>  	if (simple_attr_open(inode, file, get, set, fmt)) {
> 

Re: [PATCH 4/4] kvm: convert kvm.users_count from atomic_t to refcount_t

[Paolo Bonzini]

On 20/02/2017 12:06, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>

Looks good, will apply after the merge window.

Paolo

> ---
>  include/linux/kvm_host.h | 3 ++-
>  virt/kvm/kvm_main.c      | 8 ++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index cda457b..7fa05a5 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -26,6 +26,7 @@
>  #include <linux/context_tracking.h>
>  #include <linux/irqbypass.h>
>  #include <linux/swait.h>
> +#include <linux/refcount.h>
>  #include <asm/signal.h>
>  
>  #include <linux/kvm.h>
> @@ -402,7 +403,7 @@ struct kvm {
>  #endif
>  	struct kvm_vm_stat stat;
>  	struct kvm_arch arch;
> -	atomic_t users_count;
> +	refcount_t users_count;
>  #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
>  	struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
>  	spinlock_t ring_lock;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index dcd1c12..6ae5775 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -622,7 +622,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>  	mutex_init(&kvm->lock);
>  	mutex_init(&kvm->irq_lock);
>  	mutex_init(&kvm->slots_lock);
> -	atomic_set(&kvm->users_count, 1);
> +	refcount_set(&kvm->users_count, 1);
>  	INIT_LIST_HEAD(&kvm->devices);
>  
>  	r = kvm_arch_init_vm(kvm, type);
> @@ -745,13 +745,13 @@ static void kvm_destroy_vm(struct kvm *kvm)
>  
>  void kvm_get_kvm(struct kvm *kvm)
>  {
> -	atomic_inc(&kvm->users_count);
> +	refcount_inc(&kvm->users_count);
>  }
>  EXPORT_SYMBOL_GPL(kvm_get_kvm);
>  
>  void kvm_put_kvm(struct kvm *kvm)
>  {
> -	if (atomic_dec_and_test(&kvm->users_count))
> +	if (refcount_dec_and_test(&kvm->users_count))
>  		kvm_destroy_vm(kvm);
>  }
>  EXPORT_SYMBOL_GPL(kvm_put_kvm);
> @@ -3640,7 +3640,7 @@ static int kvm_debugfs_open(struct inode *inode, struct file *file,
>  	 * To avoid the race between open and the removal of the debugfs
>  	 * directory we test against the users count.
>  	 */
> -	if (!atomic_add_unless(&stat_data->kvm->users_count, 1, 0))
> +	if (!refcount_inc_not_zero(&stat_data->kvm->users_count))
>  		return -ENOENT;
>  
>  	if (simple_attr_open(inode, file, get, set, fmt)) {
> 

RE: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Reshetova, Elena]

> On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote:
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> 
> That SOB chain tells me that you wrote the patch and Hans, Kees and
> David handled it in some way and the last one - David - is sending it to
> me. It doesn't look like that though.
> 
> So what are you trying to express with it?

Whole refcount conversion was a long piece of work and the above people contributed to this code either as
writes or reviewers or both.  I am primary writer of the code and I am handing patches in our tree and sending them out, 
so how exactly the above should look like? 

Please note that we have about 300 patches and if I have to modify each sign-off to
reflect who contributed to each commit in what particular way, I will go insane.  

Best Regards,
Elena.

RE: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Reshetova, Elena]

DQo+IE9uIE1vbiwgRmViIDIwLCAyMDE3IGF0IDAxOjA2OjE5UE0gKzAyMDAsIEVsZW5hIFJlc2hl
dG92YSB3cm90ZToNCj4gPiByZWZjb3VudF90IHR5cGUgYW5kIGNvcnJlc3BvbmRpbmcgQVBJIHNo
b3VsZCBiZQ0KPiA+IHVzZWQgaW5zdGVhZCBvZiBhdG9taWNfdCB3aGVuIHRoZSB2YXJpYWJsZSBp
cyB1c2VkIGFzDQo+ID4gYSByZWZlcmVuY2UgY291bnRlci4gVGhpcyBhbGxvd3MgdG8gYXZvaWQg
YWNjaWRlbnRhbA0KPiA+IHJlZmNvdW50ZXIgb3ZlcmZsb3dzIHRoYXQgbWlnaHQgbGVhZCB0byB1
c2UtYWZ0ZXItZnJlZQ0KPiA+IHNpdHVhdGlvbnMuDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBF
bGVuYSBSZXNoZXRvdmEgPGVsZW5hLnJlc2hldG92YUBpbnRlbC5jb20+DQo+ID4gU2lnbmVkLW9m
Zi1ieTogSGFucyBMaWxqZXN0cmFuZCA8aXNoa2FtaWVsQGdtYWlsLmNvbT4NCj4gPiBTaWduZWQt
b2ZmLWJ5OiBLZWVzIENvb2sgPGtlZXNjb29rQGNocm9taXVtLm9yZz4NCj4gPiBTaWduZWQtb2Zm
LWJ5OiBEYXZpZCBXaW5kc29yIDxkd2luZHNvckBnbWFpbC5jb20+DQo+IA0KPiBUaGF0IFNPQiBj
aGFpbiB0ZWxscyBtZSB0aGF0IHlvdSB3cm90ZSB0aGUgcGF0Y2ggYW5kIEhhbnMsIEtlZXMgYW5k
DQo+IERhdmlkIGhhbmRsZWQgaXQgaW4gc29tZSB3YXkgYW5kIHRoZSBsYXN0IG9uZSAtIERhdmlk
IC0gaXMgc2VuZGluZyBpdCB0bw0KPiBtZS4gSXQgZG9lc24ndCBsb29rIGxpa2UgdGhhdCB0aG91
Z2guDQo+IA0KPiBTbyB3aGF0IGFyZSB5b3UgdHJ5aW5nIHRvIGV4cHJlc3Mgd2l0aCBpdD8NCg0K
V2hvbGUgcmVmY291bnQgY29udmVyc2lvbiB3YXMgYSBsb25nIHBpZWNlIG9mIHdvcmsgYW5kIHRo
ZSBhYm92ZSBwZW9wbGUgY29udHJpYnV0ZWQgdG8gdGhpcyBjb2RlIGVpdGhlciBhcw0Kd3JpdGVz
IG9yIHJldmlld2VycyBvciBib3RoLiAgSSBhbSBwcmltYXJ5IHdyaXRlciBvZiB0aGUgY29kZSBh
bmQgSSBhbSBoYW5kaW5nIHBhdGNoZXMgaW4gb3VyIHRyZWUgYW5kIHNlbmRpbmcgdGhlbSBvdXQs
IA0Kc28gaG93IGV4YWN0bHkgdGhlIGFib3ZlIHNob3VsZCBsb29rIGxpa2U/IA0KDQpQbGVhc2Ug
bm90ZSB0aGF0IHdlIGhhdmUgYWJvdXQgMzAwIHBhdGNoZXMgYW5kIGlmIEkgaGF2ZSB0byBtb2Rp
ZnkgZWFjaCBzaWduLW9mZiB0bw0KcmVmbGVjdCB3aG8gY29udHJpYnV0ZWQgdG8gZWFjaCBjb21t
aXQgaW4gd2hhdCBwYXJ0aWN1bGFyIHdheSwgSSB3aWxsIGdvIGluc2FuZS4gIA0KDQpCZXN0IFJl
Z2FyZHMsDQpFbGVuYS4NCg=

RE: [PATCH 4/4] kvm: convert kvm.users_count from atomic_t to refcount_t

[Reshetova, Elena]

> On 20/02/2017 12:06, Elena Reshetova wrote:
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> 
> Looks good, will apply after the merge window.
> 
> Paolo

Thank you very much!

Best Regards,
Elena.

> 
> > ---
> >  include/linux/kvm_host.h | 3 ++-
> >  virt/kvm/kvm_main.c      | 8 ++++----
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index cda457b..7fa05a5 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -26,6 +26,7 @@
> >  #include <linux/context_tracking.h>
> >  #include <linux/irqbypass.h>
> >  #include <linux/swait.h>
> > +#include <linux/refcount.h>
> >  #include <asm/signal.h>
> >
> >  #include <linux/kvm.h>
> > @@ -402,7 +403,7 @@ struct kvm {
> >  #endif
> >  	struct kvm_vm_stat stat;
> >  	struct kvm_arch arch;
> > -	atomic_t users_count;
> > +	refcount_t users_count;
> >  #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
> >  	struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
> >  	spinlock_t ring_lock;
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index dcd1c12..6ae5775 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -622,7 +622,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
> >  	mutex_init(&kvm->lock);
> >  	mutex_init(&kvm->irq_lock);
> >  	mutex_init(&kvm->slots_lock);
> > -	atomic_set(&kvm->users_count, 1);
> > +	refcount_set(&kvm->users_count, 1);
> >  	INIT_LIST_HEAD(&kvm->devices);
> >
> >  	r = kvm_arch_init_vm(kvm, type);
> > @@ -745,13 +745,13 @@ static void kvm_destroy_vm(struct kvm *kvm)
> >
> >  void kvm_get_kvm(struct kvm *kvm)
> >  {
> > -	atomic_inc(&kvm->users_count);
> > +	refcount_inc(&kvm->users_count);
> >  }
> >  EXPORT_SYMBOL_GPL(kvm_get_kvm);
> >
> >  void kvm_put_kvm(struct kvm *kvm)
> >  {
> > -	if (atomic_dec_and_test(&kvm->users_count))
> > +	if (refcount_dec_and_test(&kvm->users_count))
> >  		kvm_destroy_vm(kvm);
> >  }
> >  EXPORT_SYMBOL_GPL(kvm_put_kvm);
> > @@ -3640,7 +3640,7 @@ static int kvm_debugfs_open(struct inode *inode,
> struct file *file,
> >  	 * To avoid the race between open and the removal of the debugfs
> >  	 * directory we test against the users count.
> >  	 */
> > -	if (!atomic_add_unless(&stat_data->kvm->users_count, 1, 0))
> > +	if (!refcount_inc_not_zero(&stat_data->kvm->users_count))
> >  		return -ENOENT;
> >
> >  	if (simple_attr_open(inode, file, get, set, fmt)) {
> >

RE: [PATCH 4/4] kvm: convert kvm.users_count from atomic_t to refcount_t

[Reshetova, Elena]

PiBPbiAyMC8wMi8yMDE3IDEyOjA2LCBFbGVuYSBSZXNoZXRvdmEgd3JvdGU6DQo+ID4gcmVmY291
bnRfdCB0eXBlIGFuZCBjb3JyZXNwb25kaW5nIEFQSSBzaG91bGQgYmUNCj4gPiB1c2VkIGluc3Rl
YWQgb2YgYXRvbWljX3Qgd2hlbiB0aGUgdmFyaWFibGUgaXMgdXNlZCBhcw0KPiA+IGEgcmVmZXJl
bmNlIGNvdW50ZXIuIFRoaXMgYWxsb3dzIHRvIGF2b2lkIGFjY2lkZW50YWwNCj4gPiByZWZjb3Vu
dGVyIG92ZXJmbG93cyB0aGF0IG1pZ2h0IGxlYWQgdG8gdXNlLWFmdGVyLWZyZWUNCj4gPiBzaXR1
YXRpb25zLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogRWxlbmEgUmVzaGV0b3ZhIDxlbGVuYS5y
ZXNoZXRvdmFAaW50ZWwuY29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6IEhhbnMgTGlsamVzdHJhbmQg
PGlzaGthbWllbEBnbWFpbC5jb20+DQo+ID4gU2lnbmVkLW9mZi1ieTogS2VlcyBDb29rIDxrZWVz
Y29va0BjaHJvbWl1bS5vcmc+DQo+ID4gU2lnbmVkLW9mZi1ieTogRGF2aWQgV2luZHNvciA8ZHdp
bmRzb3JAZ21haWwuY29tPg0KPiANCj4gTG9va3MgZ29vZCwgd2lsbCBhcHBseSBhZnRlciB0aGUg
bWVyZ2Ugd2luZG93Lg0KPiANCj4gUGFvbG8NCg0KVGhhbmsgeW91IHZlcnkgbXVjaCENCg0KQmVz
dCBSZWdhcmRzLA0KRWxlbmEuDQoNCj4gDQo+ID4gLS0tDQo+ID4gIGluY2x1ZGUvbGludXgva3Zt
X2hvc3QuaCB8IDMgKystDQo+ID4gIHZpcnQva3ZtL2t2bV9tYWluLmMgICAgICB8IDggKysrKy0t
LS0NCj4gPiAgMiBmaWxlcyBjaGFuZ2VkLCA2IGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25zKC0p
DQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC9rdm1faG9zdC5oIGIvaW5jbHVk
ZS9saW51eC9rdm1faG9zdC5oDQo+ID4gaW5kZXggY2RhNDU3Yi4uN2ZhMDVhNSAxMDA2NDQNCj4g
PiAtLS0gYS9pbmNsdWRlL2xpbnV4L2t2bV9ob3N0LmgNCj4gPiArKysgYi9pbmNsdWRlL2xpbnV4
L2t2bV9ob3N0LmgNCj4gPiBAQCAtMjYsNiArMjYsNyBAQA0KPiA+ICAjaW5jbHVkZSA8bGludXgv
Y29udGV4dF90cmFja2luZy5oPg0KPiA+ICAjaW5jbHVkZSA8bGludXgvaXJxYnlwYXNzLmg+DQo+
ID4gICNpbmNsdWRlIDxsaW51eC9zd2FpdC5oPg0KPiA+ICsjaW5jbHVkZSA8bGludXgvcmVmY291
bnQuaD4NCj4gPiAgI2luY2x1ZGUgPGFzbS9zaWduYWwuaD4NCj4gPg0KPiA+ICAjaW5jbHVkZSA8
bGludXgva3ZtLmg+DQo+ID4gQEAgLTQwMiw3ICs0MDMsNyBAQCBzdHJ1Y3Qga3ZtIHsNCj4gPiAg
I2VuZGlmDQo+ID4gIAlzdHJ1Y3Qga3ZtX3ZtX3N0YXQgc3RhdDsNCj4gPiAgCXN0cnVjdCBrdm1f
YXJjaCBhcmNoOw0KPiA+IC0JYXRvbWljX3QgdXNlcnNfY291bnQ7DQo+ID4gKwlyZWZjb3VudF90
IHVzZXJzX2NvdW50Ow0KPiA+ICAjaWZkZWYgS1ZNX0NPQUxFU0NFRF9NTUlPX1BBR0VfT0ZGU0VU
DQo+ID4gIAlzdHJ1Y3Qga3ZtX2NvYWxlc2NlZF9tbWlvX3JpbmcgKmNvYWxlc2NlZF9tbWlvX3Jp
bmc7DQo+ID4gIAlzcGlubG9ja190IHJpbmdfbG9jazsNCj4gPiBkaWZmIC0tZ2l0IGEvdmlydC9r
dm0va3ZtX21haW4uYyBiL3ZpcnQva3ZtL2t2bV9tYWluLmMNCj4gPiBpbmRleCBkY2QxYzEyLi42
YWU1Nzc1IDEwMDY0NA0KPiA+IC0tLSBhL3ZpcnQva3ZtL2t2bV9tYWluLmMNCj4gPiArKysgYi92
aXJ0L2t2bS9rdm1fbWFpbi5jDQo+ID4gQEAgLTYyMiw3ICs2MjIsNyBAQCBzdGF0aWMgc3RydWN0
IGt2bSAqa3ZtX2NyZWF0ZV92bSh1bnNpZ25lZCBsb25nIHR5cGUpDQo+ID4gIAltdXRleF9pbml0
KCZrdm0tPmxvY2spOw0KPiA+ICAJbXV0ZXhfaW5pdCgma3ZtLT5pcnFfbG9jayk7DQo+ID4gIAlt
dXRleF9pbml0KCZrdm0tPnNsb3RzX2xvY2spOw0KPiA+IC0JYXRvbWljX3NldCgma3ZtLT51c2Vy
c19jb3VudCwgMSk7DQo+ID4gKwlyZWZjb3VudF9zZXQoJmt2bS0+dXNlcnNfY291bnQsIDEpOw0K
PiA+ICAJSU5JVF9MSVNUX0hFQUQoJmt2bS0+ZGV2aWNlcyk7DQo+ID4NCj4gPiAgCXIgPSBrdm1f
YXJjaF9pbml0X3ZtKGt2bSwgdHlwZSk7DQo+ID4gQEAgLTc0NSwxMyArNzQ1LDEzIEBAIHN0YXRp
YyB2b2lkIGt2bV9kZXN0cm95X3ZtKHN0cnVjdCBrdm0gKmt2bSkNCj4gPg0KPiA+ICB2b2lkIGt2
bV9nZXRfa3ZtKHN0cnVjdCBrdm0gKmt2bSkNCj4gPiAgew0KPiA+IC0JYXRvbWljX2luYygma3Zt
LT51c2Vyc19jb3VudCk7DQo+ID4gKwlyZWZjb3VudF9pbmMoJmt2bS0+dXNlcnNfY291bnQpOw0K
PiA+ICB9DQo+ID4gIEVYUE9SVF9TWU1CT0xfR1BMKGt2bV9nZXRfa3ZtKTsNCj4gPg0KPiA+ICB2
b2lkIGt2bV9wdXRfa3ZtKHN0cnVjdCBrdm0gKmt2bSkNCj4gPiAgew0KPiA+IC0JaWYgKGF0b21p
Y19kZWNfYW5kX3Rlc3QoJmt2bS0+dXNlcnNfY291bnQpKQ0KPiA+ICsJaWYgKHJlZmNvdW50X2Rl
Y19hbmRfdGVzdCgma3ZtLT51c2Vyc19jb3VudCkpDQo+ID4gIAkJa3ZtX2Rlc3Ryb3lfdm0oa3Zt
KTsNCj4gPiAgfQ0KPiA+ICBFWFBPUlRfU1lNQk9MX0dQTChrdm1fcHV0X2t2bSk7DQo+ID4gQEAg
LTM2NDAsNyArMzY0MCw3IEBAIHN0YXRpYyBpbnQga3ZtX2RlYnVnZnNfb3BlbihzdHJ1Y3QgaW5v
ZGUgKmlub2RlLA0KPiBzdHJ1Y3QgZmlsZSAqZmlsZSwNCj4gPiAgCSAqIFRvIGF2b2lkIHRoZSBy
YWNlIGJldHdlZW4gb3BlbiBhbmQgdGhlIHJlbW92YWwgb2YgdGhlIGRlYnVnZnMNCj4gPiAgCSAq
IGRpcmVjdG9yeSB3ZSB0ZXN0IGFnYWluc3QgdGhlIHVzZXJzIGNvdW50Lg0KPiA+ICAJICovDQo+
ID4gLQlpZiAoIWF0b21pY19hZGRfdW5sZXNzKCZzdGF0X2RhdGEtPmt2bS0+dXNlcnNfY291bnQs
IDEsIDApKQ0KPiA+ICsJaWYgKCFyZWZjb3VudF9pbmNfbm90X3plcm8oJnN0YXRfZGF0YS0+a3Zt
LT51c2Vyc19jb3VudCkpDQo+ID4gIAkJcmV0dXJuIC1FTk9FTlQ7DQo+ID4NCj4gPiAgCWlmIChz
aW1wbGVfYXR0cl9vcGVuKGlub2RlLCBmaWxlLCBnZXQsIHNldCwgZm10KSkgew0KPiA+DQo

Re: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Borislav Petkov]

On Mon, Feb 20, 2017 at 12:20:04PM +0000, Reshetova, Elena wrote:
> Whole refcount conversion was a long piece of work and the above
> people contributed to this code either as writes or reviewers or both.

We have

Reviewed-by:

for reviewers.

> I am primary writer of the code and I am handing patches in our tree
> and sending them out, so how exactly the above should look like?

Well, the SOB chain should reflect who handled the patch on its way from
the original author to the upstream committer. If you want to express
who contributed, you can use Originally-by, Suggested-by, ...

You could also have free text in the commit message:

"People who contributed to this: ..."

In any case, it is all in Documentation/process/submitting-patches.rst.
Have a look.

> Please note that we have about 300 patches and if I have to modify
> each sign-off to reflect who contributed to each commit in what
> particular way, I will go insane.

There's sed/awk/perl/python/bash... whatever tools that can do the
proper conversions with.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Borislav Petkov]

On Mon, Feb 20, 2017 at 12:20:04PM +0000, Reshetova, Elena wrote:
> Whole refcount conversion was a long piece of work and the above
> people contributed to this code either as writes or reviewers or both.

We have

Reviewed-by:

for reviewers.

> I am primary writer of the code and I am handing patches in our tree
> and sending them out, so how exactly the above should look like?

Well, the SOB chain should reflect who handled the patch on its way from
the original author to the upstream committer. If you want to express
who contributed, you can use Originally-by, Suggested-by, ...

You could also have free text in the commit message:

"People who contributed to this: ..."

In any case, it is all in Documentation/process/submitting-patches.rst.
Have a look.

> Please note that we have about 300 patches and if I have to modify
> each sign-off to reflect who contributed to each commit in what
> particular way, I will go insane.

There's sed/awk/perl/python/bash... whatever tools that can do the
proper conversions with.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Heiko Carstens]

On Mon, Feb 20, 2017 at 01:06:18PM +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
>  arch/s390/include/asm/debug.h | 3 ++-
>  arch/s390/kernel/debug.c      | 8 ++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)

I can only see a pull request from Ingo a couple of hours ago for Peter's
refcount code. So the refcount code is not merged yet. It would have been
good if you would have waited until it is really merged to avoid confusion.

> @@ -361,7 +361,7 @@ debug_info_create(const char *name, int pages_per_area, int nr_areas,
>          debug_area_last = rc;
>          rc->next = NULL;
>  
> -	debug_info_get(rc);
> +	refcount_set(&rc->ref_count, 1);

This is not wrong, but I will remove this hunk before applying your patch,
since this doesn't look like an obvious correct change at first glance.

Thanks,
Heiko

Re: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Heiko Carstens]

On Mon, Feb 20, 2017 at 01:06:18PM +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
>  arch/s390/include/asm/debug.h | 3 ++-
>  arch/s390/kernel/debug.c      | 8 ++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)

I can only see a pull request from Ingo a couple of hours ago for Peter's
refcount code. So the refcount code is not merged yet. It would have been
good if you would have waited until it is really merged to avoid confusion.

> @@ -361,7 +361,7 @@ debug_info_create(const char *name, int pages_per_area, int nr_areas,
>          debug_area_last = rc;
>          rc->next = NULL;
>  
> -	debug_info_get(rc);
> +	refcount_set(&rc->ref_count, 1);

This is not wrong, but I will remove this hunk before applying your patch,
since this doesn't look like an obvious correct change at first glance.

Thanks,
Heiko

Re: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Heiko Carstens]

On Mon, Feb 20, 2017 at 02:24:24PM +0100, Heiko Carstens wrote:
> > @@ -361,7 +361,7 @@ debug_info_create(const char *name, int pages_per_area, int nr_areas,
> >          debug_area_last = rc;
> >          rc->next = NULL;
> >  
> > -	debug_info_get(rc);
> > +	refcount_set(&rc->ref_count, 1);
> 
> This is not wrong, but I will remove this hunk before applying your patch,
> since this doesn't look like an obvious correct change at first glance.

Actually your version is needed - just looked at refcount_inc().
Sorry for the confusion in my side now.

Re: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Heiko Carstens]

On Mon, Feb 20, 2017 at 02:24:24PM +0100, Heiko Carstens wrote:
> > @@ -361,7 +361,7 @@ debug_info_create(const char *name, int pages_per_area, int nr_areas,
> >          debug_area_last = rc;
> >          rc->next = NULL;
> >  
> > -	debug_info_get(rc);
> > +	refcount_set(&rc->ref_count, 1);
> 
> This is not wrong, but I will remove this hunk before applying your patch,
> since this doesn't look like an obvious correct change at first glance.

Actually your version is needed - just looked at refcount_inc().
Sorry for the confusion in my side now.

RE: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Reshetova, Elena]

> On Mon, Feb 20, 2017 at 01:06:18PM +0200, Elena Reshetova wrote:
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> > ---
> >  arch/s390/include/asm/debug.h | 3 ++-
> >  arch/s390/kernel/debug.c      | 8 ++++----
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> I can only see a pull request from Ingo a couple of hours ago for Peter's
> refcount code. So the refcount code is not merged yet. It would have been
> good if you would have waited until it is really merged to avoid confusion.

Sorry, I guess I was a bit too rushy, but I also want to be able to fix all things that come up as I post these before next merge window closes. 
> 
> > @@ -361,7 +361,7 @@ debug_info_create(const char *name, int
> pages_per_area, int nr_areas,
> >          debug_area_last = rc;
> >          rc->next = NULL;
> >
> > -	debug_info_get(rc);
> > +	refcount_set(&rc->ref_count, 1);
> 
> This is not wrong, but I will remove this hunk before applying your patch,
> since this doesn't look like an obvious correct change at first glance.

It isn't obvious, but needed unfortunately. refcount_inc is done in the way that it won't increment on zero value.
And since for this variable you set the initial refcounter value to zero and then call debug_info_get (that does inc), this
would only WARN and not increment. So for this initial case, we changed it to call refcount_set to "1" to make sure things work
as before.

Best Regards,
Elena.

Re: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Peter Zijlstra]

On Mon, Feb 20, 2017 at 02:24:24PM +0100, Heiko Carstens wrote:
> On Mon, Feb 20, 2017 at 01:06:18PM +0200, Elena Reshetova wrote:

> > @@ -361,7 +361,7 @@ debug_info_create(const char *name, int pages_per_area, int nr_areas,
> >          debug_area_last = rc;
> >          rc->next = NULL;
> >  
> > -	debug_info_get(rc);
> > +	refcount_set(&rc->ref_count, 1);
> 
> This is not wrong, but I will remove this hunk before applying your patch,
> since this doesn't look like an obvious correct change at first glance.

I suspect; but have not looked at the code; that this would otherwise
attempt to do a 0 -> 1 increment, which refcount_inc() will refuse (and
WARN) over.

Re: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Peter Zijlstra]

On Mon, Feb 20, 2017 at 02:24:24PM +0100, Heiko Carstens wrote:
> On Mon, Feb 20, 2017 at 01:06:18PM +0200, Elena Reshetova wrote:

> > @@ -361,7 +361,7 @@ debug_info_create(const char *name, int pages_per_area, int nr_areas,
> >          debug_area_last = rc;
> >          rc->next = NULL;
> >  
> > -	debug_info_get(rc);
> > +	refcount_set(&rc->ref_count, 1);
> 
> This is not wrong, but I will remove this hunk before applying your patch,
> since this doesn't look like an obvious correct change at first glance.

I suspect; but have not looked at the code; that this would otherwise
attempt to do a 0 -> 1 increment, which refcount_inc() will refuse (and
WARN) over.

Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[David Miller]

From: Elena Reshetova <elena.reshetova@intel.com>
Date: Mon, 20 Feb 2017 13:06:20 +0200

> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[David Miller]

From: Elena Reshetova <elena.reshetova@intel.com>
Date: Mon, 20 Feb 2017 13:06:20 +0200

> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Kees Cook]

On Mon, Feb 20, 2017 at 3:17 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote:
>> refcount_t type and corresponding API should be
>> used instead of atomic_t when the variable is used as
>> a reference counter. This allows to avoid accidental
>> refcounter overflows that might lead to use-after-free
>> situations.
>>
>> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
>> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: David Windsor <dwindsor@gmail.com>
>
> That SOB chain tells me that you wrote the patch and Hans, Kees and
> David handled it in some way and the last one - David - is sending it to
> me. It doesn't look like that though.

Perhaps the least inaccurate form of this might be:


Inspired by atomic protections in PaX/grsecurity.

Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>


As this is something I'd suggested we implement based on the work in
PaX/grsecurity, David took the first (and continuing) stab at
conversions, Hans did more, and Elena has been doing even more along
with the heavy-lifting of keeping the series organized. That way the
first SoB is still the author, the last SoB is still the email sender,
and everyone's name is mentioned.

Or just:


Inspired by atomic protections in PaX/grsecurity, based on work from
David Windsor, Hans Liljestrand, and myself.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>


I'm not picky -- I just want to see the conversion to refcount_t
happen, and everyone in Elena's SoB list has done work on it...

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Kees Cook]

On Mon, Feb 20, 2017 at 3:17 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote:
>> refcount_t type and corresponding API should be
>> used instead of atomic_t when the variable is used as
>> a reference counter. This allows to avoid accidental
>> refcounter overflows that might lead to use-after-free
>> situations.
>>
>> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
>> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: David Windsor <dwindsor@gmail.com>
>
> That SOB chain tells me that you wrote the patch and Hans, Kees and
> David handled it in some way and the last one - David - is sending it to
> me. It doesn't look like that though.

Perhaps the least inaccurate form of this might be:


Inspired by atomic protections in PaX/grsecurity.

Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Windsor <dwindsor@gmail.com>
Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>


As this is something I'd suggested we implement based on the work in
PaX/grsecurity, David took the first (and continuing) stab at
conversions, Hans did more, and Elena has been doing even more along
with the heavy-lifting of keeping the series organized. That way the
first SoB is still the author, the last SoB is still the email sender,
and everyone's name is mentioned.

Or just:


Inspired by atomic protections in PaX/grsecurity, based on work from
David Windsor, Hans Liljestrand, and myself.

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>


I'm not picky -- I just want to see the conversion to refcount_t
happen, and everyone in Elena's SoB list has done work on it...

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Borislav Petkov]

On Tue, Feb 21, 2017 at 12:45:30PM -0800, Kees Cook wrote:
> On Mon, Feb 20, 2017 at 3:17 AM, Borislav Petkov <bp@alien8.de> wrote:
> > On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote:
> >> refcount_t type and corresponding API should be
> >> used instead of atomic_t when the variable is used as
> >> a reference counter. This allows to avoid accidental
> >> refcounter overflows that might lead to use-after-free
> >> situations.
> >>
> >> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> >> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >> Signed-off-by: David Windsor <dwindsor@gmail.com>
> >
> > That SOB chain tells me that you wrote the patch and Hans, Kees and
> > David handled it in some way and the last one - David - is sending it to
> > me. It doesn't look like that though.
> 
> Perhaps the least inaccurate form of this might be:
> 
> 
> Inspired by atomic protections in PaX/grsecurity.
> 
> Suggested-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: David Windsor <dwindsor@gmail.com>
> Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> 
> 
> As this is something I'd suggested we implement based on the work in
> PaX/grsecurity, David took the first (and continuing) stab at
> conversions, Hans did more, and Elena has been doing even more along
> with the heavy-lifting of keeping the series organized. That way the
> first SoB is still the author, the last SoB is still the email sender,
> and everyone's name is mentioned.
> 
> Or just:
> 
> 
> Inspired by atomic protections in PaX/grsecurity, based on work from
> David Windsor, Hans Liljestrand, and myself.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> 
> 
> I'm not picky -- I just want to see the conversion to refcount_t

Me neither - both look good to me and actually explain what the SOB
chain was trying to say.

Thanks!

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 2/4] x86: convert threshold_bank.cpus from atomic_t to refcount_t

[Borislav Petkov]

On Tue, Feb 21, 2017 at 12:45:30PM -0800, Kees Cook wrote:
> On Mon, Feb 20, 2017 at 3:17 AM, Borislav Petkov <bp@alien8.de> wrote:
> > On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote:
> >> refcount_t type and corresponding API should be
> >> used instead of atomic_t when the variable is used as
> >> a reference counter. This allows to avoid accidental
> >> refcounter overflows that might lead to use-after-free
> >> situations.
> >>
> >> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> >> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >> Signed-off-by: David Windsor <dwindsor@gmail.com>
> >
> > That SOB chain tells me that you wrote the patch and Hans, Kees and
> > David handled it in some way and the last one - David - is sending it to
> > me. It doesn't look like that though.
> 
> Perhaps the least inaccurate form of this might be:
> 
> 
> Inspired by atomic protections in PaX/grsecurity.
> 
> Suggested-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: David Windsor <dwindsor@gmail.com>
> Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> 
> 
> As this is something I'd suggested we implement based on the work in
> PaX/grsecurity, David took the first (and continuing) stab at
> conversions, Hans did more, and Elena has been doing even more along
> with the heavy-lifting of keeping the series organized. That way the
> first SoB is still the author, the last SoB is still the email sender,
> and everyone's name is mentioned.
> 
> Or just:
> 
> 
> Inspired by atomic protections in PaX/grsecurity, based on work from
> David Windsor, Hans Liljestrand, and myself.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> 
> 
> I'm not picky -- I just want to see the conversion to refcount_t

Me neither - both look good to me and actually explain what the SOB
chain was trying to say.

Thanks!

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

RE: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[Reshetova, Elena]

> From: Elena Reshetova <elena.reshetova@intel.com>
> Date: Mon, 20 Feb 2017 13:06:20 +0200
> 
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Hi David, 

Would you be able to propagate this patch further or should I send it (with your acked-by) once more to specific list/maintainer for the inclusion?

Best Regards,
Elena

RE: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[Reshetova, Elena]

> From: Elena Reshetova <elena.reshetova@intel.com>
> Date: Mon, 20 Feb 2017 13:06:20 +0200
> 
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Hi David, 

Would you be able to propagate this patch further or should I send it (with your acked-by) once more to specific list/maintainer for the inclusion?

Best Regards,
Elena

Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[David Miller]

From: "Reshetova, Elena" <elena.reshetova@intel.com>
Date: Mon, 3 Apr 2017 07:28:01 +0000

> 
>> From: Elena Reshetova <elena.reshetova@intel.com>
>> Date: Mon, 20 Feb 2017 13:06:20 +0200
>> 
>> > refcount_t type and corresponding API should be
>> > used instead of atomic_t when the variable is used as
>> > a reference counter. This allows to avoid accidental
>> > refcounter overflows that might lead to use-after-free
>> > situations.
>> >
>> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
>> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > Signed-off-by: David Windsor <dwindsor@gmail.com>
>> 
>> Acked-by: David S. Miller <davem@davemloft.net>
> 
> Hi David, 
> 
> Would you be able to propagate this patch further or should I send
> it (with your acked-by) once more to specific list/maintainer for
> the inclusion?

I'm generally not happy with the refcount_t and the added overhead it
has compared to the existing atomic_t operations.

I know it is going to make a difference for networking.

I understand that this sparc case is a slow path, but I know that if
we just apply all of these refcount_t conversions, there will be no
work done to address the performance issues.

So I'm reluctant to ACK in any way these refcount_t conversions,
sorry.

Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[David Miller]

From: "Reshetova, Elena" <elena.reshetova@intel.com>
Date: Mon, 3 Apr 2017 07:28:01 +0000

> 
>> From: Elena Reshetova <elena.reshetova@intel.com>
>> Date: Mon, 20 Feb 2017 13:06:20 +0200
>> 
>> > refcount_t type and corresponding API should be
>> > used instead of atomic_t when the variable is used as
>> > a reference counter. This allows to avoid accidental
>> > refcounter overflows that might lead to use-after-free
>> > situations.
>> >
>> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
>> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > Signed-off-by: David Windsor <dwindsor@gmail.com>
>> 
>> Acked-by: David S. Miller <davem@davemloft.net>
> 
> Hi David, 
> 
> Would you be able to propagate this patch further or should I send
> it (with your acked-by) once more to specific list/maintainer for
> the inclusion?

I'm generally not happy with the refcount_t and the added overhead it
has compared to the existing atomic_t operations.

I know it is going to make a difference for networking.

I understand that this sparc case is a slow path, but I know that if
we just apply all of these refcount_t conversions, there will be no
work done to address the performance issues.

So I'm reluctant to ACK in any way these refcount_t conversions,
sorry.

RE: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[Reshetova, Elena]

> From: "Reshetova, Elena" <elena.reshetova@intel.com>
> Date: Mon, 3 Apr 2017 07:28:01 +0000
> 
> >
> >> From: Elena Reshetova <elena.reshetova@intel.com>
> >> Date: Mon, 20 Feb 2017 13:06:20 +0200
> >>
> >> > refcount_t type and corresponding API should be
> >> > used instead of atomic_t when the variable is used as
> >> > a reference counter. This allows to avoid accidental
> >> > refcounter overflows that might lead to use-after-free
> >> > situations.
> >> >
> >> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> >> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> >> > Signed-off-by: Kees Cook <keescook@chromium.org>
> >> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> >>
> >> Acked-by: David S. Miller <davem@davemloft.net>
> >
> > Hi David,
> >
> > Would you be able to propagate this patch further or should I send
> > it (with your acked-by) once more to specific list/maintainer for
> > the inclusion?
> 
> I'm generally not happy with the refcount_t and the added overhead it
> has compared to the existing atomic_t operations.
> 
> I know it is going to make a difference for networking.
> 
> I understand that this sparc case is a slow path, but I know that if
> we just apply all of these refcount_t conversions, there will be no
> work done to address the performance issues.

I think we will have to address the performance problems in places where we can see it matters. 
The problem is that so far noone told us how to measure in any reasonable way the overhead neither in networking, not in mm changes. 
If this change is a slow path, why would it matter for *this particular patch*?

Best Regards,
Elena.
 
> So I'm reluctant to ACK in any way these refcount_t conversions,
> sorry.

RE: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[Reshetova, Elena]

> From: "Reshetova, Elena" <elena.reshetova@intel.com>
> Date: Mon, 3 Apr 2017 07:28:01 +0000
> 
> >
> >> From: Elena Reshetova <elena.reshetova@intel.com>
> >> Date: Mon, 20 Feb 2017 13:06:20 +0200
> >>
> >> > refcount_t type and corresponding API should be
> >> > used instead of atomic_t when the variable is used as
> >> > a reference counter. This allows to avoid accidental
> >> > refcounter overflows that might lead to use-after-free
> >> > situations.
> >> >
> >> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> >> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> >> > Signed-off-by: Kees Cook <keescook@chromium.org>
> >> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> >>
> >> Acked-by: David S. Miller <davem@davemloft.net>
> >
> > Hi David,
> >
> > Would you be able to propagate this patch further or should I send
> > it (with your acked-by) once more to specific list/maintainer for
> > the inclusion?
> 
> I'm generally not happy with the refcount_t and the added overhead it
> has compared to the existing atomic_t operations.
> 
> I know it is going to make a difference for networking.
> 
> I understand that this sparc case is a slow path, but I know that if
> we just apply all of these refcount_t conversions, there will be no
> work done to address the performance issues.

I think we will have to address the performance problems in places where we can see it matters. 
The problem is that so far noone told us how to measure in any reasonable way the overhead neither in networking, not in mm changes. 
If this change is a slow path, why would it matter for *this particular patch*?

Best Regards,
Elena.
 
> So I'm reluctant to ACK in any way these refcount_t conversions,
> sorry.

Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[David Miller]

From: "Reshetova, Elena" <elena.reshetova@intel.com>
Date: Mon, 3 Apr 2017 16:06:44 +0000

>> From: "Reshetova, Elena" <elena.reshetova@intel.com>
>> Date: Mon, 3 Apr 2017 07:28:01 +0000
>> 
>> >
>> >> From: Elena Reshetova <elena.reshetova@intel.com>
>> >> Date: Mon, 20 Feb 2017 13:06:20 +0200
>> >>
>> >> > refcount_t type and corresponding API should be
>> >> > used instead of atomic_t when the variable is used as
>> >> > a reference counter. This allows to avoid accidental
>> >> > refcounter overflows that might lead to use-after-free
>> >> > situations.
>> >> >
>> >> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
>> >> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>> >> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> >> > Signed-off-by: David Windsor <dwindsor@gmail.com>
>> >>
>> >> Acked-by: David S. Miller <davem@davemloft.net>
>> >
>> > Hi David,
>> >
>> > Would you be able to propagate this patch further or should I send
>> > it (with your acked-by) once more to specific list/maintainer for
>> > the inclusion?
>> 
>> I'm generally not happy with the refcount_t and the added overhead it
>> has compared to the existing atomic_t operations.
>> 
>> I know it is going to make a difference for networking.
>> 
>> I understand that this sparc case is a slow path, but I know that if
>> we just apply all of these refcount_t conversions, there will be no
>> work done to address the performance issues.
> 
> I think we will have to address the performance problems in places where we can see it matters. 
> The problem is that so far noone told us how to measure in any reasonable way the overhead neither in networking, not in mm changes. 
> If this change is a slow path, why would it matter for *this particular patch*?

I think not having a way to avoid the functional call makes the facility
unusable as a core kernel facility.

You can't just say "oh well, just convert the slow paths, we'll solve the
fundamental performance issue later".  Sorry, that is not how we do things.

Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

[David Miller]

From: "Reshetova, Elena" <elena.reshetova@intel.com>
Date: Mon, 3 Apr 2017 16:06:44 +0000

>> From: "Reshetova, Elena" <elena.reshetova@intel.com>
>> Date: Mon, 3 Apr 2017 07:28:01 +0000
>> 
>> >
>> >> From: Elena Reshetova <elena.reshetova@intel.com>
>> >> Date: Mon, 20 Feb 2017 13:06:20 +0200
>> >>
>> >> > refcount_t type and corresponding API should be
>> >> > used instead of atomic_t when the variable is used as
>> >> > a reference counter. This allows to avoid accidental
>> >> > refcounter overflows that might lead to use-after-free
>> >> > situations.
>> >> >
>> >> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
>> >> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>> >> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> >> > Signed-off-by: David Windsor <dwindsor@gmail.com>
>> >>
>> >> Acked-by: David S. Miller <davem@davemloft.net>
>> >
>> > Hi David,
>> >
>> > Would you be able to propagate this patch further or should I send
>> > it (with your acked-by) once more to specific list/maintainer for
>> > the inclusion?
>> 
>> I'm generally not happy with the refcount_t and the added overhead it
>> has compared to the existing atomic_t operations.
>> 
>> I know it is going to make a difference for networking.
>> 
>> I understand that this sparc case is a slow path, but I know that if
>> we just apply all of these refcount_t conversions, there will be no
>> work done to address the performance issues.
> 
> I think we will have to address the performance problems in places where we can see it matters. 
> The problem is that so far noone told us how to measure in any reasonable way the overhead neither in networking, not in mm changes. 
> If this change is a slow path, why would it matter for *this particular patch*?

I think not having a way to avoid the functional call makes the facility
unusable as a core kernel facility.

You can't just say "oh well, just convert the slow paths, we'll solve the
fundamental performance issue later".  Sorry, that is not how we do things.