Messages in this thread | | | Subject | Re: [PATCH -next v2] locking/percpu-rwsem: fix a task_struct refcount | From | Qian Cai <> | Date | Mon, 6 Apr 2020 09:31:18 -0400 |
| |
> On Mar 30, 2020, at 5:30 PM, Qian Cai <cai@lca.pw> wrote: > > The commit 7f26482a872c ("locking/percpu-rwsem: Remove the embedded > rwsem") introduced some task_struct memory leaks due to messing up with > a task_struct refcount. At the beginning of > percpu_rwsem_wake_function(), it calls get_task_struct(), but if the > trylock failed, it will remain in the waitqueue. However, it will run > percpu_rwsem_wake_function() again with get_task_struct() to increase > the refcount but then only call put_task_struct() once the trylock > succeeded. > > Fix it by adjusting percpu_rwsem_wake_function() a bit to guard against > when percpu_rwsem_wait() observing !private, terminating the wait and > doing a quick exit() while percpu_rwsem_wake_function() then doing > wake_up_process(p) as a use-after-free. > > Fixes: 7f26482a872c ("locking/percpu-rwsem: Remove the embedded rwsem")
Peter, Ingo, can you take a look at this patch when you have a chance?
For some reasons Ingo had decided to send a pull request which is now merged even though I had informed the commit was broken a few days earlier, it makes no sense to leave known memory leaks in mainline like this.
> Suggested-by: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Qian Cai <cai@lca.pw> > --- > kernel/locking/percpu-rwsem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c > index a008a1ba21a7..8bbafe3e5203 100644 > --- a/kernel/locking/percpu-rwsem.c > +++ b/kernel/locking/percpu-rwsem.c > @@ -118,14 +118,15 @@ static int percpu_rwsem_wake_function(struct wait_queue_entry *wq_entry, > unsigned int mode, int wake_flags, > void *key) > { > - struct task_struct *p = get_task_struct(wq_entry->private); > bool reader = wq_entry->flags & WQ_FLAG_CUSTOM; > struct percpu_rw_semaphore *sem = key; > + struct task_struct *p; > > /* concurrent against percpu_down_write(), can get stolen */ > if (!__percpu_rwsem_trylock(sem, reader)) > return 1; > > + p = get_task_struct(wq_entry->private); > list_del_init(&wq_entry->entry); > smp_store_release(&wq_entry->private, NULL); > > -- > 2.21.0 (Apple Git-122.2) >
| |