Re: [PATCH v4 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 dynamically loaded modules to register reactors. However, `rv_unregister_reactor()` fails to detach the reactor from monitors currently using it, and `struct rv_reactor` lacks an `owner` field to pin the module in memory. This leads to a dangling pointer and use-after-free when a reactor module is unloaded.
--

commit f153a63763986ecb001f784d45c5262d45c8d0a5
Author: Wen Yang <wen.yang@linux.dev>
Subject: rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

This commit exports rv_register_reactor() and rv_unregister_reactor() so
that dynamically loaded modules and KUnit tests can register and unregister
reactors without link-time errors.

> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index afc97d0971091..606c530de2f04 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);

[ ... ]

> @@ -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]
By exporting these functions, dynamic modules can now register and unregister
reactors. However, when a module is unloaded and calls rv_unregister_reactor(),
does this leave dangling pointers in monitors that are currently using it?

Looking at how monitors assign reactors, struct rv_reactor lacks an owner
field to pin the module in memory:

kernel/trace/rv/rv_reactors.c:monitor_swap_reactors_single() {
    ...
	mon->reactor = reactor;
	mon->react = reactor->react;
    ...
}

Since rv_unregister_reactor() only removes the reactor from the global list
without clearing active monitor references, won't rv_react() later execute a
dangling pointer when an event triggers?

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

Could this result in a use-after-free pointing to unmapped module memory if
an authorized user unloads a reactor module that is still attached to an
active monitor?

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