linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/19] Kernel subsystem refcounter conversions
@ 2017-02-20 10:18 Elena Reshetova
  2017-02-20 10:18 ` [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t Elena Reshetova
                   ` (18 more replies)
  0 siblings, 19 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, 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 kernel 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 (19):
  kernel: convert sighand_struct.count from atomic_t to refcount_t
  kernel: convert signal_struct.sigcnt from atomic_t to refcount_t
  kernel: convert user_struct.__count from atomic_t to refcount_t
  kernel: convert task_struct.usage from atomic_t to refcount_t
  kernel: convert task_struct.stack_refcount from atomic_t to refcount_t
  kernel: convert perf_event_context.refcount from atomic_t to
    refcount_t
  kernel: convert ring_buffer.refcount from atomic_t to refcount_t
  kernel: convert ring_buffer.aux_refcount from atomic_t to refcount_t
  kernel: convert uprobe.ref from atomic_t to refcount_t
  kernel: convert nsproxy.count from atomic_t to refcount_t
  kernel: convert cgroup_namespace.count from atomic_t to refcount_t
  kernel: convert css_set.refcount from atomic_t to refcount_t
  kernel: convert group_info.usage from atomic_t to refcount_t
  kernel: convert cred.usage from atomic_t to refcount_t
  kernel: convert audit_tree.count from atomic_t to refcount_t
  kernel: convert audit_watch.count from atomic_t to refcount_t
  kernel: convert numa_group.refcount from atomic_t to refcount_t
  kernel: convert futex_pi_state.refcount from atomic_t to refcount_t
  kernel: convert kcov.refcount from atomic_t to refcount_t

 fs/exec.c                       |  4 ++--
 include/linux/cgroup-defs.h     |  3 ++-
 include/linux/cgroup.h          |  7 ++++---
 include/linux/cred.h            | 13 ++++++------
 include/linux/init_task.h       |  7 ++++---
 include/linux/nsproxy.h         |  6 +++---
 include/linux/perf_event.h      |  3 ++-
 include/linux/sched.h           | 19 +++++++++--------
 kernel/audit_tree.c             |  8 +++----
 kernel/audit_watch.c            |  8 +++----
 kernel/cgroup/cgroup-internal.h | 10 ++++++---
 kernel/cgroup/cgroup-v1.c       |  4 ++--
 kernel/cgroup/cgroup.c          | 10 ++++-----
 kernel/cgroup/namespace.c       |  2 +-
 kernel/cred.c                   | 46 ++++++++++++++++++++---------------------
 kernel/events/core.c            | 18 ++++++++--------
 kernel/events/internal.h        |  5 +++--
 kernel/events/ring_buffer.c     |  8 +++----
 kernel/events/uprobes.c         |  8 +++----
 kernel/fork.c                   | 24 ++++++++++-----------
 kernel/futex.c                  | 15 +++++++-------
 kernel/groups.c                 |  2 +-
 kernel/kcov.c                   |  9 ++++----
 kernel/nsproxy.c                |  6 +++---
 kernel/sched/fair.c             |  8 +++----
 kernel/user.c                   |  8 +++----
 26 files changed, 137 insertions(+), 124 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 12:30   ` kbuild test robot
  2017-02-20 12:42   ` kbuild test robot
  2017-02-20 10:18 ` [PATCH 02/19] kernel: convert signal_struct.sigcnt " Elena Reshetova
                   ` (17 subsequent siblings)
  18 siblings, 2 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 fs/exec.c                 | 4 ++--
 include/linux/init_task.h | 2 +-
 include/linux/sched.h     | 3 ++-
 kernel/fork.c             | 8 ++++----
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 698a860..3f78ff2 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1174,7 +1174,7 @@ static int de_thread(struct task_struct *tsk)
 	flush_itimer_signals();
 #endif
 
