Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1628817 > unrolled thread
| Started by | zhong jiang <zhongjiang@huawei.com> |
|---|---|
| First post | 2017-04-22 12:50 +0200 |
| Last post | 2017-04-24 12:30 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 2/5] mm: convert anon_vma.refcount from atomic_t to refcount_t zhong jiang <zhongjiang@huawei.com> - 2017-04-22 12:50 +0200
RE: [PATCH 2/5] mm: convert anon_vma.refcount from atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-04-24 12:30 +0200
| From | zhong jiang <zhongjiang@huawei.com> |
|---|---|
| Date | 2017-04-22 12:50 +0200 |
| Subject | Re: [PATCH 2/5] mm: convert anon_vma.refcount from atomic_t to refcount_t |
| Message-ID | <tz0M1-1LO-1@gated-at.bofh.it> |
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);
> }
>
[toc] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2017-04-24 12:30 +0200 |
| Subject | RE: [PATCH 2/5] mm: convert anon_vma.refcount from atomic_t to refcount_t |
| Message-ID | <tzJpM-5Mp-5@gated-at.bofh.it> |
| In reply to | #1628817 |
> 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);
> > }
> >
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web