aboutsummaryrefslogtreecommitdiffstats
path: root/kernel
diff options
context:
space:
mode:
authorDarren Hart <dvhltc@us.ibm.com>2009-09-22 01:30:38 -0400
committerIngo Molnar <mingo@elte.hu>2009-09-22 04:37:44 -0400
commit0729e196147692d84d4c099fcff056eba2ed61d8 (patch)
tree5374dc5cd1cb2cfb3da1174b67ed07685592e83a /kernel
parentd8d88fbb186fe3ea37b2a58adb32413c98b59656 (diff)
futex: Fix wakeup race by setting TASK_INTERRUPTIBLE before queue_me()
PI futexes do not use the same plist_node_empty() test for wakeup. It was possible for the waiter (in futex_wait_requeue_pi()) to set TASK_INTERRUPTIBLE after the waker assigned the rtmutex to the waiter. The waiter would then note the plist was not empty and call schedule(). The task would not be found by any subsequeuent futex wakeups, resulting in a userspace hang. By moving the setting of TASK_INTERRUPTIBLE to before the call to queue_me(), the race with the waker is eliminated. Since we no longer call get_user() from within queue_me(), there is no need to delay the setting of TASK_INTERRUPTIBLE until after the call to queue_me(). The FUTEX_LOCK_PI operation is not affected as futex_lock_pi() relies entirely on the rtmutex code to handle schedule() and wakeup. The requeue PI code is affected because the waiter starts as a non-PI waiter and is woken on a PI futex. Remove the crusty old comment about holding spinlocks() across get_user() as we no longer do that. Correct the locking statement with a description of why the test is performed. Signed-off-by: Darren Hart <dvhltc@us.ibm.com> Acked-by: Peter Zijlstra <peterz@infradead.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Eric Dumazet <eric.dumazet@gmail.com> Cc: Dinakar Guniguntala <dino@in.ibm.com> Cc: John Stultz <johnstul@us.ibm.com> LKML-Reference: <20090922053038.8717.97838.stgit@Aeon> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Diffstat (limited to 'kernel')
-rw-r--r--kernel/futex.c15
1 files changed, 3 insertions, 12 deletions
diff --git a/kernel/futex.c b/kernel/futex.c
index f92afbe3d3a1..463af2efa512 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1656,17 +1656,8 @@ out:
1656static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q, 1656static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q,
1657 struct hrtimer_sleeper *timeout) 1657 struct hrtimer_sleeper *timeout)
1658{ 1658{
1659 queue_me(q, hb);
1660
1661 /*
1662 * There might have been scheduling since the queue_me(), as we
1663 * cannot hold a spinlock across the get_user() in case it
1664 * faults, and we cannot just set TASK_INTERRUPTIBLE state when
1665 * queueing ourselves into the futex hash. This code thus has to
1666 * rely on the futex_wake() code removing us from hash when it
1667 * wakes us up.
1668 */
1669 set_current_state(TASK_INTERRUPTIBLE); 1659 set_current_state(TASK_INTERRUPTIBLE);
1660 queue_me(q, hb);
1670 1661
1671 /* Arm the timer */ 1662 /* Arm the timer */
1672 if (timeout) { 1663 if (timeout) {
@@ -1676,8 +1667,8 @@ static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q,
1676 } 1667 }
1677 1668
1678 /* 1669 /*
1679 * !plist_node_empty() is safe here without any lock. 1670 * If we have been removed from the hash list, then another task
1680 * q.lock_ptr != 0 is not safe, because of ordering against wakeup. 1671 * has tried to wake us, and we can skip the call to schedule().
1681 */ 1672 */
1682 if (likely(!plist_node_empty(&q->list))) { 1673 if (likely(!plist_node_empty(&q->list))) {
1683 /* 1674 /*