On Wed, 14 Oct 2026 09:54:54 +0000, [email protected] <[email protected]> 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 <[email protected]> > > 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/[email protected]?part=2 -- Masami Hiramatsu (Google) <[email protected]>
