Messages in this thread Patch in this message | | | From | Jan Stancek <> | Subject | [RFC PATCH] futex: don't retry futex_wait() with stale uaddr/val after spurious wakeup | Date | Wed, 6 Nov 2019 23:43:19 +0100 |
| |
Assume following scenario: process A is sleeping on futex (uaddr1) inside futex_wait(). Process B requeues this waiter via FUTEX_CMP_REQUEUE to uaddr2, but doesn't wake it up. Later, process A spuriously wakes up and futex_wait() retries to queue again with same uaddr1/val1: if (!signal_pending(current)) goto retry;
Problem: Userspace fails to wake process A with futex(uaddr2, FUTEX_WAKE)
Store target uaddr/val in futex_requeue() and let futex_wait() retry after spurious wake up using stored values.
Fixes: d58e6576b0de ("futex: Handle spurious wake up") Signed-off-by: Jan Stancek <jstancek@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Darren Hart <dvhart@infradead.org> Cc: Waiman Long <longman@redhat.com> --- kernel/futex.c | 52 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/kernel/futex.c b/kernel/futex.c index bd18f60e4c6c..c13cfee25d35 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -237,6 +237,9 @@ struct futex_q { struct rt_mutex_waiter *rt_waiter; union futex_key *requeue_pi_key; u32 bitset; + + u32 *uaddr; + u32 val; } __randomize_layout; static const struct futex_q futex_q_init = { @@ -1939,6 +1942,7 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, struct futex_hash_bucket *hb1, *hb2; struct futex_q *this, *next; DEFINE_WAKE_Q(wake_q); + u32 curval, targetval; if (nr_wake < 0 || nr_requeue < 0) return -EINVAL; @@ -2005,30 +2009,32 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, hb_waiters_inc(hb2); double_lock_hb(hb1, hb2); - if (likely(cmpval != NULL)) { - u32 curval; - + ret = get_futex_value_locked(&targetval, uaddr2); + if (!ret && likely(cmpval != NULL)) ret = get_futex_value_locked(&curval, uaddr1); - if (unlikely(ret)) { - double_unlock_hb(hb1, hb2); - hb_waiters_dec(hb2); + if (unlikely(ret)) { + double_unlock_hb(hb1, hb2); + hb_waiters_dec(hb2); + ret = get_user(targetval, uaddr2); + if (!ret && likely(cmpval != NULL)) ret = get_user(curval, uaddr1); - if (ret) - goto out_put_keys; - if (!(flags & FLAGS_SHARED)) - goto retry_private; + if (ret) + goto out_put_keys; - put_futex_key(&key2); - put_futex_key(&key1); - goto retry; - } - if (curval != *cmpval) { - ret = -EAGAIN; - goto out_unlock; - } + if (!(flags & FLAGS_SHARED)) + goto retry_private; + + put_futex_key(&key2); + put_futex_key(&key1); + goto retry; + } + + if (likely(cmpval != NULL) && (curval != *cmpval)) { + ret = -EAGAIN; + goto out_unlock; } if (requeue_pi && (task_count - nr_wake < nr_requeue)) { @@ -2185,6 +2191,12 @@ static int futex_requeue(u32 __user *uaddr1, unsigned int flags, } } requeue_futex(this, hb1, hb2, &key2); + /* + * Save target uaddr/val, in case futex_wait() wakes + * up spuriously and retries to requeue. + */ + this->uaddr = uaddr2; + this->val = targetval; drop_count++; } @@ -2717,6 +2729,8 @@ static int futex_wait(u32 __user *uaddr, unsigned int flags, u32 val, if (!bitset) return -EINVAL; q.bitset = bitset; + q.uaddr = uaddr; + q.val = val; to = futex_setup_timer(abs_time, &timeout, flags, current->timer_slack_ns); @@ -2725,7 +2739,7 @@ static int futex_wait(u32 __user *uaddr, unsigned int flags, u32 val, * Prepare to wait on uaddr. On success, holds hb lock and increments * q.key refs. */ - ret = futex_wait_setup(uaddr, val, flags, &q, &hb); + ret = futex_wait_setup(q.uaddr, q.val, flags, &q, &hb); if (ret) goto out; -- 1.8.3.1
| |