Re: [PATCH v2] module: Fix use-after-free in module_put() tracepoint

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] module: trace_module_put() records pre-decrement refcnt in module_put()
--

commit 2c168275e42fc7a4482d549cb0c9f1fc9f71e395
Author: Aaron Tomlin <atomlin@atomlin.com>

module: Fix use-after-free in module_put() tracepoint

This commit moves trace_module_put() before atomic_dec_if_positive() in
module_put() to prevent a use-after-free if the thread is preempted after
decrementing the module reference count while the module is being unloaded.

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index f1392c10907a2..70063fea52d03 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -954,9 +954,9 @@ void module_put(struct module *module)
>  	int ret;
>  
>  	if (module) {
> +		trace_module_put(module, _RET_IP_);
>  		ret = atomic_dec_if_positive(&module->refcnt);
>  		WARN_ON(ret < 0);	/* Failed to put refcount */
> -		trace_module_put(module, _RET_IP_);

[Severity: Medium]
Does moving trace_module_put() before atomic_dec_if_positive() in
module_put() cause the module_put trace event to record a reference count
that is off by one?

In include/trace/events/module.h, both module_get and module_put use the
module_refcnt event class, which reads mod->refcnt directly inside
TP_fast_assign():

include/trace/events/module.h:
DECLARE_EVENT_CLASS(module_refcnt,
	...
	TP_fast_assign(
		__entry->ip	= ip;
		__entry->refcnt	= atomic_read(&mod->refcnt);
		__assign_str(name);
	),

In kernel/module/main.c, __module_get() and try_module_get() call
trace_module_get() after incrementing module->refcnt, recording the
post-increment value:

kernel/module/main.c:__module_get() {
	if (module) {
		atomic_inc(&module->refcnt);
		trace_module_get(module, _RET_IP_);
	}
}

With trace_module_put() now invoked before atomic_dec_if_positive() in
module_put() when a non-null module pointer is passed and the module_put
tracepoint is enabled, TP_fast_assign() reads atomic_read(&mod->refcnt)
before the decrement occurs. As a result, a matched get and put pair on a
live module logs the pre-decrement refcnt on module_put instead of the
post-decrement refcnt.

>  	}
>  }
>  EXPORT_SYMBOL(module_put);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022184517.66438-1-atomlin@atomlin.com?part=1