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

[sashiko-bot@kernel.org]

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? 

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