Commit 69cd9eba authored by Linus Torvalds's avatar Linus Torvalds

futex: avoid race between requeue and wake

Jan Stancek reported:
 "pthread_cond_broadcast/4-1.c testcase from openposix testsuite (LTP)
  occasionally fails, because some threads fail to wake up.

  Testcase creates 5 threads, which are all waiting on same condition.
  Main thread then calls pthread_cond_broadcast() without holding mutex,
  which calls:

      futex(uaddr1, FUTEX_CMP_REQUEUE_PRIVATE, 1, 2147483647, uaddr2, ..)

  This immediately wakes up single thread A, which unlocks mutex and
  tries to wake up another thread:

      futex(uaddr2, FUTEX_WAKE_PRIVATE, 1)

  If thread A manages to call futex_wake() before any waiters are
  requeued for uaddr2, no other thread is woken up"

The ordering constraints for the hash bucket waiter counting are that
the waiter counts have to be incremented _before_ getting the spinlock
(because the spinlock acts as part of the memory barrier), but the
"requeue" operation didn't honor those rules, and nobody had even
thought about that case.

This fairly simple patch just increments the waiter count for the target
hash bucket (hb2) when requeing a futex before taking the locks.  It
then decrements them again after releasing the lock - the code that
actually moves the futex(es) between hash buckets will do the additional
required waiter count housekeeping.
Reported-and-tested-by: default avatarJan Stancek <jstancek@redhat.com>
Acked-by: default avatarDavidlohr Bueso <davidlohr@hp.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org # 3.14
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 75ff24fa
...@@ -1452,6 +1452,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, ...@@ -1452,6 +1452,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,
hb2 = hash_futex(&key2); hb2 = hash_futex(&key2);
retry_private: retry_private:
hb_waiters_inc(hb2);
double_lock_hb(hb1, hb2); double_lock_hb(hb1, hb2);
if (likely(cmpval != NULL)) { if (likely(cmpval != NULL)) {
...@@ -1461,6 +1462,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, ...@@ -1461,6 +1462,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,
if (unlikely(ret)) { if (unlikely(ret)) {
double_unlock_hb(hb1, hb2); double_unlock_hb(hb1, hb2);
hb_waiters_dec(hb2);
ret = get_user(curval, uaddr1); ret = get_user(curval, uaddr1);
if (ret) if (ret)
...@@ -1510,6 +1512,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, ...@@ -1510,6 +1512,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,
break; break;
case -EFAULT: case -EFAULT:
double_unlock_hb(hb1, hb2); double_unlock_hb(hb1, hb2);
hb_waiters_dec(hb2);
put_futex_key(&key2); put_futex_key(&key2);
put_futex_key(&key1); put_futex_key(&key1);
ret = fault_in_user_writeable(uaddr2); ret = fault_in_user_writeable(uaddr2);
...@@ -1519,6 +1522,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, ...@@ -1519,6 +1522,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,
case -EAGAIN: case -EAGAIN:
/* The owner was exiting, try again. */ /* The owner was exiting, try again. */
double_unlock_hb(hb1, hb2); double_unlock_hb(hb1, hb2);
hb_waiters_dec(hb2);
put_futex_key(&key2); put_futex_key(&key2);
put_futex_key(&key1); put_futex_key(&key1);
cond_resched(); cond_resched();
...@@ -1594,6 +1598,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, ...@@ -1594,6 +1598,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags,
out_unlock: out_unlock:
double_unlock_hb(hb1, hb2); double_unlock_hb(hb1, hb2);
hb_waiters_dec(hb2);
/* /*
* drop_futex_key_refs() must be called outside the spinlocks. During * drop_futex_key_refs() must be called outside the spinlocks. During
......
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