On Thu, Nov 24, 2016 at 04:38:08PM +0100, Peter Zijlstra wrote: > On Thu, Nov 24, 2016 at 04:19:41PM +0100, Thomas Gleixner wrote: > > On Thu, 24 Nov 2016, Peter Zijlstra wrote: > > > > > > > > While working on the futex code, I stumbled over this potential > > > use-after-free scenario. > > > > > > pi_mutex is a pointer into pi_state, which we drop the reference on in > > > unqueue_me_pi(). So any access to that pointer after that is bad. > > > > > > Since other sites already do rt_mutex_unlock() with hb->lock held, see > > > for example futex_lock_pi(), simply move the unlock before > > > unqueue_me_pi(). > > > > > > Signed-off-by: Peter Zijlstra (Intel) <[email protected]> > > > --- > > > kernel/futex.c | 22 +++++++++++++--------- > > > 1 file changed, 13 insertions(+), 9 deletions(-) > > > > > > diff --git a/kernel/futex.c b/kernel/futex.c > > > index 2c4be467fecd..d5a81339209f 100644 > > > --- a/kernel/futex.c > > > +++ b/kernel/futex.c > > > @@ -2813,7 +2813,6 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, > > > unsigned int flags, > > > { > > > struct hrtimer_sleeper timeout, *to = NULL; > > > struct rt_mutex_waiter rt_waiter; > > > - struct rt_mutex *pi_mutex = NULL; > > > struct futex_hash_bucket *hb; > > > union futex_key key2 = FUTEX_KEY_INIT; > > > struct futex_q q = futex_q_init; > > > @@ -2905,6 +2904,8 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, > > > unsigned int flags, > > > spin_unlock(q.lock_ptr); > > > > In this path the fixup can return -EFAIL as well, so it should drop rtmutex > > too if it owns it. We should move the rtmutex drop into the fixup > > functions... > > Urgh, so would really like to avoid doing that, I'll have to instantly > drag it back out again :/
Why would you have to drag it back out again? Something else you're working on? -- Darren Hart Intel Open Source Technology Center
