commit 6d4514ca9cdf ("futex: Prevent robust futex exit race some more")
fixed a lost wakeup in handle_futex_death(): when a robust waiter dies
mid-lock (list_op_pending set) while the futex is owned by a third thread
with FUTEX_WAITERS clear, the kernel must still wake the next waiter,
otherwise a waiter parked behind that futex sleeps forever. Since the
dying thread is not the owner, FUTEX_OWNER_DIED must not be set either.Add a regression test to robust_list.c reproducing this "state B" sequence: a waiter parks on a futex whose word is a foreign TID without FUTEX_WAITERS, and a robust thread then dies with that futex in its list_op_pending. It reuses the existing robust-list scaffolding. Without the fix the wakeup is lost and futex_wait() times out at FUTEX_TIMEOUT; with it handle_futex_death() issues the wakeup. Checked both ways: the test fails on a kernel without the fix and passes on one with it. Signed-off-by: Zhan Xusheng <[email protected]> --- .../selftests/futex/functional/robust_list.c | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tools/testing/selftests/futex/functional/robust_list.c b/tools/testing/selftests/futex/functional/robust_list.c index b3fab60181d5..be8149cc67f8 100644 --- a/tools/testing/selftests/futex/functional/robust_list.c +++ b/tools/testing/selftests/futex/functional/robust_list.c @@ -434,6 +434,85 @@ TEST(test_set_list_op_pending) ksft_test_result_fail("%s\n", __func__); } +/* + * State B reproducer for commit 6d4514ca9cdf ("futex: Prevent robust futex + * exit race some more"). + * + * Model the sequence where the original owner unlocked and woke us, a third + * thread then took the futex uncontended (so the futex word holds a foreign + * TID with FUTEX_WAITERS clear), and we die mid-lock before acquiring it or + * re-arming FUTEX_WAITERS. handle_futex_death() must still wake the next + * waiter, otherwise a waiter parked behind a foreign-owned futex whose + * FUTEX_WAITERS bit was lost sleeps forever. As the dying thread is not the + * owner, it must not set FUTEX_OWNER_DIED either. + */ +#define FOREIGN_TID 0x00c0ffee + +static int child_fn_foreign_owner(void *arg) +{ + struct lock_struct *lock = arg; + struct robust_list_head head; + + if (set_list(&head)) { + ksft_test_result_fail("set_robust_list error\n"); + return -1; + } + if ((unsigned int)gettid() == FOREIGN_TID) { + ksft_test_result_fail("TID collided with FOREIGN_TID\n"); + return -1; + } + + /* We are mid-lock ... */ + head.list_op_pending = &lock->list; + /* ... and the futex is owned by a third thread, FUTEX_WAITERS clear. */ + atomic_store(&lock->futex, FOREIGN_TID); + + /* Let the parent enter futex_wait() before we die (see child_fn_lock). */ + pthread_barrier_wait(&barrier); + usleep(SLEEP_US); + + return 0; +} + +/* + * A waiter is parked on a futex whose word is a foreign TID with no + * FUTEX_WAITERS bit (state B). A robust thread dies with that futex in its + * list_op_pending. The kernel must wake the parked waiter even though the + * dying thread does not own the futex, and it must not set FUTEX_OWNER_DIED. + */ +TEST(test_robust_death_foreign_owner_wakeup) +{ + struct lock_struct lock = { .futex = 0 }; + struct timespec to = { .tv_sec = FUTEX_TIMEOUT, .tv_nsec = 0 }; + int ret, pid, wstatus; + + ret = pthread_barrier_init(&barrier, NULL, 2); + ASSERT_EQ(ret, 0); + + pid = create_child(&child_fn_foreign_owner, &lock); + ASSERT_NE(pid, -1); + + pthread_barrier_wait(&barrier); + + /* + * *futex == FOREIGN_TID here, so this blocks. With the fix the dying + * child's robust walk wakes us; without it, futex_wait() times out. + */ + ret = futex_wait((futex_t *)&lock.futex, FOREIGN_TID, &to, 0); + ASSERT_EQ(ret, 0); + + /* The dying thread was not the owner: OWNER_DIED must not be set. */ + ASSERT_FALSE(lock.futex & FUTEX_OWNER_DIED); + + wait(&wstatus); + pthread_barrier_destroy(&barrier); + + if (!WEXITSTATUS(wstatus)) + ksft_test_result_pass("%s\n", __func__); + else + ksft_test_result_fail("%s\n", __func__); +} + #define CHILD_NR 10 static int child_lock_holder(void *arg) -- 2.43.0
