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? 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
