Re: [RFC PATCH 19/20] tools/rv: Add selftest for rv bpf

[sashiko-bot@kernel.org]

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 <gmonaco@redhat.com>

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/20260831090524.106845-1-gmonaco@redhat.com?part=19