Re: [PATCH v5 2/2] riscv: kprobes: add KUnit test for LR/SC sequence rejection

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Wed, 14 Oct 2026 09:54:54 +0000, sashiko-bot@kernel.org <sashiko-bot@kernel.org> wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] kprobes: stack-allocated kprobe leak and UAF on unexpected test failure
> --
>
> commit 10b8cab517db949a43dcca583dbcd1f20f6d68e8
> Author: Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com>
>
> riscv: kprobes: add KUnit test for LR/SC sequence rejection
>
> This commit extends the kprobes KUnit test to verify that register_kprobe()
> correctly handles an LR/SC sequence. It asserts that probes are accepted
> around the sequence but rejected if placed directly on the LR, SC, or any
> instruction inside the sequence.
>
> > diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.c b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> > index 027424a3ff7b1..2727537569739 100644
> > --- a/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> > +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
>
> [ ... ]
>
> > +static void test_kprobe_lrsc(struct kunit *test)
> > +{
> > +	/*
> > +	 * test_kprobes_lrsc_offsets[] points, in order, at an instruction
> > +	 * right before an LR/SC sequence, the LR itself, an instruction
> > +	 * inside the sequence, the SC, and an instruction right after it.
> > +	 */
> > +	bool reject[] = { false, true, true, true, false };
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(reject); i++) {
> > +		struct kprobe kp = {};
> > +
> > +		kp.pre_handler = kprobe_dummy_handler;
> > +		kp.addr = (kprobe_opcode_t *)test_kprobes_lrsc_offsets[i];
> > +
> > +		if (reject[i]) {
> > +			KUNIT_EXPECT_LT(test, register_kprobe(&kp), 0);
> > +		} else {
> > +			KUNIT_EXPECT_EQ(test, register_kprobe(&kp), 0);
> > +			unregister_kprobe(&kp);
> > +		}
>
> [Severity: High]
> Does this leave the stack-allocated kprobe registered if register_kprobe() 
> unexpectedly succeeds? 

Good catch!

Xiaofeng, can you fix it?

Thanks,

>
> In the rejection path where reject[i] is true, KUNIT_EXPECT_LT() will log 
> a test failure if register_kprobe(&kp) returns 0, but execution will
> continue. Because unregister_kprobe(&kp) is only called in the else block, 
> the kprobe remains in the global kprobe list.
>
> Since struct kprobe kp is allocated on the stack within the loop, advancing
> to the next iteration zeroes out the reused stack memory. When the function 
> eventually returns, the stack frame is popped. This would leave a dangling
> pointer in the kprobes subsystem, causing a use-after-free or kernel panic 
> on subsequent kprobe operations or interrupts.
>
> Should there be a check to unregister the kprobe if the registration 
> unexpectedly succeeds in the reject path?
>
> > +	}
> > +}
>
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261014094415.374-1-yuanxiaofeng@eswincomputing.com?part=2

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>