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(a)intel.com&gt; Signed-off-by: Hans Liljestrand <ishkamiel(a)gmail.com&gt; Signed-off-by: Kees Cook <keescook(a)chromium.org&gt; Signed-off-by: David Windsor <dwindsor(a)gmail.com&gt; --- 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

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(a)intel.com&gt; Signed-off-by: Hans Liljestrand <ishkamiel(a)gmail.com&gt; Signed-off-by: Kees Cook <keescook(a)chromium.org&gt; Signed-off-by: David Windsor <dwindsor(a)gmail.com&gt; --- 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

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(a)intel.com&gt; Signed-off-by: Hans Liljestrand <ishkamiel(a)gmail.com&gt; Signed-off-by: Kees Cook <keescook(a)chromium.org&gt; Signed-off-by: David Windsor <dwindsor(a)gmail.com&gt; --- 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

True, will fix. Thanks!

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(a)intel.com&gt; Signed-off-by: Hans Liljestrand <ishkamiel(a)gmail.com&gt; Signed-off-by: Kees Cook <keescook(a)chromium.org&gt; Signed-off-by: David Windsor <dwindsor(a)gmail.com&gt; --- 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

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(a)intel.com&gt; Signed-off-by: Hans Liljestrand <ishkamiel(a)gmail.com&gt; Signed-off-by: Kees Cook <keescook(a)chromium.org&gt; Signed-off-by: David Windsor <dwindsor(a)gmail.com&gt; --- 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

Hello, On Mon, Feb 20, 2017 at 12:19:01PM +0200, Elena Reshetova wrote: 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

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(a)intel.com&gt; Signed-off-by: Hans Liljestrand <ishkamiel(a)gmail.com&gt; Signed-off-by: Kees Cook <keescook(a)chromium.org&gt; Signed-off-by: David Windsor <dwindsor(a)gmail.com&gt; --- 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

On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova <elena.reshetova(a)intel.com&gt; wrote: 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? -- paul moore www.paul-moore.com

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(a)intel.com&gt; Signed-off-by: Hans Liljestrand <ishkamiel(a)gmail.com&gt; Signed-off-by: Kees Cook <keescook(a)chromium.org&gt; Signed-off-by: David Windsor <dwindsor(a)gmail.com&gt; --- 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

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(a)intel.com&gt; Signed-off-by: Hans Liljestrand <ishkamiel(a)gmail.com&gt; Signed-off-by: Kees Cook <keescook(a)chromium.org&gt; Signed-off-by: David Windsor <dwindsor(a)gmail.com&gt; --- 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