Re: [PATCH v6 3/4] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Exporting `rv_register_reactor` and `rv_unregister_reactor` allows loadable modules to register reactors, but failing to detach the reactor from active monitors creates a critical execute-after-free vulnerability when the module is unloaded.
--

commit 516262534066676c40b564e9e3018cd406dd5532
Author: Wen Yang <wen.yang@linux.dev>

rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

This commit exports rv_register_reactor() and rv_unregister_reactor()
with EXPORT_SYMBOL_GPL() so that reactor modules and the tristate KUnit
test module can register and unregister reactors.

> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index ff7d478227c32..8e57a95d446b5 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -314,6 +314,7 @@ int rv_register_reactor(struct rv_reactor *reactor)
>  	guard(mutex)(&rv_interface_lock);
>  	return __rv_register_reactor(reactor);
>  }
> +EXPORT_SYMBOL_GPL(rv_register_reactor);

[Severity: High]
Does exporting this API without module pinning allow an execute-after-free
if a module is unloaded?

Although the commit message notes that actual pinning is left to a follow-up
series, exporting the API in this patch makes it immediately usable and
inherently unsafe for any module. This includes the in-tree KUnit module,
which can race with a sysfs write during its test execution.

>  
>  /**
>   * rv_unregister_reactor - unregister a rv reactor.
> @@ -327,6 +328,7 @@ int rv_unregister_reactor(struct rv_reactor *reactor)
>  	list_del(&reactor->list);
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(rv_unregister_reactor);

[Severity: High]
Can unregistering a reactor from a loadable module leave a dangling pointer
in an active monitor?

If a loadable module registers a reactor, and userspace assigns it to an
active monitor via /sys/kernel/tracing/rv/monitors/*/reactors, unloading
the module calls rv_unregister_reactor().

This function removes the reactor from the global list but fails to detach
it from the active monitor. When the monitor subsequently triggers, it will
dereference the freed memory in rv_react():

kernel/trace/rv/rv_reactors.c:rv_react() {
    ...
    monitor->react(msg, args);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789412799.git.wen.yang@linux.dev?part=3