lkml.org 
[lkml]   [2020]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH RFC] cred: Add WARN to detect wrong use of get/put_cred
Date
From: Xiaoming Ni
> Sent: 12 June 2020 11:28
> Cred release and usage check code flow:
> 1. put_cred()
> if (atomic_dec_and_test(&(cred)->usage))
> __put_cred(cred);
>
> 2. __put_cred()
> BUG_ON(atomic_read(&cred->usage) != 0);
> call_rcu(&cred->rcu, put_cred_rcu);
>
> 3. put_cred_rcu()
> if (atomic_read(&cred->usage) != 0)
> panic("CRED: put_cred_rcu() sees %p with usage %d\n",
> cred, atomic_read(&cred->usage));
> kmem_cache_free(cred_jar, cred);
>
> If panic is triggered on put_cred_rcu(), there are two possibilities
> 1. Call get_cred() after __put_cred(), usage > 0
> 2. Call put_cred() after __put_cred(), usage < 0
> Since put_cred_rcu is an asynchronous behavior, it is no longer the first
> scene when panic, there is no information about the murderer in the panic
> call stack...
>
> So, add WARN() in get_cred()/put_cred(), and pray to catch the murderer
> at the first scene.
>
> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com>
> ---
> include/linux/cred.h | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 18639c0..c00d5a1 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -224,11 +224,16 @@ static inline bool cap_ambient_invariant_ok(const struct cred *cred)
> *
> * Get a reference on the specified set of new credentials. The caller must
> * release the reference.
> + *
> + * Initialize usage to 1 during cred resource allocation,
> + * so when calling get_cred, usage cannot be 0.
> */
> static inline struct cred *get_new_cred(struct cred *cred)
> {
> - atomic_inc(&cred->usage);
> - return cred;
> + if (atomic_inc_not_zero(&cred->usage))
> + return cred;
> + WARN(1, "get_new_cred after __put_cred");
> + return NULL;
> }
>
> /**
> @@ -280,11 +285,14 @@ static inline const struct cred *get_cred_rcu(const struct cred *cred)
> static inline void put_cred(const struct cred *_cred)
> {
> struct cred *cred = (struct cred *) _cred;
> + int usage;
>
> if (cred) {
> validate_creds(cred);
> - if (atomic_dec_and_test(&(cred)->usage))
> + usage = atomic_dec_return(&(cred)->usage);
> + if (usage == 0)
> __put_cred(cred);
> + WARN(usage < 0, "put_cred after __put_cred");
> }
> }

You really don't want to add WARN() to static inline functions.
It will bloat horribly.
It might be possible to the message into a called function.

One thing I've thought about for reference counts is for the
code that allocates and frees the item to add a big number
and code that only borrows a reference just adds 1.
If the counter is large enough you can separately detect
double frees and missing frees for the two different types
of allocation.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

\
 
 \ /
  Last update: 2020-06-12 18:17    [W:0.330 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site