Messages in this thread | | | Date | Fri, 12 Jun 2020 10:06:49 -0700 | From | Kees Cook <> | Subject | Re: [PATCH RFC] cred: Add WARN to detect wrong use of get/put_cred |
| |
On Fri, Jun 12, 2020 at 06:33:45PM +0200, Peter Zijlstra wrote: > On Fri, Jun 12, 2020 at 06:28:15PM +0800, Xiaoming Ni wrote: > > 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. > > Why not not use refcount_t? It has all that goodness and more.
I thought these had been applied already, I guess not: https://lore.kernel.org/lkml/20190306110549.7628-1-elena.reshetova@intel.com/
Can we try again?
-- Kees Cook
| |