* [PATCH 0/5] mm subsystem refcounter conversions
@ 2017-02-20 10:49 Elena Reshetova
2017-02-20 10:49 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Elena Reshetova @ 2017-02-20 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, 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 mm 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 (5):
mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t
mm: convert anon_vma.refcount from atomic_t to refcount_t
mm: convert kmemleak_object.use_count from atomic_t to refcount_t
mm: convert mm_struct.mm_users from atomic_t to refcount_t
mm: convert mm_struct.mm_count from atomic_t to refcount_t
arch/alpha/kernel/smp.c | 6 +++---
arch/arc/mm/tlb.c | 2 +-
arch/blackfin/mach-common/smp.c | 2 +-
arch/ia64/include/asm/tlbflush.h | 2 +-
arch/ia64/kernel/smp.c | 2 +-
arch/ia64/sn/kernel/sn2/sn2_smp.c | 4 ++--
arch/mips/kernel/process.c | 2 +-
arch/mips/kernel/smp.c | 6 +++---
arch/parisc/include/asm/mmu_context.h | 2 +-
arch/powerpc/mm/hugetlbpage.c | 2 +-
arch/powerpc/mm/icswx.c | 4 ++--
arch/sh/kernel/smp.c | 6 +++---
arch/sparc/kernel/smp_64.c | 6 +++---
arch/sparc/mm/srmmu.c | 2 +-
arch/um/kernel/tlb.c | 2 +-
arch/x86/kernel/tboot.c | 4 ++--
arch/xtensa/kernel/smp.c | 5 +++++
drivers/firmware/efi/arm-runtime.c | 4 ++--
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +-
drivers/gpu/drm/i915/i915_gem_userptr.c | 1 -
fs/coredump.c | 2 +-
fs/proc/base.c | 2 +-
fs/userfaultfd.c | 3 +--
include/linux/backing-dev-defs.h | 3 ++-
include/linux/backing-dev.h | 4 ++--
include/linux/mm_types.h | 5 +++--
include/linux/rmap.h | 7 ++++---
include/linux/sched.h | 10 +++++-----
kernel/events/uprobes.c | 2 +-
kernel/exit.c | 2 +-
kernel/fork.c | 12 ++++++------
kernel/sched/core.c | 2 +-
lib/is_single_threaded.c | 2 +-
mm/backing-dev.c | 11 ++++++-----
mm/debug.c | 4 ++--
mm/init-mm.c | 4 ++--
mm/khugepaged.c | 2 +-
mm/kmemleak.c | 16 ++++++++--------
mm/ksm.c | 2 +-
mm/memory.c | 2 +-
mm/mmu_notifier.c | 10 +++++-----
mm/mprotect.c | 2 +-
mm/oom_kill.c | 2 +-
mm/rmap.c | 14 +++++++-------
mm/swapfile.c | 2 +-
mm/vmacache.c | 2 +-
46 files changed, 101 insertions(+), 94 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t 2017-02-20 10:49 [PATCH 0/5] mm subsystem refcounter conversions Elena Reshetova @ 2017-02-20 10:49 ` Elena Reshetova 2017-02-20 11:33 ` kbuild test robot 2017-02-20 10:49 ` [PATCH 2/5] mm: convert anon_vma.refcount " Elena Reshetova ` (3 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Elena Reshetova @ 2017-02-20 10:49 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, 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. Switch bdi_writeback_congested.refcnt from atomic_t to refcount_t and increment initial value by 1. The incrementation affecs the function wb_congested_get_create which previously incremented both found and created objects. After this patch the function will increment only found objects and instead set the refcount of new objects to 1. Note that new_congested is initially NULL, and will be discarded unless exiting via the 'if (new_congested)' section. 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/backing-dev-defs.h | 3 ++- include/linux/backing-dev.h | 4 ++-- mm/backing-dev.c | 11 ++++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h index ad95581..609ee6f 100644 --- a/include/linux/backing-dev-defs.h +++ b/include/linux/backing-dev-defs.h @@ -4,6 +4,7 @@ #include <linux/list.h> #include <linux/radix-tree.h> #include <linux/rbtree.h> +#include <linux/refcount.h> #include <linux/spinlock.h> #include <linux/percpu_counter.h> #include <linux/percpu-refcount.h> @@ -51,7 +52,7 @@ enum wb_stat_item { */ struct bdi_writeback_congested { unsigned long state; /* WB_[a]sync_congested flags */ - atomic_t refcnt; /* nr of attached wb's and blkg */ + refcount_t refcnt; /* nr of attached wb's and blkg */ #ifdef CONFIG_CGROUP_WRITEBACK struct backing_dev_info *bdi; /* the associated bdi */ diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h index c52a48c..4726d81 100644 --- a/include/linux/backing-dev.h +++ b/include/linux/backing-dev.h @@ -430,13 +430,13 @@ static inline bool inode_cgwb_enabled(struct inode *inode) static inline struct bdi_writeback_congested * wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) { - atomic_inc(&bdi->wb_congested->refcnt); + refcount_inc(&bdi->wb_congested->refcnt); return bdi->wb_congested; } static inline void wb_congested_put(struct bdi_writeback_congested *congested) { - if (atomic_dec_and_test(&congested->refcnt)) + if (refcount_dec_and_test(&congested->refcnt)) kfree(congested); } diff --git a/mm/backing-dev.c b/mm/backing-dev.c index 6d861d0..2df3ed7 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -417,14 +417,17 @@ wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) node = &parent->rb_left; else if (congested->blkcg_id > blkcg_id) node = &parent->rb_right; - else - goto found; + else { + refcount_inc(&congested->refcnt); + goto found; + } } if (new_congested) { /* !found and storage for new one already allocated, insert */ congested = new_congested; new_congested = NULL; + refcount_set(&congested->refcnt, 1); rb_link_node(&congested->rb_node, parent, node); rb_insert_color(&congested->rb_node, &bdi->cgwb_congested_tree); goto found; @@ -437,13 +440,11 @@ wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) if (!new_congested) return NULL; - atomic_set(&new_congested->refcnt, 0); new_congested->bdi = bdi; new_congested->blkcg_id = blkcg_id; goto retry; found: - atomic_inc(&congested->refcnt); spin_unlock_irqrestore(&cgwb_lock, flags); kfree(new_congested); return congested; @@ -460,7 +461,7 @@ void wb_congested_put(struct bdi_writeback_congested *congested) unsigned long flags; local_irq_save(flags); - if (!atomic_dec_and_lock(&congested->refcnt, &cgwb_lock)) { + if (!refcount_dec_and_lock(&congested->refcnt, &cgwb_lock)) { local_irq_restore(flags); return; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t 2017-02-20 10:49 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova @ 2017-02-20 11:33 ` kbuild test robot 0 siblings, 0 replies; 13+ messages in thread From: kbuild test robot @ 2017-02-20 11:33 UTC (permalink / raw) To: Elena Reshetova Cc: kbuild-all, linux-kernel, linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook, David Windsor [-- Attachment #1: Type: text/plain, Size: 2732 bytes --] Hi Elena, [auto build test ERROR on mmotm/master] [also build test ERROR on next-20170220] [cannot apply to linus/master linux/master] [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/mm-subsystem-refcounter-conversions/20170220-190351 base: git://git.cmpxchg.org/linux-mmotm.git master config: i386-tinyconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): mm/backing-dev.c: In function 'cgwb_bdi_init': >> mm/backing-dev.c:798:13: error: passing argument 1 of 'atomic_set' from incompatible pointer type [-Werror=incompatible-pointer-types] atomic_set(&bdi->wb_congested->refcnt, 1); ^ In file included from include/linux/atomic.h:4:0, from arch/x86/include/asm/thread_info.h:53, from include/linux/thread_info.h:25, from arch/x86/include/asm/preempt.h:6, from include/linux/preempt.h:59, from include/linux/spinlock.h:50, from include/linux/wait.h:8, from mm/backing-dev.c:2: arch/x86/include/asm/atomic.h:36:29: note: expected 'atomic_t * {aka struct <anonymous> *}' but argument is of type 'refcount_t * {aka struct refcount_struct *}' static __always_inline void atomic_set(atomic_t *v, int i) ^~~~~~~~~~ cc1: some warnings being treated as errors vim +/atomic_set +798 mm/backing-dev.c a13f35e8 Tejun Heo 2015-07-02 792 int err; a13f35e8 Tejun Heo 2015-07-02 793 a13f35e8 Tejun Heo 2015-07-02 794 bdi->wb_congested = kzalloc(sizeof(*bdi->wb_congested), GFP_KERNEL); a13f35e8 Tejun Heo 2015-07-02 795 if (!bdi->wb_congested) a13f35e8 Tejun Heo 2015-07-02 796 return -ENOMEM; a13f35e8 Tejun Heo 2015-07-02 797 d3036542 mmotm auto import 2017-02-18 @798 atomic_set(&bdi->wb_congested->refcnt, 1); d3036542 mmotm auto import 2017-02-18 799 a13f35e8 Tejun Heo 2015-07-02 800 err = wb_init(&bdi->wb, bdi, 1, GFP_KERNEL); a13f35e8 Tejun Heo 2015-07-02 801 if (err) { :::::: The code at line 798 was first introduced by commit :::::: d3036542f0baefd61fb18ce9023c2cd96c89349c linux-next :::::: TO: mmotm auto import <mm-commits@vger.kernel.org> :::::: CC: Johannes Weiner <hannes@cmpxchg.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: 6523 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/5] mm: convert anon_vma.refcount from atomic_t to refcount_t 2017-02-20 10:49 [PATCH 0/5] mm subsystem refcounter conversions Elena Reshetova 2017-02-20 10:49 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova @ 2017-02-20 10:49 ` Elena Reshetova 2017-04-22 10:44 ` zhong jiang 2017-02-20 10:49 ` [PATCH 3/5] mm: convert kmemleak_object.use_count " Elena Reshetova ` (2 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Elena Reshetova @ 2017-02-20 10:49 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, 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/rmap.h | 7 ++++--- mm/rmap.c | 14 +++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/include/linux/rmap.h b/include/linux/rmap.h index 8c89e90..a8f4a97 100644 --- a/include/linux/rmap.h +++ b/include/linux/rmap.h @@ -10,6 +10,7 @@ #include <linux/rwsem.h> #include <linux/memcontrol.h> #include <linux/highmem.h> +#include <linux/refcount.h> /* * The anon_vma heads a list of private "related" vmas, to scan if @@ -35,7 +36,7 @@ struct anon_vma { * the reference is responsible for clearing up the * anon_vma if they are the last user on release */ - atomic_t refcount; + refcount_t refcount; /* * Count of child anon_vmas and VMAs which points to this anon_vma. @@ -102,14 +103,14 @@ enum ttu_flags { #ifdef CONFIG_MMU static inline void get_anon_vma(struct anon_vma *anon_vma) { - atomic_inc(&anon_vma->refcount); + refcount_inc(&anon_vma->refcount); } void __put_anon_vma(struct anon_vma *anon_vma); static inline void put_anon_vma(struct anon_vma *anon_vma) { - if (atomic_dec_and_test(&anon_vma->refcount)) + if (refcount_dec_and_test(&anon_vma->refcount)) __put_anon_vma(anon_vma); } diff --git a/mm/rmap.c b/mm/rmap.c index 8774791..3321c86 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -77,7 +77,7 @@ static inline struct anon_vma *anon_vma_alloc(void) anon_vma = kmem_cache_alloc(anon_vma_cachep, GFP_KERNEL); if (anon_vma) { - atomic_set(&anon_vma->refcount, 1); + refcount_set(&anon_vma->refcount, 1); anon_vma->degree = 1; /* Reference for first vma */ anon_vma->parent = anon_vma; /* @@ -92,7 +92,7 @@ static inline struct anon_vma *anon_vma_alloc(void) static inline void anon_vma_free(struct anon_vma *anon_vma) { - VM_BUG_ON(atomic_read(&anon_vma->refcount)); + VM_BUG_ON(refcount_read(&anon_vma->refcount)); /* * Synchronize against page_lock_anon_vma_read() such that @@ -421,7 +421,7 @@ static void anon_vma_ctor(void *data) struct anon_vma *anon_vma = data; init_rwsem(&anon_vma->rwsem); - atomic_set(&anon_vma->refcount, 0); + refcount_set(&anon_vma->refcount, 0); anon_vma->rb_root = RB_ROOT; } @@ -470,7 +470,7 @@ struct anon_vma *page_get_anon_vma(struct page *page) goto out; anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON); - if (!atomic_inc_not_zero(&anon_vma->refcount)) { + if (!refcount_inc_not_zero(&anon_vma->refcount)) { anon_vma = NULL; goto out; } @@ -529,7 +529,7 @@ struct anon_vma *page_lock_anon_vma_read(struct page *page) } /* trylock failed, we got to sleep */ - if (!atomic_inc_not_zero(&anon_vma->refcount)) { + if (!refcount_inc_not_zero(&anon_vma->refcount)) { anon_vma = NULL; goto out; } @@ -544,7 +544,7 @@ struct anon_vma *page_lock_anon_vma_read(struct page *page) rcu_read_unlock(); anon_vma_lock_read(anon_vma); - if (atomic_dec_and_test(&anon_vma->refcount)) { + if (refcount_dec_and_test(&anon_vma->refcount)) { /* * Oops, we held the last refcount, release the lock * and bail -- can't simply use put_anon_vma() because @@ -1577,7 +1577,7 @@ void __put_anon_vma(struct anon_vma *anon_vma) struct anon_vma *root = anon_vma->root; anon_vma_free(anon_vma); - if (root != anon_vma && atomic_dec_and_test(&root->refcount)) + if (root != anon_vma && refcount_dec_and_test(&root->refcount)) anon_vma_free(root); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/5] mm: convert anon_vma.refcount from atomic_t to refcount_t 2017-02-20 10:49 ` [PATCH 2/5] mm: convert anon_vma.refcount " Elena Reshetova @ 2017-04-22 10:44 ` zhong jiang 2017-04-24 10:20 ` Reshetova, Elena 0 siblings, 1 reply; 13+ messages in thread From: zhong jiang @ 2017-04-22 10:44 UTC (permalink / raw) To: Elena Reshetova Cc: linux-kernel, linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, akpm, arnd, luto, Hans Liljestrand, Kees Cook, David Windsor Hi, Elean Do the issue had really occured, use-after-free. but why the patch is not received. or is is possible for the situation. Thanks zhongjiang On 2017/2/20 18:49, 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> > --- > include/linux/rmap.h | 7 ++++--- > mm/rmap.c | 14 +++++++------- > 2 files changed, 11 insertions(+), 10 deletions(-) > > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > index 8c89e90..a8f4a97 100644 > --- a/include/linux/rmap.h > +++ b/include/linux/rmap.h > @@ -10,6 +10,7 @@ > #include <linux/rwsem.h> > #include <linux/memcontrol.h> > #include <linux/highmem.h> > +#include <linux/refcount.h> > > /* > * The anon_vma heads a list of private "related" vmas, to scan if > @@ -35,7 +36,7 @@ struct anon_vma { > * the reference is responsible for clearing up the > * anon_vma if they are the last user on release > */ > - atomic_t refcount; > + refcount_t refcount; > > /* > * Count of child anon_vmas and VMAs which points to this anon_vma. > @@ -102,14 +103,14 @@ enum ttu_flags { > #ifdef CONFIG_MMU > static inline void get_anon_vma(struct anon_vma *anon_vma) > { > - atomic_inc(&anon_vma->refcount); > + refcount_inc(&anon_vma->refcount); > } > > void __put_anon_vma(struct anon_vma *anon_vma); > > static inline void put_anon_vma(struct anon_vma *anon_vma) > { > - if (atomic_dec_and_test(&anon_vma->refcount)) > + if (refcount_dec_and_test(&anon_vma->refcount)) > __put_anon_vma(anon_vma); > } > > diff --git a/mm/rmap.c b/mm/rmap.c > index 8774791..3321c86 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -77,7 +77,7 @@ static inline struct anon_vma *anon_vma_alloc(void) > > anon_vma = kmem_cache_alloc(anon_vma_cachep, GFP_KERNEL); > if (anon_vma) { > - atomic_set(&anon_vma->refcount, 1); > + refcount_set(&anon_vma->refcount, 1); > anon_vma->degree = 1; /* Reference for first vma */ > anon_vma->parent = anon_vma; > /* > @@ -92,7 +92,7 @@ static inline struct anon_vma *anon_vma_alloc(void) > > static inline void anon_vma_free(struct anon_vma *anon_vma) > { > - VM_BUG_ON(atomic_read(&anon_vma->refcount)); > + VM_BUG_ON(refcount_read(&anon_vma->refcount)); > > /* > * Synchronize against page_lock_anon_vma_read() such that > @@ -421,7 +421,7 @@ static void anon_vma_ctor(void *data) > struct anon_vma *anon_vma = data; > > init_rwsem(&anon_vma->rwsem); > - atomic_set(&anon_vma->refcount, 0); > + refcount_set(&anon_vma->refcount, 0); > anon_vma->rb_root = RB_ROOT; > } > > @@ -470,7 +470,7 @@ struct anon_vma *page_get_anon_vma(struct page *page) > goto out; > > anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON); > - if (!atomic_inc_not_zero(&anon_vma->refcount)) { > + if (!refcount_inc_not_zero(&anon_vma->refcount)) { > anon_vma = NULL; > goto out; > } > @@ -529,7 +529,7 @@ struct anon_vma *page_lock_anon_vma_read(struct page *page) > } > > /* trylock failed, we got to sleep */ > - if (!atomic_inc_not_zero(&anon_vma->refcount)) { > + if (!refcount_inc_not_zero(&anon_vma->refcount)) { > anon_vma = NULL; > goto out; > } > @@ -544,7 +544,7 @@ struct anon_vma *page_lock_anon_vma_read(struct page *page) > rcu_read_unlock(); > anon_vma_lock_read(anon_vma); > > - if (atomic_dec_and_test(&anon_vma->refcount)) { > + if (refcount_dec_and_test(&anon_vma->refcount)) { > /* > * Oops, we held the last refcount, release the lock > * and bail -- can't simply use put_anon_vma() because > @@ -1577,7 +1577,7 @@ void __put_anon_vma(struct anon_vma *anon_vma) > struct anon_vma *root = anon_vma->root; > > anon_vma_free(anon_vma); > - if (root != anon_vma && atomic_dec_and_test(&root->refcount)) > + if (root != anon_vma && refcount_dec_and_test(&root->refcount)) > anon_vma_free(root); > } > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/5] mm: convert anon_vma.refcount from atomic_t to refcount_t 2017-04-22 10:44 ` zhong jiang @ 2017-04-24 10:20 ` Reshetova, Elena 0 siblings, 0 replies; 13+ messages in thread From: Reshetova, Elena @ 2017-04-24 10:20 UTC (permalink / raw) To: zhong jiang Cc: linux-kernel, linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, akpm, arnd, luto, Hans Liljestrand, Kees Cook, David Windsor > Hi, Elean > > Do the issue had really occured, use-after-free. but why the patch > is not received. or is is possible for the situation. Hi Zhongjiang, We have had such issues happening and being exploited in other places of kernel in the past. Therefore the intention with this and all other similar patches is to prevent the whole classes of bugs that might come even somewhere in the future by converting the "true" refcounters to a safe new type refcount_t. That makes sure if you ever have a bug in your code (now or in the future), it cannot be misused by attackers. So you can look at it as a safety net for your code. Best Regards, Elena. > > Thanks > zhongjiang > On 2017/2/20 18:49, 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> > > --- > > include/linux/rmap.h | 7 ++++--- > > mm/rmap.c | 14 +++++++------- > > 2 files changed, 11 insertions(+), 10 deletions(-) > > > > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > > index 8c89e90..a8f4a97 100644 > > --- a/include/linux/rmap.h > > +++ b/include/linux/rmap.h > > @@ -10,6 +10,7 @@ > > #include <linux/rwsem.h> > > #include <linux/memcontrol.h> > > #include <linux/highmem.h> > > +#include <linux/refcount.h> > > > > /* > > * The anon_vma heads a list of private "related" vmas, to scan if > > @@ -35,7 +36,7 @@ struct anon_vma { > > * the reference is responsible for clearing up the > > * anon_vma if they are the last user on release > > */ > > - atomic_t refcount; > > + refcount_t refcount; > > > > /* > > * Count of child anon_vmas and VMAs which points to this > anon_vma. > > @@ -102,14 +103,14 @@ enum ttu_flags { > > #ifdef CONFIG_MMU > > static inline void get_anon_vma(struct anon_vma *anon_vma) > > { > > - atomic_inc(&anon_vma->refcount); > > + refcount_inc(&anon_vma->refcount); > > } > > > > void __put_anon_vma(struct anon_vma *anon_vma); > > > > static inline void put_anon_vma(struct anon_vma *anon_vma) > > { > > - if (atomic_dec_and_test(&anon_vma->refcount)) > > + if (refcount_dec_and_test(&anon_vma->refcount)) > > __put_anon_vma(anon_vma); > > } > > > > diff --git a/mm/rmap.c b/mm/rmap.c > > index 8774791..3321c86 100644 > > --- a/mm/rmap.c > > +++ b/mm/rmap.c > > @@ -77,7 +77,7 @@ static inline struct anon_vma *anon_vma_alloc(void) > > > > anon_vma = kmem_cache_alloc(anon_vma_cachep, > GFP_KERNEL); > > if (anon_vma) { > > - atomic_set(&anon_vma->refcount, 1); > > + refcount_set(&anon_vma->refcount, 1); > > anon_vma->degree = 1; /* Reference for first > vma */ > > anon_vma->parent = anon_vma; > > /* > > @@ -92,7 +92,7 @@ static inline struct anon_vma *anon_vma_alloc(void) > > > > static inline void anon_vma_free(struct anon_vma *anon_vma) > > { > > - VM_BUG_ON(atomic_read(&anon_vma->refcount)); > > + VM_BUG_ON(refcount_read(&anon_vma->refcount)); > > > > /* > > * Synchronize against page_lock_anon_vma_read() such that > > @@ -421,7 +421,7 @@ static void anon_vma_ctor(void *data) > > struct anon_vma *anon_vma = data; > > > > init_rwsem(&anon_vma->rwsem); > > - atomic_set(&anon_vma->refcount, 0); > > + refcount_set(&anon_vma->refcount, 0); > > anon_vma->rb_root = RB_ROOT; > > } > > > > @@ -470,7 +470,7 @@ struct anon_vma *page_get_anon_vma(struct page > *page) > > goto out; > > > > anon_vma = (struct anon_vma *) (anon_mapping - > PAGE_MAPPING_ANON); > > - if (!atomic_inc_not_zero(&anon_vma->refcount)) { > > + if (!refcount_inc_not_zero(&anon_vma->refcount)) { > > anon_vma = NULL; > > goto out; > > } > > @@ -529,7 +529,7 @@ struct anon_vma *page_lock_anon_vma_read(struct > page *page) > > } > > > > /* trylock failed, we got to sleep */ > > - if (!atomic_inc_not_zero(&anon_vma->refcount)) { > > + if (!refcount_inc_not_zero(&anon_vma->refcount)) { > > anon_vma = NULL; > > goto out; > > } > > @@ -544,7 +544,7 @@ struct anon_vma *page_lock_anon_vma_read(struct > page *page) > > rcu_read_unlock(); > > anon_vma_lock_read(anon_vma); > > > > - if (atomic_dec_and_test(&anon_vma->refcount)) { > > + if (refcount_dec_and_test(&anon_vma->refcount)) { > > /* > > * Oops, we held the last refcount, release the lock > > * and bail -- can't simply use put_anon_vma() > because > > @@ -1577,7 +1577,7 @@ void __put_anon_vma(struct anon_vma *anon_vma) > > struct anon_vma *root = anon_vma->root; > > > > anon_vma_free(anon_vma); > > - if (root != anon_vma && atomic_dec_and_test(&root->refcount)) > > + if (root != anon_vma && refcount_dec_and_test(&root- > >refcount)) > > anon_vma_free(root); > > } > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/5] mm: convert kmemleak_object.use_count from atomic_t to refcount_t 2017-02-20 10:49 [PATCH 0/5] mm subsystem refcounter conversions Elena Reshetova 2017-02-20 10:49 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova 2017-02-20 10:49 ` [PATCH 2/5] mm: convert anon_vma.refcount " Elena Reshetova @ 2017-02-20 10:49 ` Elena Reshetova 2017-02-20 10:49 ` [PATCH 4/5] mm: convert mm_struct.mm_users " Elena Reshetova 2017-02-20 10:49 ` [PATCH 5/5] mm: convert mm_struct.mm_count " Elena Reshetova 4 siblings, 0 replies; 13+ messages in thread From: Elena Reshetova @ 2017-02-20 10:49 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, 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> --- mm/kmemleak.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/mm/kmemleak.c b/mm/kmemleak.c index da34369..2e1167b 100644 --- a/mm/kmemleak.c +++ b/mm/kmemleak.c @@ -105,7 +105,7 @@ #include <asm/sections.h> #include <asm/processor.h> -#include <linux/atomic.h> +#include <linux/refcount.h> #include <linux/kasan.h> #include <linux/kmemcheck.h> @@ -154,7 +154,7 @@ struct kmemleak_object { struct rb_node rb_node; struct rcu_head rcu; /* object_list lockless traversal */ /* object usage count; object freed when use_count == 0 */ - atomic_t use_count; + refcount_t use_count; unsigned long pointer; size_t size; /* minimum number of a pointers found before it is considered leak */ @@ -434,7 +434,7 @@ static struct kmemleak_object *lookup_object(unsigned long ptr, int alias) */ static int get_object(struct kmemleak_object *object) { - return atomic_inc_not_zero(&object->use_count); + return refcount_inc_not_zero(&object->use_count); } /* @@ -467,7 +467,7 @@ static void free_object_rcu(struct rcu_head *rcu) */ static void put_object(struct kmemleak_object *object) { - if (!atomic_dec_and_test(&object->use_count)) + if (!refcount_dec_and_test(&object->use_count)) return; /* should only get here after delete_object was called */ @@ -556,7 +556,7 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size, INIT_LIST_HEAD(&object->gray_list); INIT_HLIST_HEAD(&object->area_list); spin_lock_init(&object->lock); - atomic_set(&object->use_count, 1); + refcount_set(&object->use_count, 1); object->flags = OBJECT_ALLOCATED; object->pointer = ptr; object->size = size; @@ -629,7 +629,7 @@ static void __delete_object(struct kmemleak_object *object) unsigned long flags; WARN_ON(!(object->flags & OBJECT_ALLOCATED)); - WARN_ON(atomic_read(&object->use_count) < 1); + WARN_ON(refcount_read(&object->use_count) < 1); /* * Locking here also ensures that the corresponding memory block @@ -1396,9 +1396,9 @@ static void kmemleak_scan(void) * With a few exceptions there should be a maximum of * 1 reference to any object at this point. */ - if (atomic_read(&object->use_count) > 1) { + if (refcount_read(&object->use_count) > 1) { pr_debug("object->use_count = %d\n", - atomic_read(&object->use_count)); + refcount_read(&object->use_count)); dump_object_info(object); } #endif -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/5] mm: convert mm_struct.mm_users from atomic_t to refcount_t 2017-02-20 10:49 [PATCH 0/5] mm subsystem refcounter conversions Elena Reshetova ` (2 preceding siblings ...) 2017-02-20 10:49 ` [PATCH 3/5] mm: convert kmemleak_object.use_count " Elena Reshetova @ 2017-02-20 10:49 ` Elena Reshetova 2017-02-20 11:46 ` kbuild test robot 2017-02-20 10:49 ` [PATCH 5/5] mm: convert mm_struct.mm_count " Elena Reshetova 4 siblings, 1 reply; 13+ messages in thread From: Elena Reshetova @ 2017-02-20 10:49 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, 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> --- arch/alpha/kernel/smp.c | 6 +++--- arch/arc/mm/tlb.c | 2 +- arch/blackfin/mach-common/smp.c | 2 +- arch/ia64/include/asm/tlbflush.h | 2 +- arch/ia64/kernel/smp.c | 2 +- arch/ia64/sn/kernel/sn2/sn2_smp.c | 4 ++-- arch/mips/kernel/process.c | 2 +- arch/mips/kernel/smp.c | 6 +++--- arch/parisc/include/asm/mmu_context.h | 2 +- arch/powerpc/mm/hugetlbpage.c | 2 +- arch/powerpc/mm/icswx.c | 4 ++-- arch/sh/kernel/smp.c | 6 +++--- arch/sparc/kernel/smp_64.c | 6 +++--- arch/sparc/mm/srmmu.c | 2 +- arch/um/kernel/tlb.c | 2 +- arch/x86/kernel/tboot.c | 2 +- arch/xtensa/kernel/smp.c | 5 +++++ drivers/firmware/efi/arm-runtime.c | 2 +- fs/coredump.c | 2 +- fs/proc/base.c | 10 +++++++++- include/linux/mm_types.h | 3 ++- include/linux/sched.h | 4 ++-- kernel/events/uprobes.c | 2 +- kernel/exit.c | 2 +- kernel/fork.c | 10 +++++----- kernel/sched/core.c | 2 +- lib/is_single_threaded.c | 2 +- mm/debug.c | 2 +- mm/init-mm.c | 2 +- mm/khugepaged.c | 2 +- mm/ksm.c | 2 +- mm/memory.c | 2 +- mm/mmu_notifier.c | 4 ++-- mm/mprotect.c | 2 +- mm/oom_kill.c | 2 +- mm/swapfile.c | 2 +- mm/vmacache.c | 2 +- 37 files changed, 66 insertions(+), 52 deletions(-) diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c index acb4b14..c4a82f0 100644 --- a/arch/alpha/kernel/smp.c +++ b/arch/alpha/kernel/smp.c @@ -653,7 +653,7 @@ flush_tlb_mm(struct mm_struct *mm) if (mm == current->active_mm) { flush_tlb_current(mm); - if (atomic_read(&mm->mm_users) <= 1) { + if (refcount_read(&mm->mm_users) <= 1) { int cpu, this_cpu = smp_processor_id(); for (cpu = 0; cpu < NR_CPUS; cpu++) { if (!cpu_online(cpu) || cpu == this_cpu) @@ -702,7 +702,7 @@ flush_tlb_page(struct vm_area_struct *vma, unsigned long addr) if (mm == current->active_mm) { flush_tlb_current_page(mm, vma, addr); - if (atomic_read(&mm->mm_users) <= 1) { + if (refcount_read(&mm->mm_users) <= 1) { int cpu, this_cpu = smp_processor_id(); for (cpu = 0; cpu < NR_CPUS; cpu++) { if (!cpu_online(cpu) || cpu == this_cpu) @@ -758,7 +758,7 @@ flush_icache_user_range(struct vm_area_struct *vma, struct page *page, if (mm == current->active_mm) { __load_new_mm_context(mm); - if (atomic_read(&mm->mm_users) <= 1) { + if (refcount_read(&mm->mm_users) <= 1) { int cpu, this_cpu = smp_processor_id(); for (cpu = 0; cpu < NR_CPUS; cpu++) { if (!cpu_online(cpu) || cpu == this_cpu) diff --git a/arch/arc/mm/tlb.c b/arch/arc/mm/tlb.c index bdb295e..6dbdfe7 100644 --- a/arch/arc/mm/tlb.c +++ b/arch/arc/mm/tlb.c @@ -297,7 +297,7 @@ noinline void local_flush_tlb_mm(struct mm_struct *mm) * Only for fork( ) do we need to move parent to a new MMU ctxt, * all other cases are NOPs, hence this check. */ - if (atomic_read(&mm->mm_users) == 0) + if (refcount_read(&mm->mm_users) == 0) return; /* diff --git a/arch/blackfin/mach-common/smp.c b/arch/blackfin/mach-common/smp.c index a2e6db2..bab73d2b 100644 --- a/arch/blackfin/mach-common/smp.c +++ b/arch/blackfin/mach-common/smp.c @@ -422,7 +422,7 @@ void cpu_die(void) { (void)cpu_report_death(); - atomic_dec(&init_mm.mm_users); + refcount_dec(&init_mm.mm_users); atomic_dec(&init_mm.mm_count); local_irq_disable(); diff --git a/arch/ia64/include/asm/tlbflush.h b/arch/ia64/include/asm/tlbflush.h index 3be25df..650708a 100644 --- a/arch/ia64/include/asm/tlbflush.h +++ b/arch/ia64/include/asm/tlbflush.h @@ -56,7 +56,7 @@ flush_tlb_mm (struct mm_struct *mm) set_bit(mm->context, ia64_ctx.flushmap); mm->context = 0; - if (atomic_read(&mm->mm_users) == 0) + if (refcount_read(&mm->mm_users) == 0) return; /* happens as a result of exit_mmap() */ #ifdef CONFIG_SMP diff --git a/arch/ia64/kernel/smp.c b/arch/ia64/kernel/smp.c index 7f706d4..dd7b680 100644 --- a/arch/ia64/kernel/smp.c +++ b/arch/ia64/kernel/smp.c @@ -295,7 +295,7 @@ smp_flush_tlb_mm (struct mm_struct *mm) cpumask_var_t cpus; preempt_disable(); /* this happens for the common case of a single-threaded fork(): */ - if (likely(mm == current->active_mm && atomic_read(&mm->mm_users) == 1)) + if (likely(mm == current->active_mm && refcount_read(&mm->mm_users) == 1)) { local_finish_flush_tlb_mm(mm); preempt_enable(); diff --git a/arch/ia64/sn/kernel/sn2/sn2_smp.c b/arch/ia64/sn/kernel/sn2/sn2_smp.c index c98dc96..1c801b3 100644 --- a/arch/ia64/sn/kernel/sn2/sn2_smp.c +++ b/arch/ia64/sn/kernel/sn2/sn2_smp.c @@ -122,7 +122,7 @@ void sn_migrate(struct task_struct *task) void sn_tlb_migrate_finish(struct mm_struct *mm) { /* flush_tlb_mm is inefficient if more than 1 users of mm */ - if (mm == current->mm && mm && atomic_read(&mm->mm_users) == 1) + if (mm == current->mm && mm && refcount_read(&mm->mm_users) == 1) flush_tlb_mm(mm); } @@ -204,7 +204,7 @@ sn2_global_tlb_purge(struct mm_struct *mm, unsigned long start, return; } - if (atomic_read(&mm->mm_users) == 1 && mymm) { + if (refcount_read(&mm->mm_users) == 1 && mymm) { flush_tlb_mm(mm); __this_cpu_inc(ptcstats.change_rid); preempt_enable(); diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c index 803e255..33fa000 100644 --- a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@ -698,7 +698,7 @@ int mips_set_process_fp_mode(struct task_struct *task, unsigned int value) /* No need to send an IPI for the local CPU */ max_users = (task->mm == current->mm) ? 1 : 0; - if (atomic_read(¤t->mm->mm_users) > max_users) + if (refcount_read(¤t->mm->mm_users) > max_users) smp_call_function(prepare_for_fp_mode_switch, (void *)current->mm, 1); } diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c index 8c60a29..e62aa56 100644 --- a/arch/mips/kernel/smp.c +++ b/arch/mips/kernel/smp.c @@ -511,7 +511,7 @@ void flush_tlb_mm(struct mm_struct *mm) { preempt_disable(); - if ((atomic_read(&mm->mm_users) != 1) || (current->mm != mm)) { + if ((refcount_read(&mm->mm_users) != 1) || (current->mm != mm)) { smp_on_other_tlbs(flush_tlb_mm_ipi, mm); } else { unsigned int cpu; @@ -544,7 +544,7 @@ void flush_tlb_range(struct vm_area_struct *vma, unsigned long start, unsigned l struct mm_struct *mm = vma->vm_mm; preempt_disable(); - if ((atomic_read(&mm->mm_users) != 1) || (current->mm != mm)) { + if ((refcount_read(&mm->mm_users) != 1) || (current->mm != mm)) { struct flush_tlb_data fd = { .vma = vma, .addr1 = start, @@ -598,7 +598,7 @@ static void flush_tlb_page_ipi(void *info) void flush_tlb_page(struct vm_area_struct *vma, unsigned long page) { preempt_disable(); - if ((atomic_read(&vma->vm_mm->mm_users) != 1) || (current->mm != vma->vm_mm)) { + if ((refcount_read(&vma->vm_mm->mm_users) != 1) || (current->mm != vma->vm_mm)) { struct flush_tlb_data fd = { .vma = vma, .addr1 = page, diff --git a/arch/parisc/include/asm/mmu_context.h b/arch/parisc/include/asm/mmu_context.h index 59be257..e64f398 100644 --- a/arch/parisc/include/asm/mmu_context.h +++ b/arch/parisc/include/asm/mmu_context.h @@ -21,7 +21,7 @@ extern void free_sid(unsigned long); static inline int init_new_context(struct task_struct *tsk, struct mm_struct *mm) { - BUG_ON(atomic_read(&mm->mm_users) != 1); + BUG_ON(refcount_read(&mm->mm_users) != 1); mm->context = alloc_sid(); return 0; diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c index 8c3389c..26aef24 100644 --- a/arch/powerpc/mm/hugetlbpage.c +++ b/arch/powerpc/mm/hugetlbpage.c @@ -403,7 +403,7 @@ static void hugepd_free(struct mmu_gather *tlb, void *hugepte) batchp = &get_cpu_var(hugepd_freelist_cur); - if (atomic_read(&tlb->mm->mm_users) < 2 || + if (refcount_read(&tlb->mm->mm_users) < 2 || cpumask_equal(mm_cpumask(tlb->mm), cpumask_of(smp_processor_id()))) { kmem_cache_free(hugepte_cache, hugepte); diff --git a/arch/powerpc/mm/icswx.c b/arch/powerpc/mm/icswx.c index 915412e..2406ff8 100644 --- a/arch/powerpc/mm/icswx.c +++ b/arch/powerpc/mm/icswx.c @@ -110,7 +110,7 @@ int use_cop(unsigned long acop, struct mm_struct *mm) * running. We need to send an IPI to force them to pick up any * change in PID and ACOP. */ - if (atomic_read(&mm->mm_users) > 1) + if (refcount_read(&mm->mm_users) > 1) smp_call_function(sync_cop, mm, 1); out: @@ -150,7 +150,7 @@ void drop_cop(unsigned long acop, struct mm_struct *mm) * running. We need to send an IPI to force them to pick up any * change in PID and ACOP. */ - if (atomic_read(&mm->mm_users) > 1) + if (refcount_read(&mm->mm_users) > 1) smp_call_function(sync_cop, mm, 1); if (free_pid != COP_PID_NONE) diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c index edc4769..9d678bb 100644 --- a/arch/sh/kernel/smp.c +++ b/arch/sh/kernel/smp.c @@ -363,7 +363,7 @@ void flush_tlb_mm(struct mm_struct *mm) { preempt_disable(); - if ((atomic_read(&mm->mm_users) != 1) || (current->mm != mm)) { + if ((refcount_read(&mm->mm_users) != 1) || (current->mm != mm)) { smp_call_function(flush_tlb_mm_ipi, (void *)mm, 1); } else { int i; @@ -395,7 +395,7 @@ void flush_tlb_range(struct vm_area_struct *vma, struct mm_struct *mm = vma->vm_mm; preempt_disable(); - if ((atomic_read(&mm->mm_users) != 1) || (current->mm != mm)) { + if ((refcount_read(&mm->mm_users) != 1) || (current->mm != mm)) { struct flush_tlb_data fd; fd.vma = vma; @@ -438,7 +438,7 @@ static void flush_tlb_page_ipi(void *info) void flush_tlb_page(struct vm_area_struct *vma, unsigned long page) { preempt_disable(); - if ((atomic_read(&vma->vm_mm->mm_users) != 1) || + if ((refcount_read(&vma->vm_mm->mm_users) != 1) || (current->mm != vma->vm_mm)) { struct flush_tlb_data fd; diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c index dcb12d9..bd5e56c 100644 --- a/arch/sparc/kernel/smp_64.c +++ b/arch/sparc/kernel/smp_64.c @@ -1063,7 +1063,7 @@ void smp_flush_tlb_mm(struct mm_struct *mm) u32 ctx = CTX_HWBITS(mm->context); int cpu = get_cpu(); - if (atomic_read(&mm->mm_users) == 1) { + if (refcount_read(&mm->mm_users) == 1) { cpumask_copy(mm_cpumask(mm), cpumask_of(cpu)); goto local_flush_and_out; } @@ -1101,7 +1101,7 @@ void smp_flush_tlb_pending(struct mm_struct *mm, unsigned long nr, unsigned long info.nr = nr; info.vaddrs = vaddrs; - if (mm == current->mm && atomic_read(&mm->mm_users) == 1) + if (mm == current->mm && refcount_read(&mm->mm_users) == 1) cpumask_copy(mm_cpumask(mm), cpumask_of(cpu)); else smp_call_function_many(mm_cpumask(mm), tlb_pending_func, @@ -1117,7 +1117,7 @@ void smp_flush_tlb_page(struct mm_struct *mm, unsigned long vaddr) unsigned long context = CTX_HWBITS(mm->context); int cpu = get_cpu(); - if (mm == current->mm && atomic_read(&mm->mm_users) == 1) + if (mm == current->mm && refcount_read(&mm->mm_users) == 1) cpumask_copy(mm_cpumask(mm), cpumask_of(cpu)); else smp_cross_call_masked(&xcall_flush_tlb_page, diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c index c7f2a52..17941a8 100644 --- a/arch/sparc/mm/srmmu.c +++ b/arch/sparc/mm/srmmu.c @@ -1662,7 +1662,7 @@ static void smp_flush_tlb_mm(struct mm_struct *mm) cpumask_clear_cpu(smp_processor_id(), &cpu_mask); if (!cpumask_empty(&cpu_mask)) { xc1((smpfunc_t) local_ops->tlb_mm, (unsigned long) mm); - if (atomic_read(&mm->mm_users) == 1 && current->active_mm == mm) + if (refcount_read(&mm->mm_users) == 1 && current->active_mm == mm) cpumask_copy(mm_cpumask(mm), cpumask_of(smp_processor_id())); } diff --git a/arch/um/kernel/tlb.c b/arch/um/kernel/tlb.c index 3777b82..1da0463 100644 --- a/arch/um/kernel/tlb.c +++ b/arch/um/kernel/tlb.c @@ -530,7 +530,7 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start, * Don't bother flushing if this address space is about to be * destroyed. */ - if (atomic_read(&mm->mm_users) == 0) + if (refcount_read(&mm->mm_users) == 0) return; fix_range(mm, start, end, 0); diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index b868fa1..39aaca5 100644 --- a/arch/x86/kernel/tboot.c +++ b/arch/x86/kernel/tboot.c @@ -102,7 +102,7 @@ static pgd_t *tboot_pg_dir; static struct mm_struct tboot_mm = { .mm_rb = RB_ROOT, .pgd = swapper_pg_dir, - .mm_users = ATOMIC_INIT(2), + .mm_users = REFCOUNT_INIT(2), .mm_count = ATOMIC_INIT(1), .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), diff --git a/arch/xtensa/kernel/smp.c b/arch/xtensa/kernel/smp.c index fcea720..c2b3067 100644 --- a/arch/xtensa/kernel/smp.c +++ b/arch/xtensa/kernel/smp.c @@ -135,8 +135,13 @@ void secondary_start_kernel(void) /* All kernel threads share the same mm context. */ +<<<<<<< e5c2d109eae88b7c203d396dfaeb284f95f6ffe5 mmget(mm); mmgrab(mm); +======= + refcount_inc(&mm->mm_users); + atomic_inc(&mm->mm_count); +>>>>>>> mm: convert mm_struct.mm_users from atomic_t to refcount_t current->active_mm = mm; cpumask_set_cpu(cpu, mm_cpumask(mm)); enter_lazy_tlb(mm, current); diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c index 349dc3e..23e41f9 100644 --- a/drivers/firmware/efi/arm-runtime.c +++ b/drivers/firmware/efi/arm-runtime.c @@ -32,7 +32,7 @@ extern u64 efi_system_table; static struct mm_struct efi_mm = { .mm_rb = RB_ROOT, - .mm_users = ATOMIC_INIT(2), + .mm_users = REFCOUNT_INIT(2), .mm_count = ATOMIC_INIT(1), .mmap_sem = __RWSEM_INITIALIZER(efi_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(efi_mm.page_table_lock), diff --git a/fs/coredump.c b/fs/coredump.c index ae6b056..05e43f1 100644 --- a/fs/coredump.c +++ b/fs/coredump.c @@ -347,7 +347,7 @@ static int zap_threads(struct task_struct *tsk, struct mm_struct *mm, return nr; tsk->flags |= PF_DUMPCORE; - if (atomic_read(&mm->mm_users) == nr + 1) + if (refcount_read(&mm->mm_users) == nr + 1) goto done; /* * We should find and kill all tasks which use this mm, and we should diff --git a/fs/proc/base.c b/fs/proc/base.c index 6e86558..a75dca3 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -921,7 +921,11 @@ static ssize_t environ_read(struct file *file, char __user *buf, return -ENOMEM; ret = 0; +<<<<<<< e5c2d109eae88b7c203d396dfaeb284f95f6ffe5 if (!mmget_not_zero(mm)) +======= + if (!refcount_inc_not_zero(&mm->mm_users)) +>>>>>>> mm: convert mm_struct.mm_users from atomic_t to refcount_t goto free; down_read(&mm->mmap_sem); @@ -1062,9 +1066,13 @@ static int __set_oom_adj(struct file *file, int oom_adj, bool legacy) struct task_struct *p = find_lock_task_mm(task); if (p) { - if (atomic_read(&p->mm->mm_users) > 1) { + if (refcount_read(&p->mm->mm_users) > 1) { mm = p->mm; +<<<<<<< e5c2d109eae88b7c203d396dfaeb284f95f6ffe5 mmgrab(mm); +======= + refcount_inc(&mm->mm_count); +>>>>>>> mm: convert mm_struct.mm_users from atomic_t to refcount_t } task_unlock(p); } diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 4f6d440..af260d6 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -7,6 +7,7 @@ #include <linux/list.h> #include <linux/spinlock.h> #include <linux/rbtree.h> +#include <linux/refcount.h> #include <linux/rwsem.h> #include <linux/completion.h> #include <linux/cpumask.h> @@ -417,7 +418,7 @@ struct mm_struct { * (which may then free the &struct mm_struct if @mm_count also * drops to 0). */ - atomic_t mm_users; + refcount_t mm_users; /** * @mm_count: The number of references to &struct mm_struct diff --git a/include/linux/sched.h b/include/linux/sched.h index affcd93..c21682c 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -2967,12 +2967,12 @@ static inline void mmdrop_async(struct mm_struct *mm) */ static inline void mmget(struct mm_struct *mm) { - atomic_inc(&mm->mm_users); + refcount_inc(&mm->mm_users); } static inline bool mmget_not_zero(struct mm_struct *mm) { - return atomic_inc_not_zero(&mm->mm_users); + return refcount_inc_not_zero(&mm->mm_users); } /* mmput gets rid of the mappings and all user-space */ diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 95e42f0..a4b33f8 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -1121,7 +1121,7 @@ void uprobe_munmap(struct vm_area_struct *vma, unsigned long start, unsigned lon if (no_uprobe_events() || !valid_vma(vma, false)) return; - if (!atomic_read(&vma->vm_mm->mm_users)) /* called by mmput() ? */ + if (!refcount_read(&vma->vm_mm->mm_users)) /* called by mmput() ? */ return; if (!test_bit(MMF_HAS_UPROBES, &vma->vm_mm->flags) || diff --git a/kernel/exit.c b/kernel/exit.c index 8a768a3..261305d 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -426,7 +426,7 @@ void mm_update_next_owner(struct mm_struct *mm) * candidates. Do not leave the mm pointing to a possibly * freed task structure. */ - if (atomic_read(&mm->mm_users) <= 1) { + if (refcount_read(&mm->mm_users) <= 1) { mm->owner = NULL; return; } diff --git a/kernel/fork.c b/kernel/fork.c index 0e096fc..60ff801 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -759,7 +759,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p, mm->mmap = NULL; mm->mm_rb = RB_ROOT; mm->vmacache_seqnum = 0; - atomic_set(&mm->mm_users, 1); + refcount_set(&mm->mm_users, 1); atomic_set(&mm->mm_count, 1); init_rwsem(&mm->mmap_sem); INIT_LIST_HEAD(&mm->mmlist); @@ -862,7 +862,7 @@ EXPORT_SYMBOL_GPL(__mmdrop); static inline void __mmput(struct mm_struct *mm) { - VM_BUG_ON(atomic_read(&mm->mm_users)); + VM_BUG_ON(refcount_read(&mm->mm_users)); uprobe_clear_state(mm); exit_aio(mm); @@ -889,7 +889,7 @@ void mmput(struct mm_struct *mm) { might_sleep(); - if (atomic_dec_and_test(&mm->mm_users)) + if (refcount_dec_and_test(&mm->mm_users)) __mmput(mm); } EXPORT_SYMBOL_GPL(mmput); @@ -903,7 +903,7 @@ static void mmput_async_fn(struct work_struct *work) void mmput_async(struct mm_struct *mm) { - if (atomic_dec_and_test(&mm->mm_users)) { + if (refcount_dec_and_test(&mm->mm_users)) { INIT_WORK(&mm->async_put_work, mmput_async_fn); schedule_work(&mm->async_put_work); } @@ -1102,7 +1102,7 @@ void mm_release(struct task_struct *tsk, struct mm_struct *mm) */ if (tsk->clear_child_tid) { if (!(tsk->signal->flags & SIGNAL_GROUP_COREDUMP) && - atomic_read(&mm->mm_users) > 1) { + refcount_read(&mm->mm_users) > 1) { /* * We don't check the error code - if userspace has * not set up a proper pointer then tough luck. diff --git a/kernel/sched/core.c b/kernel/sched/core.c index c7ded33..324ea09 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2196,7 +2196,7 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p) #endif #ifdef CONFIG_NUMA_BALANCING - if (p->mm && atomic_read(&p->mm->mm_users) == 1) { + if (p->mm && refcount_read(&p->mm->mm_users) == 1) { p->mm->numa_next_scan = jiffies + msecs_to_jiffies(sysctl_numa_balancing_scan_delay); p->mm->numa_scan_seq = 0; } diff --git a/lib/is_single_threaded.c b/lib/is_single_threaded.c index 391fd23..295ddcf 100644 --- a/lib/is_single_threaded.c +++ b/lib/is_single_threaded.c @@ -25,7 +25,7 @@ bool current_is_single_threaded(void) if (atomic_read(&task->signal->live) != 1) return false; - if (atomic_read(&mm->mm_users) == 1) + if (refcount_read(&mm->mm_users) == 1) return true; ret = false; diff --git a/mm/debug.c b/mm/debug.c index db1cd26..0866505 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -134,7 +134,7 @@ void dump_mm(const struct mm_struct *mm) mm->get_unmapped_area, #endif mm->mmap_base, mm->mmap_legacy_base, mm->highest_vm_end, - mm->pgd, atomic_read(&mm->mm_users), + mm->pgd, refcount_read(&mm->mm_users), atomic_read(&mm->mm_count), atomic_long_read((atomic_long_t *)&mm->nr_ptes), mm_nr_pmds((struct mm_struct *)mm), diff --git a/mm/init-mm.c b/mm/init-mm.c index 975e49f..6927a72 100644 --- a/mm/init-mm.c +++ b/mm/init-mm.c @@ -17,7 +17,7 @@ struct mm_struct init_mm = { .mm_rb = RB_ROOT, .pgd = swapper_pg_dir, - .mm_users = ATOMIC_INIT(2), + .mm_users = REFCOUNT_INIT(2), .mm_count = ATOMIC_INIT(1), .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 34bce5c..e7c11a6 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -391,7 +391,7 @@ static void insert_to_mm_slots_hash(struct mm_struct *mm, static inline int khugepaged_test_exit(struct mm_struct *mm) { - return atomic_read(&mm->mm_users) == 0; + return refcount_read(&mm->mm_users) == 0; } int __khugepaged_enter(struct mm_struct *mm) diff --git a/mm/ksm.c b/mm/ksm.c index 2e129f0..6152465 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -358,7 +358,7 @@ static void insert_to_mm_slots_hash(struct mm_struct *mm, */ static inline bool ksm_test_exit(struct mm_struct *mm) { - return atomic_read(&mm->mm_users) == 0; + return refcount_read(&mm->mm_users) == 0; } /* diff --git a/mm/memory.c b/mm/memory.c index 0c759ba..cc93a22 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -374,7 +374,7 @@ void tlb_remove_table(struct mmu_gather *tlb, void *table) * When there's less then two users of this mm there cannot be a * concurrent page-table walk. */ - if (atomic_read(&tlb->mm->mm_users) < 2) { + if (refcount_read(&tlb->mm->mm_users) < 2) { __tlb_remove_table(table); return; } diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c index 32bc9f2..d8c11e1 100644 --- a/mm/mmu_notifier.c +++ b/mm/mmu_notifier.c @@ -249,7 +249,7 @@ static int do_mmu_notifier_register(struct mmu_notifier *mn, struct mmu_notifier_mm *mmu_notifier_mm; int ret; - BUG_ON(atomic_read(&mm->mm_users) <= 0); + BUG_ON(refcount_read(&mm->mm_users) <= 0); /* * Verify that mmu_notifier_init() already run and the global srcu is @@ -295,7 +295,7 @@ static int do_mmu_notifier_register(struct mmu_notifier *mn, up_write(&mm->mmap_sem); kfree(mmu_notifier_mm); out: - BUG_ON(atomic_read(&mm->mm_users) <= 0); + BUG_ON(refcount_read(&mm->mm_users) <= 0); return ret; } diff --git a/mm/mprotect.c b/mm/mprotect.c index 77115bb..f482ce9 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -63,7 +63,7 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd, /* Get target node for single threaded private VMAs */ if (prot_numa && !(vma->vm_flags & VM_SHARED) && - atomic_read(&vma->vm_mm->mm_users) == 1) + refcount_read(&vma->vm_mm->mm_users) == 1) target_node = numa_node_id(); arch_enter_lazy_mmu_mode(); diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 51c0918..cc0348d 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -774,7 +774,7 @@ static bool task_will_free_mem(struct task_struct *task) if (test_bit(MMF_OOM_SKIP, &mm->flags)) return false; - if (atomic_read(&mm->mm_users) <= 1) + if (refcount_read(&mm->mm_users) <= 1) return true; /* diff --git a/mm/swapfile.c b/mm/swapfile.c index 5ac2cb4..be949e4 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -1722,7 +1722,7 @@ int try_to_unuse(unsigned int type, bool frontswap, /* * Don't hold on to start_mm if it looks like exiting. */ - if (atomic_read(&start_mm->mm_users) == 1) { + if (refcount_read(&start_mm->mm_users) == 1) { mmput(start_mm); start_mm = &init_mm; mmget(&init_mm); diff --git a/mm/vmacache.c b/mm/vmacache.c index 035fdeb..4747ee6 100644 --- a/mm/vmacache.c +++ b/mm/vmacache.c @@ -26,7 +26,7 @@ void vmacache_flush_all(struct mm_struct *mm) * to worry about other threads' seqnum. Current's * flush will occur upon the next lookup. */ - if (atomic_read(&mm->mm_users) == 1) + if (refcount_read(&mm->mm_users) == 1) return; rcu_read_lock(); -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/5] mm: convert mm_struct.mm_users from atomic_t to refcount_t 2017-02-20 10:49 ` [PATCH 4/5] mm: convert mm_struct.mm_users " Elena Reshetova @ 2017-02-20 11:46 ` kbuild test robot 0 siblings, 0 replies; 13+ messages in thread From: kbuild test robot @ 2017-02-20 11:46 UTC (permalink / raw) To: Elena Reshetova Cc: kbuild-all, linux-kernel, linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook, David Windsor [-- Attachment #1: Type: text/plain, Size: 2036 bytes --] Hi Elena, [auto build test ERROR on mmotm/master] [also build test ERROR on next-20170220] [cannot apply to linus/master linux/master v4.10] [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/mm-subsystem-refcounter-conversions/20170220-190351 base: git://git.cmpxchg.org/linux-mmotm.git master config: i386-randconfig-x011-201708 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 Note: the linux-review/Elena-Reshetova/mm-subsystem-refcounter-conversions/20170220-190351 HEAD 3b4bd307e7ad16dfd7fa504e7dbd951598fcc757 builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): fs/proc/base.c: In function 'environ_read': >> fs/proc/base.c:924:1: error: version control conflict marker in file <<<<<<< e5c2d109eae88b7c203d396dfaeb284f95f6ffe5 ^~~~~~~ fs/proc/base.c:968:1: warning: label 'free' defined but not used [-Wunused-label] free: ^~~~ fs/proc/base.c: In function '__set_oom_adj': fs/proc/base.c:1071:1: error: version control conflict marker in file <<<<<<< e5c2d109eae88b7c203d396dfaeb284f95f6ffe5 ^~~~~~~ fs/proc/base.c:1073:1: error: version control conflict marker in file ======= ^~~~~~~ fs/proc/base.c:1075:1: error: version control conflict marker in file >>>>>>> mm: convert mm_struct.mm_users from atomic_t to refcount_t ^~~~~~~ vim +924 fs/proc/base.c 918 919 page = (char *)__get_free_page(GFP_TEMPORARY); 920 if (!page) 921 return -ENOMEM; 922 923 ret = 0; > 924 <<<<<<< e5c2d109eae88b7c203d396dfaeb284f95f6ffe5 925 if (!mmget_not_zero(mm)) 926 ======= 927 if (!refcount_inc_not_zero(&mm->mm_users)) --- 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: 29067 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/5] mm: convert mm_struct.mm_count from atomic_t to refcount_t 2017-02-20 10:49 [PATCH 0/5] mm subsystem refcounter conversions Elena Reshetova ` (3 preceding siblings ...) 2017-02-20 10:49 ` [PATCH 4/5] mm: convert mm_struct.mm_users " Elena Reshetova @ 2017-02-20 10:49 ` Elena Reshetova 2017-02-20 13:16 ` kbuild test robot 4 siblings, 1 reply; 13+ messages in thread From: Elena Reshetova @ 2017-02-20 10:49 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, 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> --- arch/x86/kernel/tboot.c | 2 +- drivers/firmware/efi/arm-runtime.c | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +- drivers/gpu/drm/i915/i915_gem_userptr.c | 1 - fs/proc/base.c | 8 -------- fs/userfaultfd.c | 3 +-- include/linux/mm_types.h | 2 +- include/linux/sched.h | 6 +++--- kernel/fork.c | 2 +- mm/debug.c | 2 +- mm/init-mm.c | 2 +- mm/mmu_notifier.c | 6 +++--- 12 files changed, 14 insertions(+), 24 deletions(-) diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 39aaca5..fdbae72 100644 --- a/arch/x86/kernel/tboot.c +++ b/arch/x86/kernel/tboot.c @@ -103,7 +103,7 @@ static struct mm_struct tboot_mm = { .mm_rb = RB_ROOT, .pgd = swapper_pg_dir, .mm_users = REFCOUNT_INIT(2), - .mm_count = ATOMIC_INIT(1), + .mm_count = REFCOUNT_INIT(1), .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), .mmlist = LIST_HEAD_INIT(init_mm.mmlist), diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c index 23e41f9..f0571f2 100644 --- a/drivers/firmware/efi/arm-runtime.c +++ b/drivers/firmware/efi/arm-runtime.c @@ -33,7 +33,7 @@ extern u64 efi_system_table; static struct mm_struct efi_mm = { .mm_rb = RB_ROOT, .mm_users = REFCOUNT_INIT(2), - .mm_count = ATOMIC_INIT(1), + .mm_count = REFCOUNT_INIT(1), .mmap_sem = __RWSEM_INITIALIZER(efi_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(efi_mm.page_table_lock), .mmlist = LIST_HEAD_INIT(efi_mm.mmlist), diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c index ca5f2aa..d86ceec 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c @@ -204,7 +204,7 @@ static void kfd_process_destroy_delayed(struct rcu_head *rcu) BUG_ON(!kfd_process_wq); p = container_of(rcu, struct kfd_process, rcu); - BUG_ON(atomic_read(&p->mm->mm_count) <= 0); + BUG_ON(refcount_read(&p->mm->mm_count) == 0); mmdrop(p->mm); diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index 0115989..68fb396 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -335,7 +335,6 @@ i915_gem_userptr_init__mm_struct(struct drm_i915_gem_object *obj) mm->mm = current->mm; mmgrab(current->mm); - mm->mn = NULL; /* Protected by dev_priv->mm_lock */ diff --git a/fs/proc/base.c b/fs/proc/base.c index a75dca3..445b259 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -921,11 +921,7 @@ static ssize_t environ_read(struct file *file, char __user *buf, return -ENOMEM; ret = 0; -<<<<<<< e5c2d109eae88b7c203d396dfaeb284f95f6ffe5 if (!mmget_not_zero(mm)) -======= - if (!refcount_inc_not_zero(&mm->mm_users)) ->>>>>>> mm: convert mm_struct.mm_users from atomic_t to refcount_t goto free; down_read(&mm->mmap_sem); @@ -1068,11 +1064,7 @@ static int __set_oom_adj(struct file *file, int oom_adj, bool legacy) if (p) { if (refcount_read(&p->mm->mm_users) > 1) { mm = p->mm; -<<<<<<< e5c2d109eae88b7c203d396dfaeb284f95f6ffe5 mmgrab(mm); -======= - refcount_inc(&mm->mm_count); ->>>>>>> mm: convert mm_struct.mm_users from atomic_t to refcount_t } task_unlock(p); } diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 3c421d0..1fd19ac 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -613,7 +613,7 @@ int dup_userfaultfd(struct vm_area_struct *vma, struct list_head *fcs) ctx->features = octx->features; ctx->released = false; ctx->mm = vma->vm_mm; - atomic_inc(&ctx->mm->mm_count); + mmgrab(ctx->mm); userfaultfd_ctx_get(octx); fctx->orig = octx; @@ -1848,7 +1848,6 @@ static struct file *userfaultfd_file_create(int flags) ctx->mm = current->mm; /* prevent the mm struct to be freed */ mmgrab(ctx->mm); - file = anon_inode_getfile("[userfaultfd]", &userfaultfd_fops, ctx, O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS)); if (IS_ERR(file)) { diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index af260d6..6445c58 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -427,7 +427,7 @@ struct mm_struct { * Use mmgrab()/mmdrop() to modify. When this drops to 0, the * &struct mm_struct is freed. */ - atomic_t mm_count; + refcount_t mm_count; atomic_long_t nr_ptes; /* PTE page table pages */ #if CONFIG_PGTABLE_LEVELS > 2 diff --git a/include/linux/sched.h b/include/linux/sched.h index c21682c..41a2e52 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -2924,14 +2924,14 @@ extern struct mm_struct * mm_alloc(void); */ static inline void mmgrab(struct mm_struct *mm) { - atomic_inc(&mm->mm_count); + refcount_inc(&mm->mm_count); } /* mmdrop drops the mm and the page tables */ extern void __mmdrop(struct mm_struct *); static inline void mmdrop(struct mm_struct *mm) { - if (unlikely(atomic_dec_and_test(&mm->mm_count))) + if (unlikely(refcount_dec_and_test(&mm->mm_count))) __mmdrop(mm); } @@ -2943,7 +2943,7 @@ static inline void mmdrop_async_fn(struct work_struct *work) static inline void mmdrop_async(struct mm_struct *mm) { - if (unlikely(atomic_dec_and_test(&mm->mm_count))) { + if (unlikely(refcount_dec_and_test(&mm->mm_count))) { INIT_WORK(&mm->async_put_work, mmdrop_async_fn); schedule_work(&mm->async_put_work); } diff --git a/kernel/fork.c b/kernel/fork.c index 60ff801..31c887c 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -760,7 +760,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p, mm->mm_rb = RB_ROOT; mm->vmacache_seqnum = 0; refcount_set(&mm->mm_users, 1); - atomic_set(&mm->mm_count, 1); + refcount_set(&mm->mm_count, 1); init_rwsem(&mm->mmap_sem); INIT_LIST_HEAD(&mm->mmlist); mm->core_state = NULL; diff --git a/mm/debug.c b/mm/debug.c index 0866505..eebbd15 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -135,7 +135,7 @@ void dump_mm(const struct mm_struct *mm) #endif mm->mmap_base, mm->mmap_legacy_base, mm->highest_vm_end, mm->pgd, refcount_read(&mm->mm_users), - atomic_read(&mm->mm_count), + refcount_read(&mm->mm_count), atomic_long_read((atomic_long_t *)&mm->nr_ptes), mm_nr_pmds((struct mm_struct *)mm), mm->map_count, diff --git a/mm/init-mm.c b/mm/init-mm.c index 6927a72..8de5267 100644 --- a/mm/init-mm.c +++ b/mm/init-mm.c @@ -18,7 +18,7 @@ struct mm_struct init_mm = { .mm_rb = RB_ROOT, .pgd = swapper_pg_dir, .mm_users = REFCOUNT_INIT(2), - .mm_count = ATOMIC_INIT(1), + .mm_count = REFCOUNT_INIT(1), .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), .mmlist = LIST_HEAD_INIT(init_mm.mmlist), diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c index d8c11e1..d179083 100644 --- a/mm/mmu_notifier.c +++ b/mm/mmu_notifier.c @@ -348,7 +348,7 @@ void __mmu_notifier_mm_destroy(struct mm_struct *mm) */ void mmu_notifier_unregister(struct mmu_notifier *mn, struct mm_struct *mm) { - BUG_ON(atomic_read(&mm->mm_count) <= 0); + BUG_ON(refcount_read(&mm->mm_count) <= 0); if (!hlist_unhashed(&mn->hlist)) { /* @@ -381,7 +381,7 @@ void mmu_notifier_unregister(struct mmu_notifier *mn, struct mm_struct *mm) */ synchronize_srcu(&srcu); - BUG_ON(atomic_read(&mm->mm_count) <= 0); + BUG_ON(refcount_read(&mm->mm_count) <= 0); mmdrop(mm); } @@ -401,7 +401,7 @@ void mmu_notifier_unregister_no_release(struct mmu_notifier *mn, hlist_del_init_rcu(&mn->hlist); spin_unlock(&mm->mmu_notifier_mm->lock); - BUG_ON(atomic_read(&mm->mm_count) <= 0); + BUG_ON(refcount_read(&mm->mm_count) <= 0); mmdrop(mm); } EXPORT_SYMBOL_GPL(mmu_notifier_unregister_no_release); -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] mm: convert mm_struct.mm_count from atomic_t to refcount_t 2017-02-20 10:49 ` [PATCH 5/5] mm: convert mm_struct.mm_count " Elena Reshetova @ 2017-02-20 13:16 ` kbuild test robot 0 siblings, 0 replies; 13+ messages in thread From: kbuild test robot @ 2017-02-20 13:16 UTC (permalink / raw) To: Elena Reshetova Cc: kbuild-all, linux-kernel, linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, Kees Cook, David Windsor [-- Attachment #1: Type: text/plain, Size: 3571 bytes --] Hi Elena, [auto build test ERROR on mmotm/master] [also build test ERROR on next-20170220] [cannot apply to linus/master linux/master v4.10] [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/mm-subsystem-refcounter-conversions/20170220-190351 base: git://git.cmpxchg.org/linux-mmotm.git master 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 >>): arch/blackfin/mach-common/smp.c: In function 'cpu_die': >> arch/blackfin/mach-common/smp.c:426:13: error: passing argument 1 of 'atomic_dec' from incompatible pointer type [-Werror=incompatible-pointer-types] atomic_dec(&init_mm.mm_count); ^ In file included from arch/blackfin/include/asm/atomic.h:45:0, from include/linux/atomic.h:4, from arch/blackfin/include/asm/spinlock.h:14, from include/linux/spinlock.h:87, from include/linux/seqlock.h:35, from include/linux/time.h:5, from include/linux/stat.h:18, from include/linux/module.h:10, from arch/blackfin/mach-common/smp.c:10: include/asm-generic/atomic.h:211:20: note: expected 'atomic_t * {aka struct <anonymous> *}' but argument is of type 'refcount_t * {aka struct refcount_struct *}' static inline void atomic_dec(atomic_t *v) ^~~~~~~~~~ cc1: some warnings being treated as errors vim +/atomic_dec +426 arch/blackfin/mach-common/smp.c 0b39db28 Graf Yang 2009-12-28 410 return -EPERM; 0b39db28 Graf Yang 2009-12-28 411 0b39db28 Graf Yang 2009-12-28 412 set_cpu_online(cpu, false); 0b39db28 Graf Yang 2009-12-28 413 return 0; 0b39db28 Graf Yang 2009-12-28 414 } 0b39db28 Graf Yang 2009-12-28 415 13dff62d Paul Gortmaker 2013-06-18 416 int __cpu_die(unsigned int cpu) 0b39db28 Graf Yang 2009-12-28 417 { a17b4b74 Paul E. McKenney 2015-02-26 418 return cpu_wait_death(cpu, 5); 0b39db28 Graf Yang 2009-12-28 419 } 0b39db28 Graf Yang 2009-12-28 420 0b39db28 Graf Yang 2009-12-28 421 void cpu_die(void) 0b39db28 Graf Yang 2009-12-28 422 { a17b4b74 Paul E. McKenney 2015-02-26 423 (void)cpu_report_death(); 0b39db28 Graf Yang 2009-12-28 424 afdf6066 Elena Reshetova 2017-02-20 425 refcount_dec(&init_mm.mm_users); 0b39db28 Graf Yang 2009-12-28 @426 atomic_dec(&init_mm.mm_count); 0b39db28 Graf Yang 2009-12-28 427 0b39db28 Graf Yang 2009-12-28 428 local_irq_disable(); 0b39db28 Graf Yang 2009-12-28 429 platform_cpu_die(); 0b39db28 Graf Yang 2009-12-28 430 } 0b39db28 Graf Yang 2009-12-28 431 #endif :::::: The code at line 426 was first introduced by commit :::::: 0b39db28b953945232719e7ff6fb802aa8a2be5f Blackfin: SMP: add PM/CPU hotplug support :::::: TO: Graf Yang <graf.yang@analog.com> :::::: 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: 10736 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/5] mm subsystem refcounter conversions @ 2017-02-21 9:58 Elena Reshetova 2017-02-21 9:58 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova 0 siblings, 1 reply; 13+ messages in thread From: Elena Reshetova @ 2017-02-21 9:58 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, akpm, arnd, luto, Elena Reshetova v2: - incorporated fixes reported 0day CI Now when new refcount_t type and API are finally merged (see include/linux/refcount.h), the following patches convert various refcounters in the mm 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 (5): mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t mm: convert anon_vma.refcount from atomic_t to refcount_t mm: convert kmemleak_object.use_count from atomic_t to refcount_t mm: convert mm_struct.mm_users from atomic_t to refcount_t mm: convert mm_struct.mm_count from atomic_t to refcount_t arch/alpha/kernel/smp.c | 6 +++--- arch/arc/mm/tlb.c | 2 +- arch/blackfin/mach-common/smp.c | 4 ++-- arch/ia64/include/asm/tlbflush.h | 2 +- arch/ia64/kernel/smp.c | 2 +- arch/ia64/sn/kernel/sn2/sn2_smp.c | 4 ++-- arch/mips/kernel/process.c | 2 +- arch/mips/kernel/smp.c | 6 +++--- arch/parisc/include/asm/mmu_context.h | 2 +- arch/powerpc/mm/hugetlbpage.c | 2 +- arch/powerpc/mm/icswx.c | 4 ++-- arch/sh/kernel/smp.c | 6 +++--- arch/sparc/kernel/smp_64.c | 6 +++--- arch/sparc/mm/srmmu.c | 2 +- arch/um/kernel/tlb.c | 2 +- arch/x86/kernel/tboot.c | 4 ++-- drivers/firmware/efi/arm-runtime.c | 4 ++-- drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +- fs/coredump.c | 2 +- fs/proc/base.c | 2 +- fs/userfaultfd.c | 3 +-- include/linux/backing-dev-defs.h | 3 ++- include/linux/backing-dev.h | 4 ++-- include/linux/mm_types.h | 5 +++-- include/linux/rmap.h | 7 ++++--- include/linux/sched.h | 10 +++++----- kernel/events/uprobes.c | 2 +- kernel/exit.c | 2 +- kernel/fork.c | 12 ++++++------ kernel/sched/core.c | 2 +- lib/is_single_threaded.c | 2 +- mm/backing-dev.c | 13 +++++++------ mm/debug.c | 4 ++-- mm/init-mm.c | 4 ++-- mm/khugepaged.c | 2 +- mm/kmemleak.c | 16 ++++++++-------- mm/ksm.c | 2 +- mm/memory.c | 2 +- mm/mmu_notifier.c | 10 +++++----- mm/mprotect.c | 2 +- mm/oom_kill.c | 2 +- mm/rmap.c | 14 +++++++------- mm/swapfile.c | 2 +- mm/vmacache.c | 2 +- 44 files changed, 98 insertions(+), 95 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t 2017-02-21 9:58 [PATCH 0/5] mm subsystem refcounter conversions Elena Reshetova @ 2017-02-21 9:58 ` Elena Reshetova 0 siblings, 0 replies; 13+ messages in thread From: Elena Reshetova @ 2017-02-21 9:58 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, viro, catalin.marinas, mingo, 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. Switch bdi_writeback_congested.refcnt from atomic_t to refcount_t and increment initial value by 1. The incrementation affecs the function wb_congested_get_create which previously incremented both found and created objects. After this patch the function will increment only found objects and instead set the refcount of new objects to 1. Note that new_congested is initially NULL, and will be discarded unless exiting via the 'if (new_congested)' section. 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/backing-dev-defs.h | 3 ++- include/linux/backing-dev.h | 4 ++-- mm/backing-dev.c | 13 +++++++------ 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h index ad95581..609ee6f 100644 --- a/include/linux/backing-dev-defs.h +++ b/include/linux/backing-dev-defs.h @@ -4,6 +4,7 @@ #include <linux/list.h> #include <linux/radix-tree.h> #include <linux/rbtree.h> +#include <linux/refcount.h> #include <linux/spinlock.h> #include <linux/percpu_counter.h> #include <linux/percpu-refcount.h> @@ -51,7 +52,7 @@ enum wb_stat_item { */ struct bdi_writeback_congested { unsigned long state; /* WB_[a]sync_congested flags */ - atomic_t refcnt; /* nr of attached wb's and blkg */ + refcount_t refcnt; /* nr of attached wb's and blkg */ #ifdef CONFIG_CGROUP_WRITEBACK struct backing_dev_info *bdi; /* the associated bdi */ diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h index c52a48c..4726d81 100644 --- a/include/linux/backing-dev.h +++ b/include/linux/backing-dev.h @@ -430,13 +430,13 @@ static inline bool inode_cgwb_enabled(struct inode *inode) static inline struct bdi_writeback_congested * wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) { - atomic_inc(&bdi->wb_congested->refcnt); + refcount_inc(&bdi->wb_congested->refcnt); return bdi->wb_congested; } static inline void wb_congested_put(struct bdi_writeback_congested *congested) { - if (atomic_dec_and_test(&congested->refcnt)) + if (refcount_dec_and_test(&congested->refcnt)) kfree(congested); } diff --git a/mm/backing-dev.c b/mm/backing-dev.c index 6d861d0..064312c 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -417,14 +417,17 @@ wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) node = &parent->rb_left; else if (congested->blkcg_id > blkcg_id) node = &parent->rb_right; - else - goto found; + else { + refcount_inc(&congested->refcnt); + goto found; + } } if (new_congested) { /* !found and storage for new one already allocated, insert */ congested = new_congested; new_congested = NULL; + refcount_set(&congested->refcnt, 1); rb_link_node(&congested->rb_node, parent, node); rb_insert_color(&congested->rb_node, &bdi->cgwb_congested_tree); goto found; @@ -437,13 +440,11 @@ wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) if (!new_congested) return NULL; - atomic_set(&new_congested->refcnt, 0); new_congested->bdi = bdi; new_congested->blkcg_id = blkcg_id; goto retry; found: - atomic_inc(&congested->refcnt); spin_unlock_irqrestore(&cgwb_lock, flags); kfree(new_congested); return congested; @@ -460,7 +461,7 @@ void wb_congested_put(struct bdi_writeback_congested *congested) unsigned long flags; local_irq_save(flags); - if (!atomic_dec_and_lock(&congested->refcnt, &cgwb_lock)) { + if (!refcount_dec_and_lock(&congested->refcnt, &cgwb_lock)) { local_irq_restore(flags); return; } @@ -759,7 +760,7 @@ static int cgwb_bdi_init(struct backing_dev_info *bdi) if (!bdi->wb_congested) return -ENOMEM; - atomic_set(&bdi->wb_congested->refcnt, 1); + refcount_set(&bdi->wb_congested->refcnt, 1); err = wb_init(&bdi->wb, bdi, 1, GFP_KERNEL); if (err) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 0/5] v2 mm subsystem refcounter conversions @ 2017-06-27 11:48 Elena Reshetova 2017-06-27 11:48 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova 0 siblings, 1 reply; 13+ messages in thread From: Elena Reshetova @ 2017-06-27 11:48 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, keescook, viro, catalin.marinas, mingo, akpm, arnd, luto, Elena Reshetova Changes in v2: No changes in patches apart from trivial rebases, but now by default refcount_t = atomic_t and uses all atomic standard operations unless CONFIG_REFCOUNT_FULL is enabled. This is a compromize for the systems that are critical on performance and cannot accept even slight delay on the refcounter operations. Elena Reshetova (5): mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t mm: convert anon_vma.refcount from atomic_t to refcount_t mm: convert kmemleak_object.use_count from atomic_t to refcount_t mm: convert mm_struct.mm_users from atomic_t to refcount_t mm: convert mm_struct.mm_count from atomic_t to refcount_t arch/alpha/kernel/smp.c | 6 +++--- arch/arc/mm/tlb.c | 2 +- arch/blackfin/mach-common/smp.c | 4 ++-- arch/ia64/include/asm/tlbflush.h | 2 +- arch/ia64/kernel/smp.c | 2 +- arch/ia64/sn/kernel/sn2/sn2_smp.c | 4 ++-- arch/mips/kernel/process.c | 2 +- arch/mips/kernel/smp.c | 6 +++--- arch/parisc/include/asm/mmu_context.h | 2 +- arch/powerpc/mm/hugetlbpage.c | 2 +- arch/powerpc/mm/icswx.c | 4 ++-- arch/sh/kernel/smp.c | 6 +++--- arch/sparc/kernel/smp_64.c | 6 +++--- arch/sparc/mm/srmmu.c | 2 +- arch/um/kernel/tlb.c | 2 +- arch/x86/kernel/tboot.c | 4 ++-- drivers/firmware/efi/arm-runtime.c | 4 ++-- drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +- fs/coredump.c | 2 +- fs/proc/base.c | 2 +- fs/proc/task_nommu.c | 4 ++-- fs/userfaultfd.c | 3 +-- include/linux/backing-dev-defs.h | 3 ++- include/linux/backing-dev.h | 4 ++-- include/linux/mm_types.h | 5 +++-- include/linux/rmap.h | 7 ++++--- include/linux/sched/mm.h | 10 +++++----- kernel/events/uprobes.c | 2 +- kernel/exit.c | 2 +- kernel/fork.c | 12 ++++++------ kernel/sched/core.c | 2 +- lib/is_single_threaded.c | 2 +- mm/backing-dev.c | 13 +++++++------ mm/debug.c | 4 ++-- mm/init-mm.c | 4 ++-- mm/khugepaged.c | 2 +- mm/kmemleak.c | 16 ++++++++-------- mm/ksm.c | 2 +- mm/memory.c | 2 +- mm/mmu_notifier.c | 10 +++++----- mm/mprotect.c | 2 +- mm/oom_kill.c | 2 +- mm/rmap.c | 14 +++++++------- mm/swapfile.c | 2 +- mm/vmacache.c | 2 +- 45 files changed, 100 insertions(+), 97 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t 2017-06-27 11:48 [PATCH 0/5] v2 mm subsystem refcounter conversions Elena Reshetova @ 2017-06-27 11:48 ` Elena Reshetova 0 siblings, 0 replies; 13+ messages in thread From: Elena Reshetova @ 2017-06-27 11:48 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, peterz, gregkh, keescook, viro, catalin.marinas, mingo, akpm, arnd, luto, Elena Reshetova, Hans Liljestrand, 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. Switch bdi_writeback_congested.refcnt from atomic_t to refcount_t and increment initial value by 1. The incrementation affecs the function wb_congested_get_create which previously incremented both found and created objects. After this patch the function will increment only found objects and instead set the refcount of new objects to 1. Note that new_congested is initially NULL, and will be discarded unless exiting via the 'if (new_congested)' section. 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/backing-dev-defs.h | 3 ++- include/linux/backing-dev.h | 4 ++-- mm/backing-dev.c | 13 +++++++------ 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h index ad95581..609ee6f 100644 --- a/include/linux/backing-dev-defs.h +++ b/include/linux/backing-dev-defs.h @@ -4,6 +4,7 @@ #include <linux/list.h> #include <linux/radix-tree.h> #include <linux/rbtree.h> +#include <linux/refcount.h> #include <linux/spinlock.h> #include <linux/percpu_counter.h> #include <linux/percpu-refcount.h> @@ -51,7 +52,7 @@ enum wb_stat_item { */ struct bdi_writeback_congested { unsigned long state; /* WB_[a]sync_congested flags */ - atomic_t refcnt; /* nr of attached wb's and blkg */ + refcount_t refcnt; /* nr of attached wb's and blkg */ #ifdef CONFIG_CGROUP_WRITEBACK struct backing_dev_info *bdi; /* the associated bdi */ diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h index c52a48c..4726d81 100644 --- a/include/linux/backing-dev.h +++ b/include/linux/backing-dev.h @@ -430,13 +430,13 @@ static inline bool inode_cgwb_enabled(struct inode *inode) static inline struct bdi_writeback_congested * wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) { - atomic_inc(&bdi->wb_congested->refcnt); + refcount_inc(&bdi->wb_congested->refcnt); return bdi->wb_congested; } static inline void wb_congested_put(struct bdi_writeback_congested *congested) { - if (atomic_dec_and_test(&congested->refcnt)) + if (refcount_dec_and_test(&congested->refcnt)) kfree(congested); } diff --git a/mm/backing-dev.c b/mm/backing-dev.c index c6f2a37..1799930 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -417,14 +417,17 @@ wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) node = &parent->rb_left; else if (congested->blkcg_id > blkcg_id) node = &parent->rb_right; - else - goto found; + else { + refcount_inc(&congested->refcnt); + goto found; + } } if (new_congested) { /* !found and storage for new one already allocated, insert */ congested = new_congested; new_congested = NULL; + refcount_set(&congested->refcnt, 1); rb_link_node(&congested->rb_node, parent, node); rb_insert_color(&congested->rb_node, &bdi->cgwb_congested_tree); goto found; @@ -437,13 +440,11 @@ wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp) if (!new_congested) return NULL; - atomic_set(&new_congested->refcnt, 0); new_congested->bdi = bdi; new_congested->blkcg_id = blkcg_id; goto retry; found: - atomic_inc(&congested->refcnt); spin_unlock_irqrestore(&cgwb_lock, flags); kfree(new_congested); return congested; @@ -460,7 +461,7 @@ void wb_congested_put(struct bdi_writeback_congested *congested) unsigned long flags; local_irq_save(flags); - if (!atomic_dec_and_lock(&congested->refcnt, &cgwb_lock)) { + if (!refcount_dec_and_lock(&congested->refcnt, &cgwb_lock)) { local_irq_restore(flags); return; } @@ -767,7 +768,7 @@ static int cgwb_bdi_init(struct backing_dev_info *bdi) if (!bdi->wb_congested) return -ENOMEM; - atomic_set(&bdi->wb_congested->refcnt, 1); + refcount_set(&bdi->wb_congested->refcnt, 1); err = wb_init(&bdi->wb, bdi, 1, GFP_KERNEL); if (err) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-06-27 11:50 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-20 10:49 [PATCH 0/5] mm subsystem refcounter conversions Elena Reshetova 2017-02-20 10:49 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova 2017-02-20 11:33 ` kbuild test robot 2017-02-20 10:49 ` [PATCH 2/5] mm: convert anon_vma.refcount " Elena Reshetova 2017-04-22 10:44 ` zhong jiang 2017-04-24 10:20 ` Reshetova, Elena 2017-02-20 10:49 ` [PATCH 3/5] mm: convert kmemleak_object.use_count " Elena Reshetova 2017-02-20 10:49 ` [PATCH 4/5] mm: convert mm_struct.mm_users " Elena Reshetova 2017-02-20 11:46 ` kbuild test robot 2017-02-20 10:49 ` [PATCH 5/5] mm: convert mm_struct.mm_count " Elena Reshetova 2017-02-20 13:16 ` kbuild test robot 2017-02-21 9:58 [PATCH 0/5] mm subsystem refcounter conversions Elena Reshetova 2017-02-21 9:58 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova 2017-06-27 11:48 [PATCH 0/5] v2 mm subsystem refcounter conversions Elena Reshetova 2017-06-27 11:48 ` [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t 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).