Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The validation check `rv_is_monitor_registered` is vulnerable to the ABA problem, allowing operations on a stale file descriptor to improperly manipulate a newly registered monitor. --
commit abab0e30c495cd011906fb219545b8b62d268a11 Author: Gabriele Monaco <[email protected]> rv: Handle unregistered monitors safely in tracefs This patch adds checks to ensure that a tracefs file descriptor's private_data still points to a registered monitor before allowing read or write operations. It aims to prevent use-after-free scenarios when pluggable BPF monitors are dynamically unregistered while their files are still open. > diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c > index 33919c5fbd6ad..6ddd3b92da1db 100644 > --- a/kernel/trace/rv/rv.c > +++ b/kernel/trace/rv/rv.c > @@ -245,13 +245,31 @@ struct rv_monitor *rv_get_monitor_by_name(const char > *name) > /* > * This section collects the monitor/ files and folders. > */ > + > +bool rv_is_monitor_registered(struct rv_monitor *mon) > +{ > + struct rv_monitor *m; > + > + list_for_each_entry_rcu(m, &rv_monitors_list, list, > + lockdep_is_held(&rv_interface_lock)) { > + if (m == mon) > + return true; [Severity: Medium] Is it possible for a newly registered monitor to be allocated at the exact same memory address as a previously unregistered monitor? If a privileged user opens a monitor's tracefs file, the monitor is then unloaded (freeing its memory), and a subsequently loaded BPF monitor happens to be allocated at that exact same memory address, it appears this pointer equality check would incorrectly evaluate to true for the stale file descriptor. > + } > + return false; > +} [ ... ] > @@ -384,6 +402,9 @@ static ssize_t monitor_enable_write_data(struct file > *filp, const char __user *u > > guard(mutex)(&rv_interface_lock); > > + if (!rv_is_monitor_registered(mon)) > + return -ENODEV; > + > if (val) > retval = rv_enable_monitor(mon); [Severity: Medium] If the registration check passes due to the reallocation scenario described above, could a write to a stale file descriptor improperly enable or disable the newly registered, unrelated monitor? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
