Re: [PATCH] futex/requeue: Revert "Prevent NULL pointer dereference in remove_waiter() on self-deadlock""

[Thomas Gleixner <tglx@kernel.org>]

On Wed, Jul 01 2026 at 15:11, Sebastian Andrzej Siewior wrote:
> The commit cited below should not have been merged. It did not fix an
> existing problem but it introduced new problems by keeping the pi_state
> in state Q_REQUEUE_PI_IN_PROGRESS and leaking it.
>
> Based on the commit description the intention was to handle the case
> when task_blocks_on_rt_mutex() returns -EDEADLK and the following
> remove_waiter() dereferences the NULL pointer in waiter->task.
>
> That has been already handled by Davidlohr in commit 40a25d59e85b3
> ("locking/rtmutex: Skip remove_waiter() when waiter is not enqueued")
> and requires no further acting.

No. If the self deadlock is obvious, then this should not even go into
the proxy lock code. I clearly failed to notice the leak problem, but
reverting it and relying on some magic down the road to handle it
correctly is not really solid.

The below should fix it properly.

---
--- a/kernel/futex/requeue.c
+++ b/kernel/futex/requeue.c
@@ -646,15 +646,13 @@ int futex_requeue(u32 __user *uaddr1, un
 			}
 
 			/* Self-deadlock: non-top waiter already owns the PI futex. */
-			if (rt_mutex_owner(&pi_state->pi_mutex) == this->task) {
+			if (likely(rt_mutex_owner(&pi_state->pi_mutex) != this->task)) {
+				ret = rt_mutex_start_proxy_lock(&pi_state->pi_mutex,
+								this->rt_waiter, this->task);
+			} else {
 				ret = -EDEADLK;
-				break;
 			}
 
-			ret = rt_mutex_start_proxy_lock(&pi_state->pi_mutex,
-							this->rt_waiter,
-							this->task);
-
 			if (ret == 1) {
 				/*
 				 * We got the lock. We do neither drop the refcount