Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1584471 > unrolled thread

[PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t

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.

Contents

  [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-02-20 11:30 +0100
    Re: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t Paul Moore <paul@paul-moore.com> - 2017-02-20 23:10 +0100
      RE: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to  refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-02-21 08:20 +0100
        Re: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t Kees Cook <keescook@chromium.org> - 2017-03-01 01:50 +0100

#1584471 — [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t

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

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

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

[toc] | [next] | [standalone]

#1584928

On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova
<elena.reshetova@intel.com> wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
>
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
>  kernel/audit_tree.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

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

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

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

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1585090 — RE: [PATCH 15/19] kernel: convert audit_tree.count from atomic_t to refcount_t

> On Mon, Feb 20, 2017 at 5:19 AM, Elena Reshetova
> <elena.reshetova@intel.com> wrote:
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> > ---
> >  kernel/audit_tree.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> No objection on my end, same for patch 16/19.
> 
> I have no problem merging both these patches into the audit/next
> branch after the merge window, is that your goal or are you merging
> these via a different tree?

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

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

Best Regards,
Elena.

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

[toc] | [prev] | [next] | [standalone]

#1589913

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

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

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web