Messages in this thread Patch in this message | | | Date | Mon, 15 Jan 2024 19:33:38 +0100 | From | Sebastian Andrzej Siewior <> | Subject | Re: [PATCH v3 7/7] locking/rtmutex: Acquire the hb lock via trylock after wait-proxylock. |
| |
On 2024-01-15 13:54:32 [+0100], Jiri Slaby wrote: > A simplified reproducer attached (in particular, no APR anymore). Build with > -pthread, obviously. If you see > BADx rv=22 > > that's bad.
So task1 owns the futex, task2 does lock_pi_futex(). task2 setups the pi state and everything for task1. Then task1 times out on the lock and does not want the lock no more. And we break user state vs kernel via:
task1 task2
futex_unlock_pi(); futex_lock_pi(); rt_mutex_wait_proxy_lock() // ETIMEDOUT spin_lock(&hb->lock); rt_mutex_cleanup_proxy_lock() /* now the lock is still owned by task1, and the * task2 removed itself as the waiter but its * futex_q is still queued */ spin_lock(&hb->lock); /* block */
top_waiter = futex_top_waiter(hb, &key); /* top_wait is task2's */ rt_waiter = rt_mutex_top_waiter(&pi_state->pi_mutex); /* rt_waiter is NULL and the futex is unlocked in userland via uaddr */
and now
task 3 task4 locks in userland futex_lock_pi(); futex_lock_pi_atomic(); -EINVAL = attach_to_pi_state() /* * becauase pi_state says owner * is task1 but uaddr says task3. */
\*/
This is due to the new lock ordering and the guarantees we no longer have since the commit cited. The pi-state is cleaned/ removed by the last one that wants the lock so in the unlock path there is either pi-state with a waiter or nothing.
This duct tape at the end waits until the pi-state leaves or we get a waiter. So this works but is not a fix.
diff --git a/kernel/futex/pi.c b/kernel/futex/pi.c index 90e5197f4e56..f504ae864cc9 100644 --- a/kernel/futex/pi.c +++ b/kernel/futex/pi.c @@ -1182,6 +1182,9 @@ int futex_unlock_pi(u32 __user *uaddr, unsigned int flags) rt_waiter = rt_mutex_top_waiter(&pi_state->pi_mutex); if (!rt_waiter) { raw_spin_unlock_irq(&pi_state->pi_mutex.wait_lock); + spin_unlock(&hb->lock); + cpu_relax(); + goto retry; goto do_uncontended; } -- 2.43.0 > regards,
Sebastian
| |