futex: Fix potential use-after-free in FUTEX_REQUEUE_PI
While working on the futex code, I stumbled over this potential use-after-free scenario. Dmitry triggered it later with syzkaller. 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(). Reported-by: Dmitry Vyukov <dvyukov@google.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Darren Hart <dvhart@linux.intel.com> Cc: juri.lelli@arm.com Cc: bigeasy@linutronix.de Cc: xlpang@redhat.com Cc: rostedt@goodmis.org Cc: mathieu.desnoyers@efficios.com Cc: jdesfossez@efficios.com Cc: dvhart@infradead.org Cc: bristot@redhat.com Cc: stable@vger.kernel.org Link: http://lkml.kernel.org/r/20170304093558.801744246@infradead.org Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
This commit is contained in:
parent
4495c08e84
commit
c236c8e95a
|
@ -2815,7 +2815,6 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
|
||||||
{
|
{
|
||||||
struct hrtimer_sleeper timeout, *to = NULL;
|
struct hrtimer_sleeper timeout, *to = NULL;
|
||||||
struct rt_mutex_waiter rt_waiter;
|
struct rt_mutex_waiter rt_waiter;
|
||||||
struct rt_mutex *pi_mutex = NULL;
|
|
||||||
struct futex_hash_bucket *hb;
|
struct futex_hash_bucket *hb;
|
||||||
union futex_key key2 = FUTEX_KEY_INIT;
|
union futex_key key2 = FUTEX_KEY_INIT;
|
||||||
struct futex_q q = futex_q_init;
|
struct futex_q q = futex_q_init;
|
||||||
|
@ -2907,6 +2906,8 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
|
||||||
spin_unlock(q.lock_ptr);
|
spin_unlock(q.lock_ptr);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
struct rt_mutex *pi_mutex;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* We have been woken up by futex_unlock_pi(), a timeout, or a
|
* We have been woken up by futex_unlock_pi(), a timeout, or a
|
||||||
* signal. futex_unlock_pi() will not destroy the lock_ptr nor
|
* signal. futex_unlock_pi() will not destroy the lock_ptr nor
|
||||||
|
@ -2930,18 +2931,19 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
|
||||||
if (res)
|
if (res)
|
||||||
ret = (res < 0) ? res : 0;
|
ret = (res < 0) ? res : 0;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* If fixup_pi_state_owner() faulted and was unable to handle
|
||||||
|
* the fault, unlock the rt_mutex and return the fault to
|
||||||
|
* userspace.
|
||||||
|
*/
|
||||||
|
if (ret && rt_mutex_owner(pi_mutex) == current)
|
||||||
|
rt_mutex_unlock(pi_mutex);
|
||||||
|
|
||||||
/* Unqueue and drop the lock. */
|
/* Unqueue and drop the lock. */
|
||||||
unqueue_me_pi(&q);
|
unqueue_me_pi(&q);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
if (ret == -EINTR) {
|
||||||
* If fixup_pi_state_owner() faulted and was unable to handle the
|
|
||||||
* fault, unlock the rt_mutex and return the fault to userspace.
|
|
||||||
*/
|
|
||||||
if (ret == -EFAULT) {
|
|
||||||
if (pi_mutex && rt_mutex_owner(pi_mutex) == current)
|
|
||||||
rt_mutex_unlock(pi_mutex);
|
|
||||||
} else if (ret == -EINTR) {
|
|
||||||
/*
|
/*
|
||||||
* We've already been requeued, but cannot restart by calling
|
* We've already been requeued, but cannot restart by calling
|
||||||
* futex_lock_pi() directly. We could restart this syscall, but
|
* futex_lock_pi() directly. We could restart this syscall, but
|
||||||
|
|
Loading…
Reference in New Issue