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

[Alexander Potapenko]

On Fri, Jul 24, 2026 at 9:21 PM Karl Mehltretter <kmehltretter@xxxxxxxxx> 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.

Looks good as it is, but increasing the buffer size would make it more useful.

> Fixes: 6cd0dd934b03 ("kcov: Add interrupt handling self test")
> Assisted-by: Claude:claude-opus-4-8

Documentation/process/coding-assistants.rst mandates writing:

  Assisted-by: LLM

I don't have a strong opinion and find listing the specific model name
generally useful, but 816d9992d9ed434ec52cfbd63080d518e535a41b has a
valid point.

> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>
Reviewed-by: Alexander Potapenko <glider@xxxxxxxxxx>


> +#ifdef CONFIG_KCOV_SELFTEST
> +static unsigned long decanonicalize_ip(unsigned long ip)
> +{
> +#ifdef CONFIG_RANDOMIZE_BASE
> +       ip += kaslr_offset();
> +#endif
> +       return ip;
> +}
> +#endif

More of a note to self: fs/pstore/ftrace.c defines functions similar
to canonicalize_ip()/decanonicalize_ip(), we could factor them out
someday.


>  #ifdef CONFIG_KCOV_SELFTEST
> +static unsigned long selftest_area[2] __initdata;

What do you think of increasing the buffer size to, say, 16 elements?
This way we'll be able to detect more than one spurious coverage event
without needing to rebuild the kernel.


> +       if (selftest_area[0]) {
> +               ip = decanonicalize_ip(selftest_area[1]);
> +               pr_err("spurious coverage detected during interrupt selftest: %pS\n",

Nit: should this be %pB?