[PATCH -rcu/dev 2/4] hazptr: Check per-CPU slot owner with CONFIG_HAZPTR_DEBUG

[Andrea Parri <parri.andrea@gmail.com>]

When a per-CPU slot records the wrong hazptr_ctx as its owner, nothing
reports it where it happens.  If the task is switched out before
releasing the slot, hazptr_note_context_switch() promotes it through the
wrong context, and if that context is detached,
hazptr_chain_backup_slot() adds its backup slot to an overflow list
again.

The kernel can then crash later in hazptr_synchronize(), far from the
cause.  For example, from hazptrtorture with readers that nest a second
hazard pointer, as "hazptrtorture: Nest a second hazard pointer in
readers" makes them do, and without "hazptr: Record per-CPU slot owner
after claiming the slot":

    Oops: stack segment: 0000 [#1] SMP PTI
    CPU: 4 UID: 0 PID: 83 Comm: hazptr_torture_ Tainted: G        W           7.3.0-rc1-00107-ga2aa5148421f #2 PREEMPTLAZY
    RIP: 0010:hazptr_scan_overflow_list_period+0x94/0x100
    RBP: dead000000000100 R08: 00000000002fa6bc R09: 0000000000000004
    Call Trace:
     <TASK>
     hazptr_synchronize+0xb7/0xd0
     hazptr_torture_writer+0x181/0x3f0
     kthread+0xd1/0x100
     ret_from_fork+0x179/0x240
     ret_from_fork_asm+0x1a/0x30
     </TASK>

Check the recorded owner where it is used.  hazptr_release_debug() now
warns when the context releasing a per-CPU slot is not the one recorded
for it.  hazptr_note_context_switch() warns when the recorded owner's
->slot is not the slot being promoted.  In that run, the checks warned
1.3 and 0.08 seconds before the oops:

    Hazard Pointer (addr=000000006dd14922) released from a per-CPU slot recorded with another owner. Acquire: caller=hazptr_torture_read_lock+0x43/0xb0, pid=82, cpu=7.
    WARNING: ./include/linux/hazptr.h:304 at hazptr_torture_read_unlock+0x137/0x160, CPU#7: hazptr_torture_/82
    Call Trace:
     <TASK>
     hazptr_torture_reader_tail+0x80/0x220
     hazptr_torture_nested_reader+0x6c/0x90
     hazptr_torture_reader+0x2f5/0x530
     kthread+0xd1/0x100
     ret_from_fork+0x179/0x240
     ret_from_fork_asm+0x1a/0x30
     </TASK>

    Hazard Pointer (addr=00000000742d8665) in per-CPU slot 1 recorded with an owner that uses another slot.
    WARNING: ./include/linux/hazptr.h:185 at __schedule+0x7ba/0xf10, CPU#0: hazptr_torture_/82
    Call Trace:
     <TASK>
     preempt_schedule+0x2d/0x50
     preempt_schedule_thunk+0x16/0x40
     hazptr_torture_read_lock+0xaa/0xb0
     hazptr_torture_nested_reader+0x39/0x90
     hazptr_torture_reader+0x2f5/0x530
     kthread+0xd1/0x100
     ret_from_fork+0x179/0x240
     ret_from_fork_asm+0x1a/0x30
     </TASK>

hazptr_release_debug() reads the owner of ctx->slot only if that
per-CPU slot is in use, and skips the backup slot that a detached or
promoted ctx uses.  If ctx holds the per-CPU slot, no interrupt can
claim it between the ->addr check and the owner read, so the check warns
only if the slot records the wrong owner.

If ctx has already released the slot, which the hazptr_release()
kernel-doc permits, the slot is in use only if another hazard pointer
has claimed it since, and this second release would clear that hazard
pointer, so the check warns then too.

One case can escape both checks: an interrupt handler that acquires and
releases the slot instead of detaching it, then a context switch before
the task's release.  The stale owner's ->slot still names the slot, and
the promotion empties the task's per-CPU slot, so hazptr_release_debug()
warns only if another hazard pointer has claimed that slot by then.

The crash was found with the help of an LLM.

Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 include/linux/hazptr.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h
index 90d8d5a3d2443..d37792af34939 100644
--- a/include/linux/hazptr.h
+++ b/include/linux/hazptr.h
@@ -182,6 +182,9 @@ void hazptr_note_context_switch(void)
 		if (!slot->addr)
 			continue;
 #ifdef CONFIG_HAZPTR_DEBUG
+		WARN_ONCE(item->ctx.ctx->slot != slot,
+			"Hazard Pointer (addr=%p) in per-CPU slot %u recorded with an owner that uses another slot.",
+			slot->addr, idx);
 		item->ctx.ctx->detach_cpu = true;
 #endif
 		hazptr_promote_to_backup_slot(item->ctx.ctx, slot);
@@ -284,6 +287,8 @@ void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr)
 	int pid = current->pid, cpu = smp_processor_id();
 	bool warn_remote_cpu = !ctx->detach_cpu && ctx->acquire_cpu != cpu,
 	     warn_remote_task = !ctx->detach_task && ctx->acquire_pid != pid;
+	struct hazptr_slot *slot = ctx->slot;
+	struct hazptr_slot_item *item;
 
 	WARN_ONCE(warn_remote_cpu || warn_remote_task,
 		"Hazard Pointer (addr=%p) released on remote %s without %s. Acquire: caller=%pS, pid=%d, cpu=%d. Release: pid=%d, cpu=%d.",
@@ -291,6 +296,14 @@ void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr)
 		warn_remote_task ? "task" : "cpu",
 		warn_remote_task ? "being detached from task" : "context switch",
 		(void *) ctx->acquire_caller, ctx->acquire_pid, ctx->acquire_cpu, pid, cpu);
+
+	if (hazptr_slot_is_backup(ctx, slot) || !READ_ONCE(slot->addr))
+		return;
+	/* Whoever claimed a per-CPU slot must be recorded as its owner. */
+	item = container_of(slot, struct hazptr_slot_item, slot);
+	WARN_ONCE(item->ctx.ctx != ctx,
+		"Hazard Pointer (addr=%p) released from a per-CPU slot recorded with another owner. Acquire: caller=%pS, pid=%d, cpu=%d.",
+		addr, (void *) ctx->acquire_caller, ctx->acquire_pid, ctx->acquire_cpu);
 }
 #else
 static inline void hazptr_release_debug(struct hazptr_ctx *ctx, void *addr) { }
-- 
2.53.0