[PATCH] hazptrtorture: Only detach acquired hazard pointers

[Bradley Morgan]

hazptr_torture_acquire() detaches unconditionally, even when the
readlock fails. A failed acquire leaves nothing to detach, but the
detach still promotes the context to its backup slot and chains that
slot into the running CPU's overflow list. The reader then retries on
its own CPU, the fast path hands out a per-CPU slot and overwrites
ctx->slot, and the chained backup node is orphaned, still linked,
with nobody left to unchain it.

The next detach of the same context chains the same node a second
time, into another CPU's list, and the node ends up reachable from
both. The eventual release unchains it once, hlist_del() poisons
node->next, and the first list is left pointing at the poisoned node.
The writer's next hazptr_synchronize() walks that list, steps onto
LIST_POISON1 (0x100 on i386, where POISON_POINTER_DELTA is 0), and
reads slot.addr at offset 8 of the backup slot, address 0x108, which
is the crash the robot hit.

  cpuA (IPI acquire)     cpuR (reader)          cpuD (do_pending)
  ---------------------  ---------------------  -------------------
  readlock() returns
    NULL
  detach chains the
    backup node into
    cpuA list
                         hpp_htp is NULL,
                           continue
                         reacquire, ctx->slot
                           is now a cpuR
                           per-CPU slot
                         acquire succeeds,
                           defer, detach chains
                           the SAME node into
                           cpuR list
                                                release, unchain
                                                  once, node->next
                                                  is POISON1
                                                kfree(hppp)
  synchronize walks cpuA
    list, node->next is
    0x100, reads 0x108,
    Oops

Skip the detach when the acquire failed. The slot holds NULL in that
case, note_context_switch() and the synchronize scanners skip NULL
slots, and the next acquire overwrites ctx->slot, so leaving the
context attached is safe.

The robot's original report was against the defer path before detach
existed, which 4bd7f458229a fixed. This is the same crash surviving
through the IPI acquire path that 6357ec235c59 added.

Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue")
Reported-by: kernel test robot <yi1.lai@xxxxxxxxx>
Closes: https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@xxxxxxxxx
Signed-off-by: Bradley Morgan <brads@xxxxxxxxxxxxxx>
---
 kernel/rcu/hazptrtorture.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/rcu/hazptrtorture.c b/kernel/rcu/hazptrtorture.c
index 7c8b589..267f262 100644
--- a/kernel/rcu/hazptrtorture.c
+++ b/kernel/rcu/hazptrtorture.c
@@ -373,8 +373,11 @@ static void hazptr_torture_acquire(void *hppp_in)
 	/*
 	 * Acquiring a hazard pointer from a remote CPU.
 	 * Detach hazptr from its task so it can be released by another task.
+	 * A failed acquire has nothing to detach, and detaching one anyway
+	 * orphans the chained backup slot on this CPU's overflow list.
 	 */
-	hazptr_detach(&hppp->hpp_hc);
+	if (hppp->hpp_htp)
+		hazptr_detach(&hppp->hpp_hc);
 	atomic_long_inc(per_cpu_ptr(&hazptr_torture_acquires_irq, raw_smp_processor_id()));
 }
 
-- 
2.47.3


--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@xxxxxxxxx/