-	if (atomic_read(&oldsighand->count) != 1) {
+	if (refcount_read(&oldsighand->count) != 1) {
 		struct sighand_struct *newsighand;
 		/*
 		 * This ->sighand is shared with the CLONE_SIGHAND
@@ -1184,7 +1184,7 @@ static int de_thread(struct task_struct *tsk)
 		if (!newsighand)
 			return -ENOMEM;
 
-		atomic_set(&newsighand->count, 1);
+		refcount_set(&newsighand->count, 1);
 		memcpy(newsighand->action, oldsighand->action,
 		       sizeof(newsighand->action));
 
diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 3a85d61..1f160b2 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -82,7 +82,7 @@ extern struct fs_struct init_fs;
 extern struct nsproxy init_nsproxy;
 
 #define INIT_SIGHAND(sighand) {						\
-	.count		= ATOMIC_INIT(1), 				\
+	.count		= REFCOUNT_INIT(1), 				\
 	.action		= { { { .sa_handler = SIG_DFL, } }, },		\
 	.siglock	= __SPIN_LOCK_UNLOCKED(sighand.siglock),	\
 	.signalfd_wqh	= __WAIT_QUEUE_HEAD_INITIALIZER(sighand.signalfd_wqh),	\
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 0f0d497..a0430aa 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -42,6 +42,7 @@ struct sched_param {
 #include <linux/seccomp.h>
 #include <linux/rcupdate.h>
 #include <linux/rculist.h>
+#include <linux/refcount.h>
 #include <linux/rtmutex.h>
 
 #include <linux/time.h>
@@ -527,7 +528,7 @@ static inline int get_dumpable(struct mm_struct *mm)
 #define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK)
 
 struct sighand_struct {
-	atomic_t		count;
+	refcount_t		count;
 	struct k_sigaction	action[_NSIG];
 	spinlock_t		siglock;
 	wait_queue_head_t	signalfd_wqh;
diff --git a/kernel/fork.c b/kernel/fork.c
index e96e256..9488f01 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1285,7 +1285,7 @@ static int copy_sighand(unsigned long clone_flags, struct task_struct *tsk)
 	struct sighand_struct *sig;
 
 	if (clone_flags & CLONE_SIGHAND) {
-		atomic_inc(&current->sighand->count);
+		refcount_inc(&current->sighand->count);
 		return 0;
 	}
 	sig = kmem_cache_alloc(sighand_cachep, GFP_KERNEL);
@@ -1293,14 +1293,14 @@ static int copy_sighand(unsigned long clone_flags, struct task_struct *tsk)
 	if (!sig)
 		return -ENOMEM;
 
-	atomic_set(&sig->count, 1);
+	refcount_set(&sig->count, 1);
 	memcpy(sig->action, current->sighand->action, sizeof(sig->action));
 	return 0;
 }
 
 void __cleanup_sighand(struct sighand_struct *sighand)
 {
-	if (atomic_dec_and_test(&sighand->count)) {
+	if (refcount_dec_and_test(&sighand->count)) {
 		signalfd_cleanup(sighand);
 		/*
 		 * sighand_cachep is SLAB_DESTROY_BY_RCU so we can free it
@@ -2170,7 +2170,7 @@ static int check_unshare_flags(unsigned long unshare_flags)
 			return -EINVAL;
 	}
 	if (unshare_flags & (CLONE_SIGHAND | CLONE_VM)) {
-		if (atomic_read(&current->sighand->count) > 1)
+		if (refcount_read(&current->sighand->count) > 1)
 			return -EINVAL;
 	}
 	if (unshare_flags & CLONE_VM) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 02/19] kernel: convert signal_struct.sigcnt from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
  2017-02-20 10:18 ` [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:18 ` [PATCH 03/19] kernel: convert user_struct.__count " Elena Reshetova
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/sched.h | 2 +-
 kernel/fork.c         | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a0430aa..6aad885 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -659,7 +659,7 @@ struct autogroup;
  * the locking of signal_struct.
  */
 struct signal_struct {
-	atomic_t		sigcnt;
+	refcount_t		sigcnt;
 	atomic_t		live;
 	int			nr_threads;
 	struct list_head	thread_head;
diff --git a/kernel/fork.c b/kernel/fork.c
index 9488f01..b432d1c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -376,7 +376,7 @@ static inline void free_signal_struct(struct signal_struct *sig)
 
 static inline void put_signal_struct(struct signal_struct *sig)
 {
-	if (atomic_dec_and_test(&sig->sigcnt))
+	if (refcount_dec_and_test(&sig->sigcnt))
 		free_signal_struct(sig);
 }
 
@@ -1347,7 +1347,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
 
 	sig->nr_threads = 1;
 	atomic_set(&sig->live, 1);
-	atomic_set(&sig->sigcnt, 1);
+	refcount_set(&sig->sigcnt, 1);
 
 	/* list_add(thread_node, thread_head) without INIT_LIST_HEAD() */
 	sig->thread_head = (struct list_head)LIST_HEAD_INIT(tsk->thread_node);
@@ -1826,7 +1826,7 @@ static __latent_entropy struct task_struct *copy_process(
 		} else {
 			current->signal->nr_threads++;
 			atomic_inc(&current->signal->live);
-			atomic_inc(&current->signal->sigcnt);
+			refcount_inc(&current->signal->sigcnt);
 			list_add_tail_rcu(&p->thread_group,
 					  &p->group_leader->thread_group);
 			list_add_tail_rcu(&p->thread_node,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 03/19] kernel: convert user_struct.__count from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
  2017-02-20 10:18 ` [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t Elena Reshetova
  2017-02-20 10:18 ` [PATCH 02/19] kernel: convert signal_struct.sigcnt " Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:18 ` [PATCH 04/19] kernel: convert task_struct.usage " Elena Reshetova
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/sched.h | 4 ++--
 kernel/user.c         | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6aad885..e90396f 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -844,7 +844,7 @@ static inline int signal_group_exit(const struct signal_struct *sig)
  * Some day this will be a full-fledged user tracking system..
  */
 struct user_struct {
-	atomic_t __count;	/* reference count */
+	refcount_t __count;	/* reference count */
 	atomic_t processes;	/* How many processes does this user have? */
 	atomic_t sigpending;	/* How many pending signals does this user have? */
 #ifdef CONFIG_FANOTIFY
@@ -2694,7 +2694,7 @@ extern struct task_struct *find_task_by_pid_ns(pid_t nr,
 extern struct user_struct * alloc_uid(kuid_t);
 static inline struct user_struct *get_uid(struct user_struct *u)
 {
-	atomic_inc(&u->__count);
+	refcount_inc(&u->__count);
 	return u;
 }
 extern void free_uid(struct user_struct *);
diff --git a/kernel/user.c b/kernel/user.c
index b069ccb..d9dff8e 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -89,7 +89,7 @@ static DEFINE_SPINLOCK(uidhash_lock);
 
 /* root_user.__count is 1, for init task cred */
 struct user_struct root_user = {
-	.__count	= ATOMIC_INIT(1),
+	.__count	= REFCOUNT_INIT(1),
 	.processes	= ATOMIC_INIT(1),
 	.sigpending	= ATOMIC_INIT(0),
 	.locked_shm     = 0,
@@ -115,7 +115,7 @@ static struct user_struct *uid_hash_find(kuid_t uid, struct hlist_head *hashent)
 
 	hlist_for_each_entry(user, hashent, uidhash_node) {
 		if (uid_eq(user->uid, uid)) {
-			atomic_inc(&user->__count);
+			refcount_inc(&user->__count);
 			return user;
 		}
 	}
@@ -162,7 +162,7 @@ void free_uid(struct user_struct *up)
 		return;
 
 	local_irq_save(flags);
-	if (atomic_dec_and_lock(&up->__count, &uidhash_lock))
+	if (refcount_dec_and_lock(&up->__count, &uidhash_lock))
 		free_user(up, flags);
 	else
 		local_irq_restore(flags);
@@ -183,7 +183,7 @@ struct user_struct *alloc_uid(kuid_t uid)
 			goto out_unlock;
 
 		new->uid = uid;
-		atomic_set(&new->__count, 1);
+		refcount_set(&new->__count, 1);
 
 		/*
 		 * Before adding this, check whether we raced
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 04/19] kernel: convert task_struct.usage from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (2 preceding siblings ...)
  2017-02-20 10:18 ` [PATCH 03/19] kernel: convert user_struct.__count " Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:18 ` [PATCH 05/19] kernel: convert task_struct.stack_refcount " Elena Reshetova
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/init_task.h | 2 +-
 include/linux/sched.h     | 6 +++---
 kernel/fork.c             | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 1f160b2..1c6df0b 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -219,7 +219,7 @@ extern struct task_group root_task_group;
 	INIT_TASK_TI(tsk)						\
 	.state		= 0,						\
 	.stack		= init_stack,					\
-	.usage		= ATOMIC_INIT(2),				\
+	.usage		= REFCOUNT_INIT(2),				\
 	.flags		= PF_KTHREAD,					\
 	.prio		= MAX_PRIO-20,					\
 	.static_prio	= MAX_PRIO-20,					\
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e90396f..d760ad6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1489,7 +1489,7 @@ struct task_struct {
 #endif
 	volatile long state;	/* -1 unrunnable, 0 runnable, >0 stopped */
 	void *stack;
-	atomic_t usage;
+	refcount_t usage;
 	unsigned int flags;	/* per process flags, defined below */
 	unsigned int ptrace;
 
@@ -2220,13 +2220,13 @@ static inline int is_global_init(struct task_struct *tsk)
 extern struct pid *cad_pid;
 
 extern void free_task(struct task_struct *tsk);
-#define get_task_struct(tsk) do { atomic_inc(&(tsk)->usage); } while(0)
+#define get_task_struct(tsk) do { refcount_inc(&(tsk)->usage); } while(0)
 
 extern void __put_task_struct(struct task_struct *t);
 
 static inline void put_task_struct(struct task_struct *t)
 {
-	if (atomic_dec_and_test(&t->usage))
+	if (refcount_dec_and_test(&t->usage))
 		__put_task_struct(t);
 }
 
diff --git a/kernel/fork.c b/kernel/fork.c
index b432d1c..b9b3296 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -383,7 +383,7 @@ static inline void put_signal_struct(struct signal_struct *sig)
 void __put_task_struct(struct task_struct *tsk)
 {
 	WARN_ON(!tsk->exit_state);
-	WARN_ON(atomic_read(&tsk->usage));
+	WARN_ON(refcount_read(&tsk->usage));
 	WARN_ON(tsk == current);
 
 	cgroup_free(tsk);
@@ -533,7 +533,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 	 * One for us, one for whoever does the "release_task()" (usually
 	 * parent)
 	 */
-	atomic_set(&tsk->usage, 2);
+	refcount_set(&tsk->usage, 2);
 #ifdef CONFIG_BLK_DEV_IO_TRACE
 	tsk->btrace_seq = 0;
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 05/19] kernel: convert task_struct.stack_refcount from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (3 preceding siblings ...)
  2017-02-20 10:18 ` [PATCH 04/19] kernel: convert task_struct.usage " Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:18 ` [PATCH 06/19] kernel: convert perf_event_context.refcount " Elena Reshetova
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/init_task.h | 3 ++-
 include/linux/sched.h     | 4 ++--
 kernel/fork.c             | 6 +++---
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 1c6df0b..c452c57 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -12,6 +12,7 @@
 #include <linux/securebits.h>
 #include <linux/seqlock.h>
 #include <linux/rbtree.h>
+#include <linux/refcount.h>
 #include <net/net_namespace.h>
 #include <linux/sched/rt.h>
 
@@ -205,7 +206,7 @@ extern struct task_group root_task_group;
 #ifdef CONFIG_THREAD_INFO_IN_TASK
 # define INIT_TASK_TI(tsk)			\
 	.thread_info = INIT_THREAD_INFO(tsk),	\
-	.stack_refcount = ATOMIC_INIT(1),
+	.stack_refcount = REFCOUNT_INIT(1),
 #else
 # define INIT_TASK_TI(tsk)
 #endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d760ad6..41a2e52 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1969,7 +1969,7 @@ struct task_struct {
 #endif
 #ifdef CONFIG_THREAD_INFO_IN_TASK
 	/* A live task holds one reference. */
-	atomic_t stack_refcount;
+	refcount_t stack_refcount;
 #endif
 /* CPU-specific state of this task */
 	struct thread_struct thread;
@@ -3260,7 +3260,7 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
 #ifdef CONFIG_THREAD_INFO_IN_TASK
 static inline void *try_get_task_stack(struct task_struct *tsk)
 {
-	return atomic_inc_not_zero(&tsk->stack_refcount) ?
+	return refcount_inc_not_zero(&tsk->stack_refcount) ?
 		task_stack_page(tsk) : NULL;
 }
 
diff --git a/kernel/fork.c b/kernel/fork.c
index b9b3296..31c887c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -331,7 +331,7 @@ static void release_task_stack(struct task_struct *tsk)
 #ifdef CONFIG_THREAD_INFO_IN_TASK
 void put_task_stack(struct task_struct *tsk)
 {
-	if (atomic_dec_and_test(&tsk->stack_refcount))
+	if (refcount_dec_and_test(&tsk->stack_refcount))
 		release_task_stack(tsk);
 }
 #endif
@@ -349,7 +349,7 @@ void free_task(struct task_struct *tsk)
 	 * If the task had a separate stack allocation, it should be gone
 	 * by now.
 	 */
-	WARN_ON_ONCE(atomic_read(&tsk->stack_refcount) != 0);
+	WARN_ON_ONCE(refcount_read(&tsk->stack_refcount) != 0);
 #endif
 	rt_mutex_debug_task_free(tsk);
 	ftrace_graph_exit_task(tsk);
@@ -504,7 +504,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 	tsk->stack_vm_area = stack_vm_area;
 #endif
 #ifdef CONFIG_THREAD_INFO_IN_TASK
-	atomic_set(&tsk->stack_refcount, 1);
+	refcount_set(&tsk->stack_refcount, 1);
 #endif
 
 	if (err)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 06/19] kernel: convert perf_event_context.refcount from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (4 preceding siblings ...)
  2017-02-20 10:18 ` [PATCH 05/19] kernel: convert task_struct.stack_refcount " Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:28   ` Peter Zijlstra
  2017-02-20 10:18 ` [PATCH 07/19] kernel: convert ring_buffer.refcount " Elena Reshetova
                   ` (12 subsequent siblings)
  18 siblings, 1 reply; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/perf_event.h |  3 ++-
 kernel/events/core.c       | 12 ++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 000fdb2..7b130fc 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -54,6 +54,7 @@ struct perf_guest_info_callbacks {
 #include <linux/perf_regs.h>
 #include <linux/workqueue.h>
 #include <linux/cgroup.h>
+#include <linux/refcount.h>
 #include <asm/local.h>
 
 struct perf_callchain_entry {
@@ -743,7 +744,7 @@ struct perf_event_context {
 	int				nr_stat;
 	int				nr_freq;
 	int				rotate_disable;
-	atomic_t			refcount;
+	refcount_t			refcount;
 	struct task_struct		*task;
 
 	/*
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4669f2c..07a778b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1108,7 +1108,7 @@ static void perf_event_ctx_deactivate(struct perf_event_context *ctx)
 
 static void get_ctx(struct perf_event_context *ctx)
 {
-	WARN_ON(!atomic_inc_not_zero(&ctx->refcount));
+	WARN_ON(!refcount_inc_not_zero(&ctx->refcount));
 }
 
 static void free_ctx(struct rcu_head *head)
@@ -1122,7 +1122,7 @@ static void free_ctx(struct rcu_head *head)
 
 static void put_ctx(struct perf_event_context *ctx)
 {
-	if (atomic_dec_and_test(&ctx->refcount)) {
+	if (refcount_dec_and_test(&ctx->refcount)) {
 		if (ctx->parent_ctx)
 			put_ctx(ctx->parent_ctx);
 		if (ctx->task && ctx->task != TASK_TOMBSTONE)
@@ -1200,7 +1200,7 @@ perf_event_ctx_lock_nested(struct perf_event *event, int nesting)
 again:
 	rcu_read_lock();
 	ctx = ACCESS_ONCE(event->ctx);
-	if (!atomic_inc_not_zero(&ctx->refcount)) {
+	if (!refcount_inc_not_zero(&ctx->refcount)) {
 		rcu_read_unlock();
 		goto again;
 	}
@@ -1328,7 +1328,7 @@ perf_lock_task_context(struct task_struct *task, int ctxn, unsigned long *flags)
 		}
 
 		if (ctx->task == TASK_TOMBSTONE ||
-		    !atomic_inc_not_zero(&ctx->refcount)) {
+		    !refcount_inc_not_zero(&ctx->refcount)) {
 			raw_spin_unlock(&ctx->lock);
 			ctx = NULL;
 		} else {
@@ -3744,7 +3744,7 @@ static void __perf_event_init_context(struct perf_event_context *ctx)
 	INIT_LIST_HEAD(&ctx->pinned_groups);
 	INIT_LIST_HEAD(&ctx->flexible_groups);
 	INIT_LIST_HEAD(&ctx->event_list);
-	atomic_set(&ctx->refcount, 1);
+	refcount_set(&ctx->refcount, 1);
 }
 
 static struct perf_event_context *
@@ -9630,7 +9630,7 @@ __perf_event_ctx_lock_double(struct perf_event *group_leader,
 again:
 	rcu_read_lock();
 	gctx = READ_ONCE(group_leader->ctx);
-	if (!atomic_inc_not_zero(&gctx->refcount)) {
+	if (!refcount_inc_not_zero(&gctx->refcount)) {
 		rcu_read_unlock();
 		goto again;
 	}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 07/19] kernel: convert ring_buffer.refcount from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (5 preceding siblings ...)
  2017-02-20 10:18 ` [PATCH 06/19] kernel: convert perf_event_context.refcount " Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:18 ` [PATCH 08/19] kernel: convert ring_buffer.aux_refcount " Elena Reshetova
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 kernel/events/core.c        | 4 ++--
 kernel/events/internal.h    | 3 ++-
 kernel/events/ring_buffer.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 07a778b..e1c337d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5039,7 +5039,7 @@ struct ring_buffer *ring_buffer_get(struct perf_event *event)
 	rcu_read_lock();
 	rb = rcu_dereference(event->rb);
 	if (rb) {
-		if (!atomic_inc_not_zero(&rb->refcount))
+		if (!refcount_inc_not_zero(&rb->refcount))
 			rb = NULL;
 	}
 	rcu_read_unlock();
@@ -5049,7 +5049,7 @@ struct ring_buffer *ring_buffer_get(struct perf_event *event)
 
 void ring_buffer_put(struct ring_buffer *rb)
 {
-	if (!atomic_dec_and_test(&rb->refcount))
+	if (!refcount_dec_and_test(&rb->refcount))
 		return;
 
 	WARN_ON_ONCE(!list_empty(&rb->event_list));
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index 486fd78..b8e6fdf 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -3,13 +3,14 @@
 
 #include <linux/hardirq.h>
 #include <linux/uaccess.h>
+#include <linux/refcount.h>
 
 /* Buffer handling */
 
 #define RING_BUFFER_WRITABLE		0x01
 
 struct ring_buffer {
-	atomic_t			refcount;
+	refcount_t			refcount;
 	struct rcu_head			rcu_head;
 #ifdef CONFIG_PERF_USE_VMALLOC
 	struct work_struct		work;
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 257fa46..8de1664 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -284,7 +284,7 @@ ring_buffer_init(struct ring_buffer *rb, long watermark, int flags)
 	else
 		rb->overwrite = 1;
 
-	atomic_set(&rb->refcount, 1);
+	refcount_set(&rb->refcount, 1);
 
 	INIT_LIST_HEAD(&rb->event_list);
 	spin_lock_init(&rb->event_lock);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 08/19] kernel: convert ring_buffer.aux_refcount from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (6 preceding siblings ...)
  2017-02-20 10:18 ` [PATCH 07/19] kernel: convert ring_buffer.refcount " Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:18 ` [PATCH 09/19] kernel: convert uprobe.ref " Elena Reshetova
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 kernel/events/core.c        | 2 +-
 kernel/events/internal.h    | 2 +-
 kernel/events/ring_buffer.c | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index e1c337d..32add6c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5114,7 +5114,7 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 
 		/* this has to be the last one */
 		rb_free_aux(rb);
-		WARN_ON_ONCE(atomic_read(&rb->aux_refcount));
+		WARN_ON_ONCE(refcount_read(&rb->aux_refcount));
 
 		mutex_unlock(&event->mmap_mutex);
 	}
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index b8e6fdf..fb55716 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -48,7 +48,7 @@ struct ring_buffer {
 	atomic_t			aux_mmap_count;
 	unsigned long			aux_mmap_locked;
 	void				(*free_aux)(void *);
-	atomic_t			aux_refcount;
+	refcount_t			aux_refcount;
 	void				**aux_pages;
 	void				*aux_priv;
 
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 8de1664..c501d4e 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -344,7 +344,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
 	if (!atomic_read(&rb->aux_mmap_count))
 		goto err;
 
-	if (!atomic_inc_not_zero(&rb->aux_refcount))
+	if (!refcount_inc_not_zero(&rb->aux_refcount))
 		goto err;
 
 	/*
@@ -636,7 +636,7 @@ int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event,
 	 * we keep a refcount here to make sure either of the two can
 	 * reference them safely.
 	 */
-	atomic_set(&rb->aux_refcount, 1);
+	refcount_set(&rb->aux_refcount, 1);
 
 	rb->aux_overwrite = overwrite;
 	rb->aux_watermark = watermark;
@@ -655,7 +655,7 @@ int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event,
 
 void rb_free_aux(struct ring_buffer *rb)
 {
-	if (atomic_dec_and_test(&rb->aux_refcount))
+	if (refcount_dec_and_test(&rb->aux_refcount))
 		__rb_free_aux(rb);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 09/19] kernel: convert uprobe.ref from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (7 preceding siblings ...)
  2017-02-20 10:18 ` [PATCH 08/19] kernel: convert ring_buffer.aux_refcount " Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:18 ` [PATCH 10/19] kernel: convert nsproxy.count " Elena Reshetova
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 kernel/events/uprobes.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index b62d6fe..a4b33f8 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -64,7 +64,7 @@ static struct percpu_rw_semaphore dup_mmap_sem;
 
 struct uprobe {
 	struct rb_node		rb_node;	/* node in the rb tree */
-	atomic_t		ref;
+	refcount_t		ref;
 	struct rw_semaphore	register_rwsem;
 	struct rw_semaphore	consumer_rwsem;
 	struct list_head	pending_list;
@@ -369,13 +369,13 @@ set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, unsigned long v
 
 static struct uprobe *get_uprobe(struct uprobe *uprobe)
 {
-	atomic_inc(&uprobe->ref);
+	refcount_inc(&uprobe->ref);
 	return uprobe;
 }
 
 static void put_uprobe(struct uprobe *uprobe)
 {
-	if (atomic_dec_and_test(&uprobe->ref))
+	if (refcount_dec_and_test(&uprobe->ref))
 		kfree(uprobe);
 }
 
@@ -457,7 +457,7 @@ static struct uprobe *__insert_uprobe(struct uprobe *uprobe)
 	rb_link_node(&uprobe->rb_node, parent, p);
 	rb_insert_color(&uprobe->rb_node, &uprobes_tree);
 	/* get access + creation ref */
-	atomic_set(&uprobe->ref, 2);
+	refcount_set(&uprobe->ref, 2);
 
 	return u;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 10/19] kernel: convert nsproxy.count from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (8 preceding siblings ...)
  2017-02-20 10:18 ` [PATCH 09/19] kernel: convert uprobe.ref " Elena Reshetova
@ 2017-02-20 10:18 ` Elena Reshetova
  2017-02-20 10:19 ` [PATCH 11/19] kernel: convert cgroup_namespace.count " Elena Reshetova
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/nsproxy.h | 6 +++---
 kernel/nsproxy.c        | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index ac0d65b..f862ba8 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -28,7 +28,7 @@ struct fs_struct;
  * nsproxy is copied.
  */
 struct nsproxy {
-	atomic_t count;
+	refcount_t count;
 	struct uts_namespace *uts_ns;
 	struct ipc_namespace *ipc_ns;
 	struct mnt_namespace *mnt_ns;
@@ -74,14 +74,14 @@ int __init nsproxy_cache_init(void);
 
 static inline void put_nsproxy(struct nsproxy *ns)
 {
-	if (atomic_dec_and_test(&ns->count)) {
+	if (refcount_dec_and_test(&ns->count)) {
 		free_nsproxy(ns);
 	}
 }
 
 static inline void get_nsproxy(struct nsproxy *ns)
 {
-	atomic_inc(&ns->count);
+	refcount_inc(&ns->count);
 }
 
 #endif
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index 782102e..435a0f9 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -30,7 +30,7 @@
 static struct kmem_cache *nsproxy_cachep;
 
 struct nsproxy init_nsproxy = {
-	.count			= ATOMIC_INIT(1),
+	.count			= REFCOUNT_INIT(1),
 	.uts_ns			= &init_uts_ns,
 #if defined(CONFIG_POSIX_MQUEUE) || defined(CONFIG_SYSVIPC)
 	.ipc_ns			= &init_ipc_ns,
@@ -51,7 +51,7 @@ static inline struct nsproxy *create_nsproxy(void)
 
 	nsproxy = kmem_cache_alloc(nsproxy_cachep, GFP_KERNEL);
 	if (nsproxy)
-		atomic_set(&nsproxy->count, 1);
+		refcount_set(&nsproxy->count, 1);
 	return nsproxy;
 }
 
@@ -224,7 +224,7 @@ void switch_task_namespaces(struct task_struct *p, struct nsproxy *new)
 	p->nsproxy = new;
 	task_unlock(p);
 
-	if (ns && atomic_dec_and_test(&ns->count))
+	if (ns && refcount_dec_and_test(&ns->count))
 		free_nsproxy(ns);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 11/19] kernel: convert cgroup_namespace.count from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (9 preceding siblings ...)
  2017-02-20 10:18 ` [PATCH 10/19] kernel: convert nsproxy.count " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  2017-03-06 19:55   ` Tejun Heo
  2017-02-20 10:19 ` [PATCH 12/19] kernel: convert css_set.refcount " Elena Reshetova
                   ` (7 subsequent siblings)
  18 siblings, 1 reply; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/cgroup.h    | 7 ++++---
 kernel/cgroup/cgroup.c    | 2 +-
 kernel/cgroup/namespace.c | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index f6b43fb..4412979 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -22,6 +22,7 @@
 #include <linux/ns_common.h>
 #include <linux/nsproxy.h>
 #include <linux/user_namespace.h>
+#include <linux/refcount.h>
 
 #include <linux/cgroup-defs.h>
 
@@ -640,7 +641,7 @@ static inline void cgroup_sk_free(struct sock_cgroup_data *skcd) {}
 #endif	/* CONFIG_CGROUP_DATA */
 
 struct cgroup_namespace {
-	atomic_t		count;
+	refcount_t		count;
 	struct ns_common	ns;
 	struct user_namespace	*user_ns;
 	struct ucounts		*ucounts;
@@ -675,12 +676,12 @@ copy_cgroup_ns(unsigned long flags, struct user_namespace *user_ns,
 static inline void get_cgroup_ns(struct cgroup_namespace *ns)
 {
 	if (ns)
-		atomic_inc(&ns->count);
+		refcount_inc(&ns->count);
 }
 
 static inline void put_cgroup_ns(struct cgroup_namespace *ns)
 {
-	if (ns && atomic_dec_and_test(&ns->count))
+	if (ns && refcount_dec_and_test(&ns->count))
 		free_cgroup_ns(ns);
 }
 
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index ffcb310..5b94c79 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -188,7 +188,7 @@ static u16 have_canfork_callback __read_mostly;
 
 /* cgroup namespace for init task */
 struct cgroup_namespace init_cgroup_ns = {
-	.count		= { .counter = 2, },
+	.count		= REFCOUNT_INIT(2),
 	.user_ns	= &init_user_ns,
 	.ns.ops		= &cgroupns_operations,
 	.ns.inum	= PROC_CGROUP_INIT_INO,
diff --git a/kernel/cgroup/namespace.c b/kernel/cgroup/namespace.c
index cff7ea6..86e9bbe 100644
--- a/kernel/cgroup/namespace.c
+++ b/kernel/cgroup/namespace.c
@@ -31,7 +31,7 @@ static struct cgroup_namespace *alloc_cgroup_ns(void)
 		kfree(new_ns);
 		return ERR_PTR(ret);
 	}
-	atomic_set(&new_ns->count, 1);
+	refcount_set(&new_ns->count, 1);
 	new_ns->ns.ops = &cgroupns_operations;
 	return new_ns;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 12/19] kernel: convert css_set.refcount from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (10 preceding siblings ...)
  2017-02-20 10:19 ` [PATCH 11/19] kernel: convert cgroup_namespace.count " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  2017-03-06 19:54   ` Tejun Heo
  2017-02-20 10:19 ` [PATCH 13/19] kernel: convert group_info.usage " Elena Reshetova
                   ` (6 subsequent siblings)
  18 siblings, 1 reply; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/cgroup-defs.h     |  3 ++-
 kernel/cgroup/cgroup-internal.h | 10 +++++++---
 kernel/cgroup/cgroup-v1.c       |  4 ++--
 kernel/cgroup/cgroup.c          |  8 ++++----
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 3c02404..e2f4b31 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -13,6 +13,7 @@
 #include <linux/wait.h>
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
+#include <linux/refcount.h>
 #include <linux/percpu-refcount.h>
 #include <linux/percpu-rwsem.h>
 #include <linux/workqueue.h>
@@ -156,7 +157,7 @@ struct css_set {
 	struct cgroup_subsys_state *subsys[CGROUP_SUBSYS_COUNT];
 
 	/* reference count */
-	atomic_t refcount;
+	refcount_t refcount;
 
 	/* the default cgroup associated with this css_set */
 	struct cgroup *dfl_cgrp;
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 9203bfb..22d87ca 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -5,6 +5,7 @@
 #include <linux/kernfs.h>
 #include <linux/workqueue.h>
 #include <linux/list.h>
+#include <linux/refcount.h>
 
 /*
  * A cgroup can be associated with multiple css_sets as different tasks may
@@ -134,10 +135,13 @@ static inline void put_css_set(struct css_set *cset)
 	 * can see it. Similar to atomic_dec_and_lock(), but for an
 	 * rwlock
 	 */
-	if (atomic_add_unless(&cset->refcount, -1, 1))
+	spin_lock_irqsave(&css_set_lock, flags);
+	if (refcount_read(&cset->refcount) != 1) {
+		WARN_ON(refcount_dec_and_test(&cset->refcount));
+		spin_unlock_irqrestore(&css_set_lock, flags);
 		return;
+	}
 
-	spin_lock_irqsave(&css_set_lock, flags);
 	put_css_set_locked(cset);
 	spin_unlock_irqrestore(&css_set_lock, flags);
 }
@@ -147,7 +151,7 @@ static inline void put_css_set(struct css_set *cset)
  */
 static inline void get_css_set(struct css_set *cset)
 {
-	atomic_inc(&cset->refcount);
+	refcount_inc(&cset->refcount);
 }
 
 bool cgroup_ssid_enabled(int ssid);
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index fc34bcf..9269179 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -343,7 +343,7 @@ static int cgroup_task_count(const struct cgroup *cgrp)
 
 	spin_lock_irq(&css_set_lock);
 	list_for_each_entry(link, &cgrp->cset_links, cset_link)
-		count += atomic_read(&link->cset->refcount);
+		count += refcount_read(&link->cset->refcount);
 	spin_unlock_irq(&css_set_lock);
 	return count;
 }
@@ -1283,7 +1283,7 @@ static u64 current_css_set_refcount_read(struct cgroup_subsys_state *css,
 	u64 count;
 
 	rcu_read_lock();
-	count = atomic_read(&task_css_set(current)->refcount);
+	count = refcount_read(&task_css_set(current)->refcount);
 	rcu_read_unlock();
 	return count;
 }
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 5b94c79..59aba84 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -553,7 +553,7 @@ EXPORT_SYMBOL_GPL(of_css);
  * haven't been created.
  */
 struct css_set init_css_set = {
-	.refcount		= ATOMIC_INIT(1),
+	.refcount		= REFCOUNT_INIT(1),
 	.tasks			= LIST_HEAD_INIT(init_css_set.tasks),
 	.mg_tasks		= LIST_HEAD_INIT(init_css_set.mg_tasks),
 	.task_iters		= LIST_HEAD_INIT(init_css_set.task_iters),
@@ -723,7 +723,7 @@ void put_css_set_locked(struct css_set *cset)
 
 	lockdep_assert_held(&css_set_lock);
 
-	if (!atomic_dec_and_test(&cset->refcount))
+	if (!refcount_dec_and_test(&cset->refcount))
 		return;
 
 	/* This css_set is dead. unlink it and release cgroup and css refs */
@@ -976,7 +976,7 @@ static struct css_set *find_css_set(struct css_set *old_cset,
 		return NULL;
 	}
 
-	atomic_set(&cset->refcount, 1);
+	refcount_set(&cset->refcount, 1);
 	INIT_LIST_HEAD(&cset->tasks);
 	INIT_LIST_HEAD(&cset->mg_tasks);
 	INIT_LIST_HEAD(&cset->task_iters);
@@ -5064,4 +5064,4 @@ int cgroup_bpf_update(struct cgroup *cgrp, struct bpf_prog *prog,
 	mutex_unlock(&cgroup_mutex);
 	return ret;
 }
-#endif /* CONFIG_CGROUP_BPF */
+#endif /* CONFIG_CGROUP_BPF */
\ No newline at end of file
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 13/19] kernel: convert group_info.usage from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (11 preceding siblings ...)
  2017-02-20 10:19 ` [PATCH 12/19] kernel: convert css_set.refcount " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  2017-02-20 10:19 ` [PATCH 14/19] kernel: convert cred.usage " Elena Reshetova
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/cred.h | 7 ++++---
 kernel/cred.c        | 2 +-
 kernel/groups.c      | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/cred.h b/include/linux/cred.h
index f0e70a1..c837f2d 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -17,6 +17,7 @@
 #include <linux/key.h>
 #include <linux/selinux.h>
 #include <linux/atomic.h>
+#include <linux/refcount.h>
 #include <linux/uidgid.h>
 
 struct user_struct;
@@ -27,7 +28,7 @@ struct inode;
  * COW Supplementary groups list
  */
 struct group_info {
-	atomic_t	usage;
+	refcount_t	usage;
 	int		ngroups;
 	kgid_t		gid[0];
 };
@@ -43,7 +44,7 @@ struct group_info {
  */
 static inline struct group_info *get_group_info(struct group_info *gi)
 {
-	atomic_inc(&gi->usage);
+	refcount_inc(&gi->usage);
 	return gi;
 }
 
@@ -53,7 +54,7 @@ static inline struct group_info *get_group_info(struct group_info *gi)
  */
 #define put_group_info(group_info)			\
 do {							\
-	if (atomic_dec_and_test(&(group_info)->usage))	\
+	if (refcount_dec_and_test(&(group_info)->usage))	\
 		groups_free(group_info);		\
 } while (0)
 
diff --git a/kernel/cred.c b/kernel/cred.c
index 5f264fb..3afeea6 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -35,7 +35,7 @@ do {									\
 static struct kmem_cache *cred_jar;
 
 /* init to 2 - one for init_task, one to ensure it is never freed */
-struct group_info init_groups = { .usage = ATOMIC_INIT(2) };
+struct group_info init_groups = { .usage = REFCOUNT_INIT(2) };
 
 /*
  * The initial credentials for the initial task
diff --git a/kernel/groups.c b/kernel/groups.c
index 8dd7a61..455267f 100644
--- a/kernel/groups.c
+++ b/kernel/groups.c
@@ -22,7 +22,7 @@ struct group_info *groups_alloc(int gidsetsize)
 	if (!gi)
 		return NULL;
 
-	atomic_set(&gi->usage, 1);
+	refcount_set(&gi->usage, 1);
 	gi->ngroups = gidsetsize;
 	return gi;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 14/19] kernel: convert cred.usage from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (12 preceding siblings ...)
  2017-02-20 10:19 ` [PATCH 13/19] kernel: convert group_info.usage " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  2017-02-20 10:19 ` [PATCH 15/19] kernel: convert audit_tree.count " Elena Reshetova
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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/cred.h |  6 +++---
 kernel/cred.c        | 44 ++++++++++++++++++++++----------------------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/linux/cred.h b/include/linux/cred.h
index c837f2d..25fdc87 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -108,7 +108,7 @@ extern bool may_setgroups(void);
  * same context as task->real_cred.
  */
 struct cred {
-	atomic_t	usage;
+	refcount_t	usage;
 #ifdef CONFIG_DEBUG_CREDENTIALS
 	atomic_t	subscribers;	/* number of processes subscribed */
 	void		*put_addr;
@@ -221,7 +221,7 @@ static inline bool cap_ambient_invariant_ok(const struct cred *cred)
  */
 static inline struct cred *get_new_cred(struct cred *cred)
 {
-	atomic_inc(&cred->usage);
+	refcount_inc(&cred->usage);
 	return cred;
 }
 
@@ -261,7 +261,7 @@ static inline void put_cred(const struct cred *_cred)
 	struct cred *cred = (struct cred *) _cred;
 
 	validate_creds(cred);
-	if (atomic_dec_and_test(&(cred)->usage))
+	if (refcount_dec_and_test(&(cred)->usage))
 		__put_cred(cred);
 }
 
diff --git a/kernel/cred.c b/kernel/cred.c
index 3afeea6..31ebce0 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -41,7 +41,7 @@ struct group_info init_groups = { .usage = REFCOUNT_INIT(2) };
  * The initial credentials for the initial task
  */
 struct cred init_cred = {
-	.usage			= ATOMIC_INIT(4),
+	.usage			= REFCOUNT_INIT(4),
 #ifdef CONFIG_DEBUG_CREDENTIALS
 	.subscribers		= ATOMIC_INIT(2),
 	.magic			= CRED_MAGIC,
@@ -100,17 +100,17 @@ static void put_cred_rcu(struct rcu_head *rcu)
 
 #ifdef CONFIG_DEBUG_CREDENTIALS
 	if (cred->magic != CRED_MAGIC_DEAD ||
-	    atomic_read(&cred->usage) != 0 ||
+	    refcount_read(&cred->usage) != 0 ||
 	    read_cred_subscribers(cred) != 0)
 		panic("CRED: put_cred_rcu() sees %p with"
 		      " mag %x, put %p, usage %d, subscr %d\n",
 		      cred, cred->magic, cred->put_addr,
-		      atomic_read(&cred->usage),
+		      refcount_read(&cred->usage),
 		      read_cred_subscribers(cred));
 #else
-	if (atomic_read(&cred->usage) != 0)
+	if (refcount_read(&cred->usage) != 0)
 		panic("CRED: put_cred_rcu() sees %p with usage %d\n",
-		      cred, atomic_read(&cred->usage));
+		      cred, refcount_read(&cred->usage));
 #endif
 
 	security_cred_free(cred);
@@ -134,10 +134,10 @@ static void put_cred_rcu(struct rcu_head *rcu)
 void __put_cred(struct cred *cred)
 {
 	kdebug("__put_cred(%p{%d,%d})", cred,
-	       atomic_read(&cred->usage),
+	       refcount_read(&cred->usage),
 	       read_cred_subscribers(cred));
 
-	BUG_ON(atomic_read(&cred->usage) != 0);
+	BUG_ON(refcount_read(&cred->usage) != 0);
 #ifdef CONFIG_DEBUG_CREDENTIALS
 	BUG_ON(read_cred_subscribers(cred) != 0);
 	cred->magic = CRED_MAGIC_DEAD;
@@ -158,7 +158,7 @@ void exit_creds(struct task_struct *tsk)
 	struct cred *cred;
 
 	kdebug("exit_creds(%u,%p,%p,{%d,%d})", tsk->pid, tsk->real_cred, tsk->cred,
-	       atomic_read(&tsk->cred->usage),
+	       refcount_read(&tsk->cred->usage),
 	       read_cred_subscribers(tsk->cred));
 
 	cred = (struct cred *) tsk->real_cred;
@@ -193,7 +193,7 @@ const struct cred *get_task_cred(struct task_struct *task)
 	do {
 		cred = __task_cred((task));
 		BUG_ON(!cred);
-	} while (!atomic_inc_not_zero(&((struct cred *)cred)->usage));
+	} while (!refcount_inc_not_zero(&((struct cred *)cred)->usage));
 
 	rcu_read_unlock();
 	return cred;
@@ -211,7 +211,7 @@ struct cred *cred_alloc_blank(void)
 	if (!new)
 		return NULL;
 
-	atomic_set(&new->usage, 1);
+	refcount_set(&new->usage, 1);
 #ifdef CONFIG_DEBUG_CREDENTIALS
 	new->magic = CRED_MAGIC;
 #endif
@@ -257,7 +257,7 @@ struct cred *prepare_creds(void)
 	old = task->cred;
 	memcpy(new, old, sizeof(struct cred));
 
-	atomic_set(&new->usage, 1);
+	refcount_set(&new->usage, 1);
 	set_cred_subscribers(new, 0);
 	get_group_info(new->group_info);
 	get_uid(new->user);
@@ -334,7 +334,7 @@ int copy_creds(struct task_struct *p, unsigned long clone_flags)
 		get_cred(p->cred);
 		alter_cred_subscribers(p->cred, 2);
 		kdebug("share_creds(%p{%d,%d})",
-		       p->cred, atomic_read(&p->cred->usage),
+		       p->cred, refcount_read(&p->cred->usage),
 		       read_cred_subscribers(p->cred));
 		atomic_inc(&p->cred->user->processes);
 		return 0;
@@ -425,7 +425,7 @@ int commit_creds(struct cred *new)
 	const struct cred *old = task->real_cred;
 
 	kdebug("commit_creds(%p{%d,%d})", new,
-	       atomic_read(&new->usage),
+	       refcount_read(&new->usage),
 	       read_cred_subscribers(new));
 
 	BUG_ON(task->cred != old);
@@ -434,7 +434,7 @@ int commit_creds(struct cred *new)
 	validate_creds(old);
 	validate_creds(new);
 #endif
-	BUG_ON(atomic_read(&new->usage) < 1);
+	BUG_ON(refcount_read(&new->usage) < 1);
 
 	get_cred(new); /* we will require a ref for the subj creds too */
 
@@ -499,13 +499,13 @@ EXPORT_SYMBOL(commit_creds);
 void abort_creds(struct cred *new)
 {
 	kdebug("abort_creds(%p{%d,%d})", new,
-	       atomic_read(&new->usage),
+	       refcount_read(&new->usage),
 	       read_cred_subscribers(new));
 
 #ifdef CONFIG_DEBUG_CREDENTIALS
 	BUG_ON(read_cred_subscribers(new) != 0);
 #endif
-	BUG_ON(atomic_read(&new->usage) < 1);
+	BUG_ON(refcount_read(&new->usage) < 1);
 	put_cred(new);
 }
 EXPORT_SYMBOL(abort_creds);
@@ -522,7 +522,7 @@ const struct cred *override_creds(const struct cred *new)
 	const struct cred *old = current->cred;
 
 	kdebug("override_creds(%p{%d,%d})", new,
-	       atomic_read(&new->usage),
+	       refcount_read(&new->usage),
 	       read_cred_subscribers(new));
 
 	validate_creds(old);
@@ -533,7 +533,7 @@ const struct cred *override_creds(const struct cred *new)
 	alter_cred_subscribers(old, -1);
 
 	kdebug("override_creds() = %p{%d,%d}", old,
-	       atomic_read(&old->usage),
+	       refcount_read(&old->usage),
 	       read_cred_subscribers(old));
 	return old;
 }
@@ -551,7 +551,7 @@ void revert_creds(const struct cred *old)
 	const struct cred *override = current->cred;
 
 	kdebug("revert_creds(%p{%d,%d})", old,
-	       atomic_read(&old->usage),
+	       refcount_read(&old->usage),
 	       read_cred_subscribers(old));
 
 	validate_creds(old);
@@ -610,7 +610,7 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon)
 	validate_creds(old);
 
 	*new = *old;
-	atomic_set(&new->usage, 1);
+	refcount_set(&new->usage, 1);
 	set_cred_subscribers(new, 0);
 	get_uid(new->user);
 	get_user_ns(new->user_ns);
@@ -734,7 +734,7 @@ static void dump_invalid_creds(const struct cred *cred, const char *label,
 	printk(KERN_ERR "CRED: ->magic=%x, put_addr=%p\n",
 	       cred->magic, cred->put_addr);
 	printk(KERN_ERR "CRED: ->usage=%d, subscr=%d\n",
-	       atomic_read(&cred->usage),
+	       refcount_read(&cred->usage),
 	       read_cred_subscribers(cred));
 	printk(KERN_ERR "CRED: ->*uid = { %d,%d,%d,%d }\n",
 		from_kuid_munged(&init_user_ns, cred->uid),
@@ -808,7 +808,7 @@ void validate_creds_for_do_exit(struct task_struct *tsk)
 {
 	kdebug("validate_creds_for_do_exit(%p,%p{%d,%d})",
 	       tsk->real_cred, tsk->cred,
-	       atomic_read(&tsk->cred->usage),
+	       refcount_read(&tsk->cred->usage),
 	       read_cred_subscribers(tsk->cred));
 
 	__validate_process_creds(tsk, __FILE__, __LINE__);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (13 preceding siblings ...)
  2017-02-20 10:19 ` [PATCH 14/19] kernel: convert cred.usage " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  2017-02-20 22:07   ` Paul Moore
  2017-02-20 10:19 ` [PATCH 16/19] kernel: convert audit_watch.count " Elena Reshetova
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 kernel/audit_tree.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 7b44195..7ed617b 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -9,7 +9,7 @@ struct audit_tree;
 struct audit_chunk;
 
 struct audit_tree {
-	atomic_t count;
+	refcount_t count;
 	int goner;
 	struct audit_chunk *root;
 	struct list_head chunks;
@@ -77,7 +77,7 @@ static struct audit_tree *alloc_tree(const char *s)
 
 	tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
 	if (tree) {
-		atomic_set(&tree->count, 1);
+		refcount_set(&tree->count, 1);
 		tree->goner = 0;
 		INIT_LIST_HEAD(&tree->chunks);
 		INIT_LIST_HEAD(&tree->rules);
@@ -91,12 +91,12 @@ static struct audit_tree *alloc_tree(const char *s)
 
 static inline void get_tree(struct audit_tree *tree)
 {
-	atomic_inc(&tree->count);
+	refcount_inc(&tree->count);
 }
 
 static inline void put_tree(struct audit_tree *tree)
 {
-	if (atomic_dec_and_test(&tree->count))
+	if (refcount_dec_and_test(&tree->count))
 		kfree_rcu(tree, head);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 16/19] kernel: convert audit_watch.count from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (14 preceding siblings ...)
  2017-02-20 10:19 ` [PATCH 15/19] kernel: convert audit_tree.count " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  2017-02-20 10:19 ` [PATCH 17/19] kernel: convert numa_group.refcount " Elena Reshetova
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 kernel/audit_watch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index f79e465..8ca9e6c 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -46,7 +46,7 @@
  */
 
 struct audit_watch {
-	atomic_t		count;	/* reference count */
+	refcount_t		count;	/* reference count */
 	dev_t			dev;	/* associated superblock device */
 	char			*path;	/* insertion path */
 	unsigned long		ino;	/* associated inode number */
@@ -111,12 +111,12 @@ static inline struct audit_parent *audit_find_parent(struct inode *inode)
 
 void audit_get_watch(struct audit_watch *watch)
 {
-	atomic_inc(&watch->count);
+	refcount_inc(&watch->count);
 }
 
 void audit_put_watch(struct audit_watch *watch)
 {
-	if (atomic_dec_and_test(&watch->count)) {
+	if (refcount_dec_and_test(&watch->count)) {
 		WARN_ON(watch->parent);
 		WARN_ON(!list_empty(&watch->rules));
 		kfree(watch->path);
@@ -178,7 +178,7 @@ static struct audit_watch *audit_init_watch(char *path)
 		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&watch->rules);
-	atomic_set(&watch->count, 1);
+	refcount_set(&watch->count, 1);
 	watch->path = path;
 	watch->dev = AUDIT_DEV_UNSET;
 	watch->ino = AUDIT_INO_UNSET;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 17/19] kernel: convert numa_group.refcount from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (15 preceding siblings ...)
  2017-02-20 10:19 ` [PATCH 16/19] kernel: convert audit_watch.count " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  2017-02-20 10:19 ` [PATCH 18/19] kernel: convert futex_pi_state.refcount " Elena Reshetova
  2017-02-20 10:19 ` [PATCH 19/19] kernel: convert kcov.refcount " Elena Reshetova
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 kernel/sched/fair.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 274c747..69405a0 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1133,7 +1133,7 @@ static void account_numa_dequeue(struct rq *rq, struct task_struct *p)
 }
 
 struct numa_group {
-	atomic_t refcount;
+	refcount_t refcount;
 
 	spinlock_t lock; /* nr_tasks, tasks */
 	int nr_tasks;
@@ -2181,12 +2181,12 @@ static void task_numa_placement(struct task_struct *p)
 
 static inline int get_numa_group(struct numa_group *grp)
 {
-	return atomic_inc_not_zero(&grp->refcount);
+	return refcount_inc_not_zero(&grp->refcount);
 }
 
 static inline void put_numa_group(struct numa_group *grp)
 {
-	if (atomic_dec_and_test(&grp->refcount))
+	if (refcount_dec_and_test(&grp->refcount))
 		kfree_rcu(grp, rcu);
 }
 
@@ -2207,7 +2207,7 @@ static void task_numa_group(struct task_struct *p, int cpupid, int flags,
 		if (!grp)
 			return;
 
-		atomic_set(&grp->refcount, 1);
+		refcount_set(&grp->refcount, 1);
 		grp->active_nodes = 1;
 		grp->max_faults_cpu = 0;
 		spin_lock_init(&grp->lock);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 18/19] kernel: convert futex_pi_state.refcount from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (16 preceding siblings ...)
  2017-02-20 10:19 ` [PATCH 17/19] kernel: convert numa_group.refcount " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  2017-02-20 10:19 ` [PATCH 19/19] kernel: convert kcov.refcount " Elena Reshetova
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 kernel/futex.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index b687cb2..4ef9c10 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -65,6 +65,7 @@
 #include <linux/freezer.h>
 #include <linux/bootmem.h>
 #include <linux/fault-inject.h>
+#include <linux/refcount.h>
 
 #include <asm/futex.h>
 
@@ -207,7 +208,7 @@ struct futex_pi_state {
 	struct rt_mutex pi_mutex;
 
 	struct task_struct *owner;
-	atomic_t refcount;
+	refcount_t refcount;
 
 	union futex_key key;
 };
@@ -792,7 +793,7 @@ static int refill_pi_state_cache(void)
 	INIT_LIST_HEAD(&pi_state->list);
 	/* pi_mutex gets initialized later */
 	pi_state->owner = NULL;
-	atomic_set(&pi_state->refcount, 1);
+	refcount_set(&pi_state->refcount, 1);
 	pi_state->key = FUTEX_KEY_INIT;
 
 	current->pi_state_cache = pi_state;
@@ -821,7 +822,7 @@ static void put_pi_state(struct futex_pi_state *pi_state)
 	if (!pi_state)
 		return;
 
-	if (!atomic_dec_and_test(&pi_state->refcount))
+	if (!refcount_dec_and_test(&pi_state->refcount))
 		return;
 
 	/*
@@ -845,7 +846,7 @@ static void put_pi_state(struct futex_pi_state *pi_state)
 		 * refcount is at 0 - put it back to 1.
 		 */
 		pi_state->owner = NULL;
-		atomic_set(&pi_state->refcount, 1);
+		refcount_set(&pi_state->refcount, 1);
 		current->pi_state_cache = pi_state;
 	}
 }
@@ -989,7 +990,7 @@ static int attach_to_pi_state(u32 uval, struct futex_pi_state *pi_state,
 	if (unlikely(!pi_state))
 		return -EINVAL;
 
-	WARN_ON(!atomic_read(&pi_state->refcount));
+	WARN_ON(!refcount_read(&pi_state->refcount));
 
 	/*
 	 * Handle the owner died case:
@@ -1040,7 +1041,7 @@ static int attach_to_pi_state(u32 uval, struct futex_pi_state *pi_state,
 	if (pid != task_pid_vnr(pi_state->owner))
 		return -EINVAL;
 out_state:
-	atomic_inc(&pi_state->refcount);
+	refcount_inc(&pi_state->refcount);
 	*ps = pi_state;
 	return 0;
 }
@@ -1907,7 +1908,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,
 			 * refcount on the pi_state and store the pointer in
 			 * the futex_q object of the waiter.
 			 */
-			atomic_inc(&pi_state->refcount);
+			refcount_inc(&pi_state->refcount);
 			this->pi_state = pi_state;
 			ret = rt_mutex_start_proxy_lock(&pi_state->pi_mutex,
 							this->rt_waiter,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 19/19] kernel: convert kcov.refcount from atomic_t to refcount_t
  2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
                   ` (17 preceding siblings ...)
  2017-02-20 10:19 ` [PATCH 18/19] kernel: convert futex_pi_state.refcount " Elena Reshetova
@ 2017-02-20 10:19 ` Elena Reshetova
  18 siblings, 0 replies; 36+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: cgroups, linux-audit, linux-fsdevel, peterz, gregkh, viro, tj,
	mingo, hannes, lizefan, acme, alexander.shishkin, paul, eparis,
	akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook,
	David Windsor

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>
---
 kernel/kcov.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/kcov.c b/kernel/kcov.c
index 85e5546..b8506c3 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -19,6 +19,7 @@
 #include <linux/debugfs.h>
 #include <linux/uaccess.h>
 #include <linux/kcov.h>
+#include <linux/refcount.h>
 #include <asm/setup.h>
 
 /*
@@ -35,7 +36,7 @@ struct kcov {
 	 *  - opened file descriptor
 	 *  - task with enabled coverage (we can't unwire it from another task)
 	 */
-	atomic_t		refcount;
+	refcount_t		refcount;
 	/* The lock protects mode, size, area and t. */
 	spinlock_t		lock;
 	enum kcov_mode		mode;
@@ -101,12 +102,12 @@ EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
 
 static void kcov_get(struct kcov *kcov)
 {
-	atomic_inc(&kcov->refcount);
+	refcount_inc(&kcov->refcount);
 }
 
 static void kcov_put(struct kcov *kcov)
 {
-	if (atomic_dec_and_test(&kcov->refcount)) {
+	if (refcount_dec_and_test(&kcov->refcount)) {
 		vfree(kcov->area);
 		kfree(kcov);
 	}
@@ -182,7 +183,7 @@ static int kcov_open(struct inode *inode, struct file *filep)
 	kcov = kzalloc(sizeof(*kcov), GFP_KERNEL);
 	if (!kcov)
 		return -ENOMEM;
-	atomic_set(&kcov->refcount, 1);
+	refcount_set(&kcov->refcount, 1);
 	spin_lock_init(&kcov->lock);
 	filep->private_data = kcov;
 	return nonseekable_open(inode, filep);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/19] kernel: convert perf_event_context.refcount from atomic_t to refcount_t
  2017-02-20 10:18 ` [PATCH 06/19] kernel: convert perf_event_context.refcount " Elena Reshetova
@ 2017-02-20 10:28   ` Peter Zijlstra
  2017-02-20 12:14     ` Reshetova, Elena
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Zijlstra @ 2017-02-20 10:28 UTC (permalink / raw)
  To: Elena Reshetova
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, gregkh, viro,
	tj, mingo, hannes, lizefan, acme, alexander.shishkin, paul,
	eparis, akpm, arnd, luto, Hans Liljestrand, Kees Cook,
	David Windsor

On Mon, Feb 20, 2017 at 12:18:55PM +0200, Elena Reshetova wrote:
> +++ b/kernel/events/core.c
> @@ -1108,7 +1108,7 @@ static void perf_event_ctx_deactivate(struct perf_event_context *ctx)
>  
>  static void get_ctx(struct perf_event_context *ctx)
>  {
> -	WARN_ON(!atomic_inc_not_zero(&ctx->refcount));
> +	WARN_ON(!refcount_inc_not_zero(&ctx->refcount));
>  }

You can change that to refcount_inc(), as that has the exact same
semantics.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH 06/19] kernel: convert perf_event_context.refcount from atomic_t to refcount_t
  2017-02-20 10:28   ` Peter Zijlstra
@ 2017-02-20 12:14     ` Reshetova, Elena
  0 siblings, 0 replies; 36+ messages in thread
From: Reshetova, Elena @ 2017-02-20 12:14 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, gregkh, viro,
	tj, mingo, hannes, lizefan, acme, alexander.shishkin, paul,
	eparis, akpm, arnd, luto, Hans Liljestrand, Kees Cook,
	David Windsor


> On Mon, Feb 20, 2017 at 12:18:55PM +0200, Elena Reshetova wrote:
> > +++ b/kernel/events/core.c
> > @@ -1108,7 +1108,7 @@ static void perf_event_ctx_deactivate(struct
> perf_event_context *ctx)
> >
> >  static void get_ctx(struct perf_event_context *ctx)
> >  {
> > -	WARN_ON(!atomic_inc_not_zero(&ctx->refcount));
> > +	WARN_ON(!refcount_inc_not_zero(&ctx->refcount));
> >  }
> 
> You can change that to refcount_inc(), as that has the exact same
> semantics.

True, will fix. Thanks!

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t
  2017-02-20 10:18 ` [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t Elena Reshetova
@ 2017-02-20 12:30   ` kbuild test robot
  2017-02-20 12:42   ` kbuild test robot
  1 sibling, 0 replies; 36+ messages in thread
From: kbuild test robot @ 2017-02-20 12:30 UTC (permalink / raw)
  To: Elena Reshetova
  Cc: kbuild-all, linux-kernel, cgroups, linux-audit, linux-fsdevel,
	peterz, gregkh, viro, tj, mingo, hannes, lizefan, acme,
	alexander.shishkin, paul, eparis, akpm, arnd, luto,
	Elena Reshetova, Hans Liljestrand, Kees Cook, David Windsor

[-- Attachment #1: Type: text/plain, Size: 6955 bytes --]

Hi Elena,

[auto build test WARNING on next-20170220]
[cannot apply to linus/master linux/master tip/perf/core v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Elena-Reshetova/kernel-convert-sighand_struct-count-from-atomic_t-to-refcount_t/20170220-183434
config: blackfin-TCM-BF537_defconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=blackfin 

All warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:4:0,
                    from arch/blackfin/include/asm/bug.h:71,
                    from include/linux/bug.h:4,
                    from include/linux/mmdebug.h:4,
                    from include/linux/mm.h:8,
                    from fs/proc/task_nommu.c:2:
   fs/proc/task_nommu.c: In function 'task_mem':
   include/asm-generic/atomic.h:177:37: error: 'refcount_t {aka struct refcount_struct}' has no member named 'counter'
    #define atomic_read(v) READ_ONCE((v)->counter)
                                        ^
   include/linux/compiler.h:316:17: note: in definition of macro '__READ_ONCE'
     union { typeof(x) __val; char __c[1]; } __u;   \
                    ^
   include/asm-generic/atomic.h:177:24: note: in expansion of macro 'READ_ONCE'
    #define atomic_read(v) READ_ONCE((v)->counter)
                           ^~~~~~~~~
>> fs/proc/task_nommu.c:64:26: note: in expansion of macro 'atomic_read'
     if (current->sighand && atomic_read(&current->sighand->count) > 1)
                             ^~~~~~~~~~~
   include/asm-generic/atomic.h:177:37: error: 'refcount_t {aka struct refcount_struct}' has no member named 'counter'
    #define atomic_read(v) READ_ONCE((v)->counter)
                                        ^
   include/linux/compiler.h:318:22: note: in definition of macro '__READ_ONCE'
      __read_once_size(&(x), __u.__c, sizeof(x));  \
                         ^
   include/asm-generic/atomic.h:177:24: note: in expansion of macro 'READ_ONCE'
    #define atomic_read(v) READ_ONCE((v)->counter)
                           ^~~~~~~~~
>> fs/proc/task_nommu.c:64:26: note: in expansion of macro 'atomic_read'
     if (current->sighand && atomic_read(&current->sighand->count) > 1)
                             ^~~~~~~~~~~
   include/asm-generic/atomic.h:177:37: error: 'refcount_t {aka struct refcount_struct}' has no member named 'counter'
    #define atomic_read(v) READ_ONCE((v)->counter)
                                        ^
   include/linux/compiler.h:318:42: note: in definition of macro '__READ_ONCE'
      __read_once_size(&(x), __u.__c, sizeof(x));  \
                                             ^
   include/asm-generic/atomic.h:177:24: note: in expansion of macro 'READ_ONCE'
    #define atomic_read(v) READ_ONCE((v)->counter)
                           ^~~~~~~~~
>> fs/proc/task_nommu.c:64:26: note: in expansion of macro 'atomic_read'
     if (current->sighand && atomic_read(&current->sighand->count) > 1)
                             ^~~~~~~~~~~
   include/asm-generic/atomic.h:177:37: error: 'refcount_t {aka struct refcount_struct}' has no member named 'counter'
    #define atomic_read(v) READ_ONCE((v)->counter)
                                        ^
   include/linux/compiler.h:320:30: note: in definition of macro '__READ_ONCE'
      __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
                                 ^
   include/asm-generic/atomic.h:177:24: note: in expansion of macro 'READ_ONCE'
    #define atomic_read(v) READ_ONCE((v)->counter)
                           ^~~~~~~~~
>> fs/proc/task_nommu.c:64:26: note: in expansion of macro 'atomic_read'
     if (current->sighand && atomic_read(&current->sighand->count) > 1)
                             ^~~~~~~~~~~
   include/asm-generic/atomic.h:177:37: error: 'refcount_t {aka struct refcount_struct}' has no member named 'counter'
    #define atomic_read(v) READ_ONCE((v)->counter)
                                        ^
   include/linux/compiler.h:320:50: note: in definition of macro '__READ_ONCE'
      __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
                                                     ^
   include/asm-generic/atomic.h:177:24: note: in expansion of macro 'READ_ONCE'
    #define atomic_read(v) READ_ONCE((v)->counter)
                           ^~~~~~~~~
>> fs/proc/task_nommu.c:64:26: note: in expansion of macro 'atomic_read'
     if (current->sighand && atomic_read(&current->sighand->count) > 1)
                             ^~~~~~~~~~~

vim +/atomic_read +64 fs/proc/task_nommu.c

^1da177e Linus Torvalds    2005-04-16  48  
^1da177e Linus Torvalds    2005-04-16  49  	if (atomic_read(&mm->mm_count) > 1)
^1da177e Linus Torvalds    2005-04-16  50  		sbytes += kobjsize(mm);
^1da177e Linus Torvalds    2005-04-16  51  	else
^1da177e Linus Torvalds    2005-04-16  52  		bytes += kobjsize(mm);
^1da177e Linus Torvalds    2005-04-16  53  	
498052bb Al Viro           2009-03-30  54  	if (current->fs && current->fs->users > 1)
^1da177e Linus Torvalds    2005-04-16  55  		sbytes += kobjsize(current->fs);
^1da177e Linus Torvalds    2005-04-16  56  	else
^1da177e Linus Torvalds    2005-04-16  57  		bytes += kobjsize(current->fs);
^1da177e Linus Torvalds    2005-04-16  58  
^1da177e Linus Torvalds    2005-04-16  59  	if (current->files && atomic_read(&current->files->count) > 1)
^1da177e Linus Torvalds    2005-04-16  60  		sbytes += kobjsize(current->files);
^1da177e Linus Torvalds    2005-04-16  61  	else
^1da177e Linus Torvalds    2005-04-16  62  		bytes += kobjsize(current->files);
^1da177e Linus Torvalds    2005-04-16  63  
^1da177e Linus Torvalds    2005-04-16 @64  	if (current->sighand && atomic_read(&current->sighand->count) > 1)
^1da177e Linus Torvalds    2005-04-16  65  		sbytes += kobjsize(current->sighand);
^1da177e Linus Torvalds    2005-04-16  66  	else
^1da177e Linus Torvalds    2005-04-16  67  		bytes += kobjsize(current->sighand);
^1da177e Linus Torvalds    2005-04-16  68  
^1da177e Linus Torvalds    2005-04-16  69  	bytes += kobjsize(current); /* includes kernel stack */
^1da177e Linus Torvalds    2005-04-16  70  
df5f8314 Eric W. Biederman 2008-02-08  71  	seq_printf(m,
^1da177e Linus Torvalds    2005-04-16  72  		"Mem:\t%8lu bytes\n"

:::::: The code at line 64 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10850 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t
  2017-02-20 10:18 ` [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t Elena Reshetova
  2017-02-20 12:30   ` kbuild test robot
@ 2017-02-20 12:42   ` kbuild test robot
  1 sibling, 0 replies; 36+ messages in thread
From: kbuild test robot @ 2017-02-20 12:42 UTC (permalink / raw)
  To: Elena Reshetova
  Cc: kbuild-all, linux-kernel, cgroups, linux-audit, linux-fsdevel,
	peterz, gregkh, viro, tj, mingo, hannes, lizefan, acme,
	alexander.shishkin, paul, eparis, akpm, arnd, luto,
	Elena Reshetova, Hans Liljestrand, Kees Cook, David Windsor

[-- Attachment #1: Type: text/plain, Size: 3326 bytes --]

Hi Elena,

[auto build test ERROR on next-20170220]
[cannot apply to linus/master linux/master tip/perf/core v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Elena-Reshetova/kernel-convert-sighand_struct-count-from-atomic_t-to-refcount_t/20170220-183434
config: blackfin-BF561-EZKIT-SMP_defconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=blackfin 

All errors (new ones prefixed by >>):

   In file included from include/linux/atomic.h:4:0,
                    from arch/blackfin/include/asm/spinlock.h:14,
                    from include/linux/spinlock.h:87,
                    from include/linux/mmzone.h:7,
                    from include/linux/gfp.h:5,
                    from include/linux/mm.h:9,
                    from fs/proc/task_nommu.c:2:
   fs/proc/task_nommu.c: In function 'task_mem':
>> arch/blackfin/include/asm/atomic.h:27:53: error: 'refcount_t {aka struct refcount_struct}' has no member named 'counter'
    #define atomic_read(v) __raw_uncached_fetch_asm(&(v)->counter)
                                                        ^
   fs/proc/task_nommu.c:64:26: note: in expansion of macro 'atomic_read'
     if (current->sighand && atomic_read(&current->sighand->count) > 1)
                             ^~~~~~~~~~~

vim +27 arch/blackfin/include/asm/atomic.h

d835b6c4 arch/blackfin/include/asm/atomic.h Peter Zijlstra 2015-04-23  21  
d835b6c4 arch/blackfin/include/asm/atomic.h Peter Zijlstra 2015-04-23  22  asmlinkage int __raw_atomic_and_asm(volatile int *ptr, int value);
d835b6c4 arch/blackfin/include/asm/atomic.h Peter Zijlstra 2015-04-23  23  asmlinkage int __raw_atomic_or_asm(volatile int *ptr, int value);
6b3087c6 arch/blackfin/include/asm/atomic.h Graf Yang      2009-01-07  24  asmlinkage int __raw_atomic_xor_asm(volatile int *ptr, int value);
6b3087c6 arch/blackfin/include/asm/atomic.h Graf Yang      2009-01-07  25  asmlinkage int __raw_atomic_test_asm(const volatile int *ptr, int value);
6b3087c6 arch/blackfin/include/asm/atomic.h Graf Yang      2009-01-07  26  
ae41f32e arch/blackfin/include/asm/atomic.h Mike Frysinger 2011-06-17 @27  #define atomic_read(v) __raw_uncached_fetch_asm(&(v)->counter)
1394f032 include/asm-blackfin/atomic.h      Bryan Wu       2007-05-06  28  
d835b6c4 arch/blackfin/include/asm/atomic.h Peter Zijlstra 2015-04-23  29  #define atomic_add_return(i, v) __raw_atomic_add_asm(&(v)->counter, i)
d835b6c4 arch/blackfin/include/asm/atomic.h Peter Zijlstra 2015-04-23  30  #define atomic_sub_return(i, v) __raw_atomic_add_asm(&(v)->counter, -(i))

:::::: The code at line 27 was first introduced by commit
:::::: ae41f32e16d8e87c84cb910a6a6aefb50318894d Blackfin: SMP: convert to common asm-generic/atomic.h

:::::: TO: Mike Frysinger <vapier@gentoo.org>
:::::: CC: Mike Frysinger <vapier@gentoo.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10733 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-02-20 10:19 ` [PATCH 15/19] kernel: convert audit_tree.count " Elena Reshetova
@ 2017-02-20 22:07   ` Paul Moore
  2017-02-21  7:15     ` Reshetova, Elena
  0 siblings, 1 reply; 36+ messages in thread
From: Paul Moore @ 2017-02-20 22:07 UTC (permalink / raw)
  To: Elena Reshetova
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, tj, mingo, hannes, lizefan, acme,
	alexander.shishkin, Eric Paris, akpm, arnd, luto,
	Hans Liljestrand, Kees Cook, David Windsor

On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova
<elena.reshetova@intel.com> 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>
> ---
>  kernel/audit_tree.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

No objection on my end, same for patch 16/19.

I have no problem merging both these patches into the audit/next
branch after the merge window, is that your goal or are you merging
these via a different tree?

> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index 7b44195..7ed617b 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -9,7 +9,7 @@ struct audit_tree;
>  struct audit_chunk;
>
>  struct audit_tree {
> -       atomic_t count;
> +       refcount_t count;
>         int goner;
>         struct audit_chunk *root;
>         struct list_head chunks;
> @@ -77,7 +77,7 @@ static struct audit_tree *alloc_tree(const char *s)
>
>         tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
>         if (tree) {
> -               atomic_set(&tree->count, 1);
> +               refcount_set(&tree->count, 1);
>                 tree->goner = 0;
>                 INIT_LIST_HEAD(&tree->chunks);
>                 INIT_LIST_HEAD(&tree->rules);
> @@ -91,12 +91,12 @@ static struct audit_tree *alloc_tree(const char *s)
>
>  static inline void get_tree(struct audit_tree *tree)
>  {
> -       atomic_inc(&tree->count);
> +       refcount_inc(&tree->count);
>  }
>
>  static inline void put_tree(struct audit_tree *tree)
>  {
> -       if (atomic_dec_and_test(&tree->count))
> +       if (refcount_dec_and_test(&tree->count))
>                 kfree_rcu(tree, head);
>  }
>

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-02-20 22:07   ` Paul Moore
@ 2017-02-21  7:15     ` Reshetova, Elena
  2017-02-28 22:11       ` Paul Moore
  0 siblings, 1 reply; 36+ messages in thread
From: Reshetova, Elena @ 2017-02-21  7:15 UTC (permalink / raw)
  To: Paul Moore
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, tj, mingo, hannes, lizefan, acme,
	alexander.shishkin, Eric Paris, akpm, arnd, luto,
	Hans Liljestrand, Kees Cook, David Windsor

> On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova
> <elena.reshetova@intel.com> 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>
> > ---
> >  kernel/audit_tree.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> No objection on my end, same for patch 16/19.
> 
> I have no problem merging both these patches into the audit/next
> branch after the merge window, is that your goal or are you merging
> these via a different tree?

Thank you Paul! I think it is better if they go through the trees they supposed to go through
since this way they would get more testing and etc. So, please take the relevant ones to your tree when the time is right. 

After the first round, I guess we will see what patches are not propagating and then maybe take them via Kees tree. 

Best Regards,
Elena.

> 
> > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> > index 7b44195..7ed617b 100644
> > --- a/kernel/audit_tree.c
> > +++ b/kernel/audit_tree.c
> > @@ -9,7 +9,7 @@ struct audit_tree;
> >  struct audit_chunk;
> >
> >  struct audit_tree {
> > -       atomic_t count;
> > +       refcount_t count;
> >         int goner;
> >         struct audit_chunk *root;
> >         struct list_head chunks;
> > @@ -77,7 +77,7 @@ static struct audit_tree *alloc_tree(const char *s)
> >
> >         tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
> >         if (tree) {
> > -               atomic_set(&tree->count, 1);
> > +               refcount_set(&tree->count, 1);
> >                 tree->goner = 0;
> >                 INIT_LIST_HEAD(&tree->chunks);
> >                 INIT_LIST_HEAD(&tree->rules);
> > @@ -91,12 +91,12 @@ static struct audit_tree *alloc_tree(const char *s)
> >
> >  static inline void get_tree(struct audit_tree *tree)
> >  {
> > -       atomic_inc(&tree->count);
> > +       refcount_inc(&tree->count);
> >  }
> >
> >  static inline void put_tree(struct audit_tree *tree)
> >  {
> > -       if (atomic_dec_and_test(&tree->count))
> > +       if (refcount_dec_and_test(&tree->count))
> >                 kfree_rcu(tree, head);
> >  }
> >
> 
> --
> paul moore
> www.paul-moore.com

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-02-21  7:15     ` Reshetova, Elena
@ 2017-02-28 22:11       ` Paul Moore
  2017-03-01  0:16         ` Kees Cook
  2017-04-11 19:01         ` Paul Moore
  0 siblings, 2 replies; 36+ messages in thread
From: Paul Moore @ 2017-02-28 22:11 UTC (permalink / raw)
  To: Reshetova, Elena
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, tj, mingo, hannes, lizefan, acme,
	alexander.shishkin, Eric Paris, akpm, arnd, luto,
	Hans Liljestrand, Kees Cook, David Windsor

On Tue, Feb 21, 2017 at 2:15 AM, Reshetova, Elena
<elena.reshetova@intel.com> wrote:
>> On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova
>> <elena.reshetova@intel.com> 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>
>> > ---
>> >  kernel/audit_tree.c | 8 ++++----
>> >  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> No objection on my end, same for patch 16/19.
>>
>> I have no problem merging both these patches into the audit/next
>> branch after the merge window, is that your goal or are you merging
>> these via a different tree?
>
> Thank you Paul! I think it is better if they go through the trees they supposed to go through
> since this way they would get more testing and etc. So, please take the relevant ones to your tree when the time is right.
>
> After the first round, I guess we will see what patches are not propagating and then maybe take them via Kees tree.

I just realized that include/linux/refcount.h didn't make it into
v4.10 which means there is going to be delay until I merge them into
the audit tree (I don't base the tree on -rc releases except under
extreme circumstances).  I've got the patches queued up in a private
holding branch (I added #includes BTW) so I won't forget, but as a
FYI, they likely won't make it in until v4.12.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-02-28 22:11       ` Paul Moore
@ 2017-03-01  0:16         ` Kees Cook
  2017-03-01 19:35           ` Paul Moore
  2017-04-11 19:01         ` Paul Moore
  1 sibling, 1 reply; 36+ messages in thread
From: Kees Cook @ 2017-03-01  0:16 UTC (permalink / raw)
  To: Paul Moore
  Cc: Reshetova, Elena, linux-kernel, cgroups, linux-audit,
	linux-fsdevel, peterz, gregkh, viro, tj, mingo, hannes, lizefan,
	acme, alexander.shishkin, Eric Paris, akpm, arnd, luto,
	Hans Liljestrand, David Windsor

On Tue, Feb 28, 2017 at 2:11 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Feb 21, 2017 at 2:15 AM, Reshetova, Elena
> <elena.reshetova@intel.com> wrote:
>>> On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova
>>> <elena.reshetova@intel.com> 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>
>>> > ---
>>> >  kernel/audit_tree.c | 8 ++++----
>>> >  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> No objection on my end, same for patch 16/19.
>>>
>>> I have no problem merging both these patches into the audit/next
>>> branch after the merge window, is that your goal or are you merging
>>> these via a different tree?
>>
>> Thank you Paul! I think it is better if they go through the trees they supposed to go through
>> since this way they would get more testing and etc. So, please take the relevant ones to your tree when the time is right.
>>
>> After the first round, I guess we will see what patches are not propagating and then maybe take them via Kees tree.
>
> I just realized that include/linux/refcount.h didn't make it into
> v4.10 which means there is going to be delay until I merge them into
> the audit tree (I don't base the tree on -rc releases except under
> extreme circumstances).  I've got the patches queued up in a private
> holding branch (I added #includes BTW) so I won't forget, but as a
> FYI, they likely won't make it in until v4.12.

I'm not asking for you to change this, but I am curious: doesn't that
force you to always be a release behind? I've tended to base trees on
-rc2 (and then the final release while the next merge window is open).
But that may be because I tend to have such wide dependencies...

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-03-01  0:16         ` Kees Cook
@ 2017-03-01 19:35           ` Paul Moore
  2017-03-01 23:04             ` Kees Cook
  0 siblings, 1 reply; 36+ messages in thread
From: Paul Moore @ 2017-03-01 19:35 UTC (permalink / raw)
  To: Kees Cook
  Cc: Reshetova, Elena, linux-kernel, cgroups, linux-audit,
	linux-fsdevel, peterz, gregkh, viro, tj, mingo, hannes, lizefan,
	acme, alexander.shishkin, Eric Paris, akpm, arnd, luto,
	Hans Liljestrand, David Windsor

On Tue, Feb 28, 2017 at 7:16 PM, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Feb 28, 2017 at 2:11 PM, Paul Moore <paul@paul-moore.com> wrote:
>> I just realized that include/linux/refcount.h didn't make it into
>> v4.10 which means there is going to be delay until I merge them into
>> the audit tree (I don't base the tree on -rc releases except under
>> extreme circumstances).  I've got the patches queued up in a private
>> holding branch (I added #includes BTW) so I won't forget, but as a
>> FYI, they likely won't make it in until v4.12.
>
> I'm not asking for you to change this, but I am curious: doesn't that
> force you to always be a release behind? I've tended to base trees on
> -rc2 (and then the final release while the next merge window is open).
> But that may be because I tend to have such wide dependencies...

In general, yes ... and if you are just looking for the short answer
I'd leave it at that.  I do leave door open for exceptions under
unusual circumstances, but I don't believe the refcount_t conversion
is one of those cases.

The longer answer lies in a balancing act between what I understand
Linus' and/or James desires (different upstream maintainers, different
approaches, but for my own sanity I like to stick to a single
"process" across my trees), the linux-next effort, branch stability
(aka limited or predictable rebases), and my own testing requirements.
First off, the current policy I follow for the SELinux and audit trees
can be found here:

* http://www.paul-moore.com/blog/d/2016/03/kernel-repo-process.html

... it's relatively simple and has proven to work reasonably well over
the past year or so.  On occasion, the subsystem changes in a given
release are significant enough that I skip a rebase (step #2 in the
process) but that has only happened once (twice?) with the audit tree
and didn't prove to be a real problem; this is less of an issue with
the SELinux tree as James often rebases the linux-security tree to the
current -rc1 or -rc2 tree.

As for the balancing act ... My understanding is that Linus doesn't
like to see pull requests from trees based on -rcX tags, he would much
prefer to see trees based on a proper release, e.g. v4.10; on the plus
side, Linus is willing to put up with some merge fuzzing so long as it
is understandable and/or well explained.  James wants pull requests
that apply with zero merge conflicts to his linux-security/next tree;
on the plus side, the linux-security/next tree tends to be based on
the current -rc1/2 so a broad set of dependencies isn't too bad (which
is important for SELinux).  The linux-next people want to see a commit
ID follow a steady progression of multiple weeks in the subsystem
-next branch and then a trickle up through various trees until it hits
Linus' tree.  The branch stability requirements are pretty obvious,
and with the exception of the -next branches during/immediately-after
the merge window, I don't really rebase branches unless there is a
Very Good Reason.  Finally, my own testing requirements are such that
I want to test the current SELinux and audit -next/-stable branches
against the latest bits in Linus' tree (e.g. -rcX releases) on a
weekly basis so keeping those branches as current as possible is
important; I talked a bit more about my testing at Flock 2016,
slides/video at the link below:

* http://www.paul-moore.com/blog/d/2016/08/flock-kernel-testing.html
* https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext

There ya go, probably more than you wanted to know, but that's why
things are the way they are with the SELinux and audit trees.  I
remain open to new ideas about how to manage the trees, but the
current arrangement seems to work reasonably well at the moment.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-03-01 19:35           ` Paul Moore
@ 2017-03-01 23:04             ` Kees Cook
  0 siblings, 0 replies; 36+ messages in thread
From: Kees Cook @ 2017-03-01 23:04 UTC (permalink / raw)
  To: Paul Moore
  Cc: Reshetova, Elena, linux-kernel, cgroups, linux-audit,
	linux-fsdevel, peterz, gregkh, viro, tj, mingo, hannes, lizefan,
	acme, alexander.shishkin, Eric Paris, akpm, arnd, luto,
	Hans Liljestrand, David Windsor

On Wed, Mar 1, 2017 at 11:35 AM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Feb 28, 2017 at 7:16 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Tue, Feb 28, 2017 at 2:11 PM, Paul Moore <paul@paul-moore.com> wrote:
>>> I just realized that include/linux/refcount.h didn't make it into
>>> v4.10 which means there is going to be delay until I merge them into
>>> the audit tree (I don't base the tree on -rc releases except under
>>> extreme circumstances).  I've got the patches queued up in a private
>>> holding branch (I added #includes BTW) so I won't forget, but as a
>>> FYI, they likely won't make it in until v4.12.
>>
>> I'm not asking for you to change this, but I am curious: doesn't that
>> force you to always be a release behind? I've tended to base trees on
>> -rc2 (and then the final release while the next merge window is open).
>> But that may be because I tend to have such wide dependencies...
>
> In general, yes ... and if you are just looking for the short answer
> I'd leave it at that.  I do leave door open for exceptions under
> unusual circumstances, but I don't believe the refcount_t conversion
> is one of those cases.

Right, totally agreed: doing refcount_t conversions is going to be a
long tail process, which is fine.

> The longer answer lies in a balancing act between what I understand
> Linus' and/or James desires (different upstream maintainers, different
> approaches, but for my own sanity I like to stick to a single
> "process" across my trees), the linux-next effort, branch stability
> (aka limited or predictable rebases), and my own testing requirements.
> First off, the current policy I follow for the SELinux and audit trees
> can be found here:
>
> * http://www.paul-moore.com/blog/d/2016/03/kernel-repo-process.html
>
> ... it's relatively simple and has proven to work reasonably well over
> the past year or so.  On occasion, the subsystem changes in a given
> release are significant enough that I skip a rebase (step #2 in the
> process) but that has only happened once (twice?) with the audit tree
> and didn't prove to be a real problem; this is less of an issue with
> the SELinux tree as James often rebases the linux-security tree to the
> current -rc1 or -rc2 tree.
>
> As for the balancing act ... My understanding is that Linus doesn't
> like to see pull requests from trees based on -rcX tags, he would much
> prefer to see trees based on a proper release, e.g. v4.10; on the plus
> side, Linus is willing to put up with some merge fuzzing so long as it
> is understandable and/or well explained.

Ah, hrm, yeah, I'd heard Linus mention that in the past, but when I
was talking with sfr about what's best for -next tree targets, he
mentioned -rc2 since it tends (in my understanding/memory of what he
told me) to be the least "noisy" position. Interesting!

> James wants pull requests
> that apply with zero merge conflicts to his linux-security/next tree;
> on the plus side, the linux-security/next tree tends to be based on
> the current -rc1/2 so a broad set of dependencies isn't too bad (which
> is important for SELinux).  The linux-next people want to see a commit
> ID follow a steady progression of multiple weeks in the subsystem
> -next branch and then a trickle up through various trees until it hits
> Linus' tree.  The branch stability requirements are pretty obvious,
> and with the exception of the -next branches during/immediately-after
> the merge window, I don't really rebase branches unless there is a
> Very Good Reason.  Finally, my own testing requirements are such that
> I want to test the current SELinux and audit -next/-stable branches
> against the latest bits in Linus' tree (e.g. -rcX releases) on a
> weekly basis so keeping those branches as current as possible is
> important; I talked a bit more about my testing at Flock 2016,
> slides/video at the link below:
>
> * http://www.paul-moore.com/blog/d/2016/08/flock-kernel-testing.html
> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext
>
> There ya go, probably more than you wanted to know, but that's why
> things are the way they are with the SELinux and audit trees.  I
> remain open to new ideas about how to manage the trees, but the
> current arrangement seems to work reasonably well at the moment.

Actually, no, this was great. Thanks! It's why I asked. :) Whenever I
see a maintainer with a different approach I try to figure out what
I'm missing, etc. :)

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 12/19] kernel: convert css_set.refcount from atomic_t to refcount_t
  2017-02-20 10:19 ` [PATCH 12/19] kernel: convert css_set.refcount " Elena Reshetova
@ 2017-03-06 19:54   ` Tejun Heo
  2017-03-07 19:12     ` Reshetova, Elena
  0 siblings, 1 reply; 36+ messages in thread
From: Tejun Heo @ 2017-03-06 19:54 UTC (permalink / raw)
  To: Elena Reshetova
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, mingo, hannes, lizefan, acme, alexander.shishkin,
	paul, eparis, akpm, arnd, luto, Hans Liljestrand, Kees Cook,
	David Windsor

Hello,

On Mon, Feb 20, 2017 at 12:19:01PM +0200, Elena Reshetova wrote:
> @@ -134,10 +135,13 @@ static inline void put_css_set(struct css_set *cset)
>  	 * can see it. Similar to atomic_dec_and_lock(), but for an
>  	 * rwlock
>  	 */
> -	if (atomic_add_unless(&cset->refcount, -1, 1))
> +	spin_lock_irqsave(&css_set_lock, flags);
> +	if (refcount_read(&cset->refcount) != 1) {
> +		WARN_ON(refcount_dec_and_test(&cset->refcount));
> +		spin_unlock_irqrestore(&css_set_lock, flags);
>  		return;
> +	}

This isn't an equivalent conversion and should have been mentioned in
the patch description.  Hmm... and I'm not sure this is a good idea.
Can't we add the matching operation on refcount_t rather than adding
extra locking like this?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 11/19] kernel: convert cgroup_namespace.count from atomic_t to refcount_t
  2017-02-20 10:19 ` [PATCH 11/19] kernel: convert cgroup_namespace.count " Elena Reshetova
@ 2017-03-06 19:55   ` Tejun Heo
  0 siblings, 0 replies; 36+ messages in thread
From: Tejun Heo @ 2017-03-06 19:55 UTC (permalink / raw)
  To: Elena Reshetova
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, mingo, hannes, lizefan, acme, alexander.shishkin,
	paul, eparis, akpm, arnd, luto, Hans Liljestrand, Kees Cook,
	David Windsor

On Mon, Feb 20, 2017 at 12:19:00PM +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>

Applied to cgroup/for-4.12.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH 12/19] kernel: convert css_set.refcount from atomic_t to refcount_t
  2017-03-06 19:54   ` Tejun Heo
@ 2017-03-07 19:12     ` Reshetova, Elena
  2017-03-07 19:21       ` Tejun Heo
  0 siblings, 1 reply; 36+ messages in thread
From: Reshetova, Elena @ 2017-03-07 19:12 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, mingo, hannes, lizefan, acme, alexander.shishkin,
	paul, eparis, akpm, arnd, luto, Hans Liljestrand, Kees Cook,
	David Windsor

> Hello,
> 
> On Mon, Feb 20, 2017 at 12:19:01PM +0200, Elena Reshetova wrote:
> > @@ -134,10 +135,13 @@ static inline void put_css_set(struct css_set *cset)
> >  	 * can see it. Similar to atomic_dec_and_lock(), but for an
> >  	 * rwlock
> >  	 */
> > -	if (atomic_add_unless(&cset->refcount, -1, 1))
> > +	spin_lock_irqsave(&css_set_lock, flags);
> > +	if (refcount_read(&cset->refcount) != 1) {
> > +		WARN_ON(refcount_dec_and_test(&cset-
> >refcount));
> > +		spin_unlock_irqrestore(&css_set_lock, flags);
> >  		return;
> > +	}
> 
> This isn't an equivalent conversion and should have been mentioned in
> the patch description.  Hmm... and I'm not sure this is a good idea.
> Can't we add the matching operation on refcount_t rather than adding
> extra locking like this?

Oh, actually this is fault on our side: initially we didn't have a refcount_dec_not_one() interface and we had to be creative in converting cases like this, but now the interface is present, but we actually forgot to convert this particular case. 
So, the above change should be:

-	if (atomic_add_unless(&cset->refcount, -1, 1))
+	if (refcount_dec_not_one(&cset->refcount))


Do you want me to resend or could you modify the patch while applying?

Best Regards,
Elena.

> 
> Thanks.
> 
> --
> tejun

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 12/19] kernel: convert css_set.refcount from atomic_t to refcount_t
  2017-03-07 19:12     ` Reshetova, Elena
@ 2017-03-07 19:21       ` Tejun Heo
  0 siblings, 0 replies; 36+ messages in thread
From: Tejun Heo @ 2017-03-07 19:21 UTC (permalink / raw)
  To: Reshetova, Elena
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, mingo, hannes, lizefan, acme, alexander.shishkin,
	paul, eparis, akpm, arnd, luto, Hans Liljestrand, Kees Cook,
	David Windsor

Hello,

On Tue, Mar 07, 2017 at 07:12:51PM +0000, Reshetova, Elena wrote:
> Do you want me to resend or could you modify the patch while applying?

Can you please send the updated patch?

Thanks!

-- 
tejun

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-02-28 22:11       ` Paul Moore
  2017-03-01  0:16         ` Kees Cook
@ 2017-04-11 19:01         ` Paul Moore
  2017-04-18  6:33           ` Reshetova, Elena
  1 sibling, 1 reply; 36+ messages in thread
From: Paul Moore @ 2017-04-11 19:01 UTC (permalink / raw)
  To: Reshetova, Elena
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, tj, mingo, hannes, lizefan, acme,
	alexander.shishkin, Eric Paris, akpm, arnd, luto,
	Hans Liljestrand, Kees Cook, David Windsor

On Tue, Feb 28, 2017 at 5:11 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Feb 21, 2017 at 2:15 AM, Reshetova, Elena
> <elena.reshetova@intel.com> wrote:
>>> On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova
>>> <elena.reshetova@intel.com> 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>
>>> > ---
>>> >  kernel/audit_tree.c | 8 ++++----
>>> >  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> No objection on my end, same for patch 16/19.
>>>
>>> I have no problem merging both these patches into the audit/next
>>> branch after the merge window, is that your goal or are you merging
>>> these via a different tree?
>>
>> Thank you Paul! I think it is better if they go through the trees they supposed to go through
>> since this way they would get more testing and etc. So, please take the relevant ones to your tree when the time is right.
>>
>> After the first round, I guess we will see what patches are not propagating and then maybe take them via Kees tree.
>
> I just realized that include/linux/refcount.h didn't make it into
> v4.10 which means there is going to be delay until I merge them into
> the audit tree (I don't base the tree on -rc releases except under
> extreme circumstances).  I've got the patches queued up in a private
> holding branch (I added #includes BTW) so I won't forget, but as a
> FYI, they likely won't make it in until v4.12.

Quick update on this: I needed to rebase the audit/next branch for
other reasons so I've gone ahead and merged 15/19 and 16/19 into
audit/next; they should go to Linus during the next merge window.
Thanks for your patience.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t
  2017-04-11 19:01         ` Paul Moore
@ 2017-04-18  6:33           ` Reshetova, Elena
  0 siblings, 0 replies; 36+ messages in thread
From: Reshetova, Elena @ 2017-04-18  6:33 UTC (permalink / raw)
  To: Paul Moore
  Cc: linux-kernel, cgroups, linux-audit, linux-fsdevel, peterz,
	gregkh, viro, tj, mingo, hannes, lizefan, acme,
	alexander.shishkin, Eric Paris, akpm, arnd, luto,
	Hans Liljestrand, Kees Cook, David Windsor

> On Tue, Feb 28, 2017 at 5:11 PM, Paul Moore <paul@paul-moore.com> wrote:
> > On Tue, Feb 21, 2017 at 2:15 AM, Reshetova, Elena
> > <elena.reshetova@intel.com> wrote:
> >>> On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova
> >>> <elena.reshetova@intel.com> 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>
> >>> > ---
> >>> >  kernel/audit_tree.c | 8 ++++----
> >>> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> No objection on my end, same for patch 16/19.
> >>>
> >>> I have no problem merging both these patches into the audit/next
> >>> branch after the merge window, is that your goal or are you merging
> >>> these via a different tree?
> >>
> >> Thank you Paul! I think it is better if they go through the trees they supposed to
> go through
> >> since this way they would get more testing and etc. So, please take the relevant
> ones to your tree when the time is right.
> >>
> >> After the first round, I guess we will see what patches are not propagating and
> then maybe take them via Kees tree.
> >
> > I just realized that include/linux/refcount.h didn't make it into
> > v4.10 which means there is going to be delay until I merge them into
> > the audit tree (I don't base the tree on -rc releases except under
> > extreme circumstances).  I've got the patches queued up in a private
> > holding branch (I added #includes BTW) so I won't forget, but as a
> > FYI, they likely won't make it in until v4.12.
> 
> Quick update on this: I needed to rebase the audit/next branch for
> other reasons so I've gone ahead and merged 15/19 and 16/19 into
> audit/next; they should go to Linus during the next merge window.
> Thanks for your patience.

Thank you very much Paul!
I just got back from holidays, so catching up with the mail slowly.

Best Regards,
Elena.
> 
> --
> paul moore
> www.paul-moore.com

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2017-04-18  6:34 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-20 10:18 [PATCH 00/19] Kernel subsystem refcounter conversions Elena Reshetova
2017-02-20 10:18 ` [PATCH 01/19] kernel: convert sighand_struct.count from atomic_t to refcount_t Elena Reshetova
2017-02-20 12:30   ` kbuild test robot
2017-02-20 12:42   ` kbuild test robot
2017-02-20 10:18 ` [PATCH 02/19] kernel: convert signal_struct.sigcnt " Elena Reshetova
2017-02-20 10:18 ` [PATCH 03/19] kernel: convert user_struct.__count " Elena Reshetova
2017-02-20 10:18 ` [PATCH 04/19] kernel: convert task_struct.usage " Elena Reshetova
2017-02-20 10:18 ` [PATCH 05/19] kernel: convert task_struct.stack_refcount " Elena Reshetova
2017-02-20 10:18 ` [PATCH 06/19] kernel: convert perf_event_context.refcount " Elena Reshetova
2017-02-20 10:28   ` Peter Zijlstra
2017-02-20 12:14     ` Reshetova, Elena
2017-02-20 10:18 ` [PATCH 07/19] kernel: convert ring_buffer.refcount " Elena Reshetova
2017-02-20 10:18 ` [PATCH 08/19] kernel: convert ring_buffer.aux_refcount " Elena Reshetova
2017-02-20 10:18 ` [PATCH 09/19] kernel: convert uprobe.ref " Elena Reshetova
2017-02-20 10:18 ` [PATCH 10/19] kernel: convert nsproxy.count " Elena Reshetova
2017-02-20 10:19 ` [PATCH 11/19] kernel: convert cgroup_namespace.count " Elena Reshetova
2017-03-06 19:55   ` Tejun Heo
2017-02-20 10:19 ` [PATCH 12/19] kernel: convert css_set.refcount " Elena Reshetova
2017-03-06 19:54   ` Tejun Heo
2017-03-07 19:12     ` Reshetova, Elena
2017-03-07 19:21       ` Tejun Heo
2017-02-20 10:19 ` [PATCH 13/19] kernel: convert group_info.usage " Elena Reshetova
2017-02-20 10:19 ` [PATCH 14/19] kernel: convert cred.usage " Elena Reshetova
2017-02-20 10:19 ` [PATCH 15/19] kernel: convert audit_tree.count " Elena Reshetova
2017-02-20 22:07   ` Paul Moore
2017-02-21  7:15     ` Reshetova, Elena
2017-02-28 22:11       ` Paul Moore
2017-03-01  0:16         ` Kees Cook
2017-03-01 19:35           ` Paul Moore
2017-03-01 23:04             ` Kees Cook
2017-04-11 19:01         ` Paul Moore
2017-04-18  6:33           ` Reshetova, Elena
2017-02-20 10:19 ` [PATCH 16/19] kernel: convert audit_watch.count " Elena Reshetova
2017-02-20 10:19 ` [PATCH 17/19] kernel: convert numa_group.refcount " Elena Reshetova
2017-02-20 10:19 ` [PATCH 18/19] kernel: convert futex_pi_state.refcount " Elena Reshetova
2017-02-20 10:19 ` [PATCH 19/19] kernel: convert kcov.refcount " Elena Reshetova

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).