Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] ABA vulnerability in tracefs UAF mitigation due to pointer reuse. --
commit d6f30a79c0307745af03ca13f0fdeb754d0c04a9 Author: Gabriele Monaco <[email protected]> tools/rv: Add selftest for rv bpf Add selftest cases for the rv userspace tool BPF commands and monitors, test BPF monitors registration, unregistration and functionality, including traces and reactors. > diff --git a/tools/verification/rv/tests/rv_bpf.t > b/tools/verification/rv/tests/rv_bpf.t > new file mode 100644 > index 0000000000000..902343f0d3f10 > --- /dev/null > +++ b/tools/verification/rv/tests/rv_bpf.t [ ... ] > +check "already enabled monitor returns error" \ > + "echo 1 > $RVDIR/monitors/nohz/enable; $RV mon nohz" 1 \ > + "monitor nohz (BPF) is already enabled" > +[ -n "$TEST_COUNT" ] && echo 0 > $RVDIR/monitors/nohz/enable > + > +check "already enabled monitor prevents unregistration" \ > + "echo 1 > $RVDIR/monitors/nohz/enable; $RV bpf unregister" 1 \ > + "monitor nohz is enabled, cannot unregister" \ > + "unregistered [0-9]\+ BPF monitor(s)" > +[ -n "$TEST_COUNT" ] && echo 0 > $RVDIR/monitors/nohz/enable > + > +check "unregister BPF monitors" \ > + "$RV bpf unregister" 0 "unregistered [0-9]\+ BPF monitor(s)" [Severity: Medium] Could this sequence of registration and unregistration expose an ABA vulnerability in the tracefs use-after-free mitigation? When a monitor is unregistered here, its struct rv_monitor inside the bpf_struct_ops map can be freed. If a user still holds a tracefs file (such as enable or reactors) open for this monitor, filp->private_data becomes a dangling pointer. If a new monitor is then loaded and allocated at the exact same memory address, the pointer check in monitor_enable_write_data() might pass incorrectly: kernel/trace/rv/rv.c:monitor_enable_write_data() { ... guard(mutex)(&rv_interface_lock); if (!rv_is_monitor_registered(mon)) return -ENODEV; if (val) retval = rv_enable_monitor(mon); ... } Since rv_is_monitor_registered() simply checks if the raw pointer mon exists in rv_monitors_list, it would return true for the new monitor. This would allow a privileged user to accidentally or intentionally alter the state of the newly registered monitor using the old file descriptor. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19
