Re: [PATCH v2] kcov: report the first spurious PC in the interrupt selftest

[Bradley Morgan <include@grrlz.net>]

On 24 July 2026 20:21:22 BST, Karl Mehltretter <kmehltretter@gmail.com>
wrote:
>The KCOV interrupt selftest enables KCOV_MODE_TRACE_PC without a
>coverage area so that spurious coverage causes a fault. If the fault
>path is instrumented, the coverage callback faults recursively.
>Observed failure modes include a stack overflow on x86_64 and arm32
>getting stuck in abort handling, neither of which identifies the
>original coverage event.
>
>The recursive failure is not new, but commit 9a79524d1420 ("kcov: use
>WRITE_ONCE() for selftest mode stores") made the selftest effective on
>configurations where the compiler had previously removed the mode
>store, exposing it more broadly.
>
>Use a two-word buffer to record the first spurious PC. Once one is
>recorded, disable KCOV, report the PC, and panic. This preserves the
>selftest's hard failure while avoiding the recursive fault path.
>
>Add decanonicalize_ip() as the inverse of canonicalize_ip(), which
>subtracts kaslr_offset() from recorded PCs, and use it before printing
>the PC with %pS.
>
>Fixes: 6cd0dd934b03 ("kcov: Add interrupt handling self test")
>Assisted-by: Claude:claude-opus-4-8
>Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>

the selftest keeps its hard failure but now names the offending PC
instead of recursing into an undebuggable fault, and the enable
ordering lives in kcov_start()/kcov_stop() instead of a second open
coded copy. LGTM,

Reviewed-by: Bradley Morgan <include@grrlz.net>


>---
>Changes in v2:
>- Use kcov_start() and kcov_stop() instead of open-coding their logic.
>- Add decanonicalize_ip() next to canonicalize_ip() and use it when
>  reporting the recorded PC.
>- Retest under QEMU on arm64, mips64le, and x86_64.
>
>This patch improves the failure diagnostics.
>
>With CONFIG_KCOV_SELFTEST, I observed failures under QEMU on arm,
>arm64, riscv64, s390x, ppc64le, mips64le, loongarch, and x86_64. The
>reported PCs indicate that this likely needs a global KCOV fix rather
>than separate exclusions for each architecture.
>
>For example, arm64 defconfig with KCOV and KCOV_SELFTEST reports:
>
>  [    2.741175] kcov: running self test
>  [    2.748637] kcov: spurious coverage detected during interrupt selftest: return_address+0x28/0xa8
>  [    2.750623] Kernel panic - not syncing: kcov: interrupt selftest detected spurious coverage
>  [    2.752147] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.2.0-rc3-...-dirty #25 PREEMPT(full)
>
> kernel/kcov.c | 40 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 32 insertions(+), 8 deletions(-)
>
>diff --git a/kernel/kcov.c b/kernel/kcov.c
>index 1df373fb562b..7ddfd01ccd8a 100644
>--- a/kernel/kcov.c
>+++ b/kernel/kcov.c
>@@ -202,6 +202,16 @@ static notrace unsigned long canonicalize_ip(unsigned long ip)
> 	return ip;
> }
> 
>+#ifdef CONFIG_KCOV_SELFTEST
>+static unsigned long decanonicalize_ip(unsigned long ip)
>+{
>+#ifdef CONFIG_RANDOMIZE_BASE
>+	ip += kaslr_offset();
>+#endif
>+	return ip;
>+}
>+#endif
>+
> /*
>  * Entry point from instrumented code.
>  * This is called once per basic-block/edge.
>@@ -1102,9 +1112,11 @@ struct kcov_common_handle_id kcov_common_handle(void)
> EXPORT_SYMBOL(kcov_common_handle);
> 
> #ifdef CONFIG_KCOV_SELFTEST
>+static unsigned long selftest_area[2] __initdata;
>+
> static void __init selftest(void)
> {
>-	unsigned long start;
>+	unsigned long start, ip;
> 
> 	pr_err("running self test\n");
> 	/*
>@@ -1114,15 +1126,27 @@ static void __init selftest(void)
> 	 * leaks out of that section and leads to spurious coverage.
> 	 * It's hard to call the actual interrupt handler directly,
> 	 * so we just loop here for a bit waiting for a timer interrupt.
>-	 * We set kcov_mode to enable tracing, but don't setup the area,
>-	 * so any attempt to trace will crash. Note: we must not call any
>-	 * potentially traced functions in this region.
>+	 * We set up a two-word coverage area rather than leaving it NULL:
>+	 * a leak then records its PC instead of crashing on a NULL
>+	 * dereference, and we can report the offending PC. Note: we
>+	 * must not call any potentially traced functions in this region.
> 	 */
>+	kcov_start(current, NULL, ARRAY_SIZE(selftest_area),
>+		   selftest_area, KCOV_MODE_TRACE_PC, 0);
> 	start = jiffies;
>-	WRITE_ONCE(current->kcov_mode, KCOV_MODE_TRACE_PC);
>-	while ((jiffies - start) * MSEC_PER_SEC / HZ < 300)
>-		;
>-	WRITE_ONCE(current->kcov_mode, 0);
>+	while ((jiffies - start) * MSEC_PER_SEC / HZ < 300) {
>+		if (READ_ONCE(selftest_area[0]))
>+			break;
>+		cpu_relax();
>+	}
>+	kcov_stop(current);
>+
>+	if (selftest_area[0]) {
>+		ip = decanonicalize_ip(selftest_area[1]);
>+		pr_err("spurious coverage detected during interrupt selftest: %pS\n",
>+		       (void *)ip);
>+		panic("kcov: interrupt selftest detected spurious coverage");
>+	}
> 	pr_err("done running self test\n");
> }
> #endif
>

Thanks!