Commit aa10990e authored by Darren Hart's avatar Darren Hart Committed by Linus Torvalds

futex: avoid wake_futex() for a PI futex_q

Dave Jones reported a bug with futex_lock_pi() that his trinity test
exposed.  Sometime between queue_me() and taking the q.lock_ptr, the
lock_ptr became NULL, resulting in a crash.

While futex_wake() is careful to not call wake_futex() on futex_q's with
a pi_state or an rt_waiter (which are either waiting for a
futex_unlock_pi() or a PI futex_requeue()), futex_wake_op() and
futex_requeue() do not perform the same test.

Update futex_wake_op() and futex_requeue() to test for q.pi_state and
q.rt_waiter and abort with -EINVAL if detected.  To ensure any future
breakage is caught, add a WARN() to wake_futex() if the same condition
is true.

This fix has seen 3 hours of testing with "trinity -c futex" on an
x86_64 VM with 4 CPUS.

[akpm@linux-foundation.org: tidy up the WARN()]
Signed-off-by: default avatarDarren Hart <dvhart@linux.intel.com>
Reported-by: default avatarDave Jones <davej@redat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: John Kacur <jkacur@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 8ffeb9b0
...@@ -843,6 +843,9 @@ static void wake_futex(struct futex_q *q) ...@@ -843,6 +843,9 @@ static void wake_futex(struct futex_q *q)
{ {
struct task_struct *p = q->task; struct task_struct *p = q->task;
if (WARN(q->pi_state || q->rt_waiter, "refusing to wake PI futex\n"))
return;
/* /*
* We set q->lock_ptr = NULL _before_ we wake up the task. If * We set q->lock_ptr = NULL _before_ we wake up the task. If
* a non-futex wake up happens on another CPU then the task * a non-futex wake up happens on another CPU then the task
...@@ -1078,6 +1081,10 @@ futex_wake_op(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2, ...@@ -1078,6 +1081,10 @@ futex_wake_op(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2,
plist_for_each_entry_safe(this, next, head, list) { plist_for_each_entry_safe(this, next, head, list) {
if (match_futex (&this->key, &key1)) { if (match_futex (&this->key, &key1)) {
if (this->pi_state || this->rt_waiter) {
ret = -EINVAL;
goto out_unlock;
}
wake_futex(this); wake_futex(this);
if (++ret >= nr_wake) if (++ret >= nr_wake)
break; break;
...@@ -1090,6 +1097,10 @@ futex_wake_op(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2, ...@@ -1090,6 +1097,10 @@ futex_wake_op(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2,
op_ret = 0; op_ret = 0;
plist_for_each_entry_safe(this, next, head, list) { plist_for_each_entry_safe(this, next, head, list) {
if (match_futex (&this->key, &key2)) { if (match_futex (&this->key, &key2)) {
if (this->pi_state || this->rt_waiter) {
ret = -EINVAL;
goto out_unlock;
}
wake_futex(this); wake_futex(this);
if (++op_ret >= nr_wake2) if (++op_ret >= nr_wake2)
break; break;
...@@ -1098,6 +1109,7 @@ futex_wake_op(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2, ...@@ -1098,6 +1109,7 @@ futex_wake_op(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2,
ret += op_ret; ret += op_ret;
} }
out_unlock:
double_unlock_hb(hb1, hb2); double_unlock_hb(hb1, hb2);
out_put_keys: out_put_keys:
put_futex_key(&key2); put_futex_key(&key2);
...@@ -1387,9 +1399,13 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, ...@@ -1387,9 +1399,13 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,
/* /*
* FUTEX_WAIT_REQEUE_PI and FUTEX_CMP_REQUEUE_PI should always * FUTEX_WAIT_REQEUE_PI and FUTEX_CMP_REQUEUE_PI should always
* be paired with each other and no other futex ops. * be paired with each other and no other futex ops.
*
* We should never be requeueing a futex_q with a pi_state,
* which is awaiting a futex_unlock_pi().
*/ */
if ((requeue_pi && !this->rt_waiter) || if ((requeue_pi && !this->rt_waiter) ||
(!requeue_pi && this->rt_waiter)) { (!requeue_pi && this->rt_waiter) ||
this->pi_state) {
ret = -EINVAL; ret = -EINVAL;
break; break;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment