Re: [PATCH v5 1/2] module: Extend module_blacklist parameter to built-in modules

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] A fatal modpost section mismatch warning occurs when CONFIG_MODULES=y because a non-__init function references an __init function.
- [High] An unsynchronized TOCTOU race condition in `do_one_initcall()` allows concurrent loadable module initialization to execute freed `__init` memory and access freed `__initdata`.
- [High] The temporal check `system_state < SYSTEM_FREEING_INITMEM` is vulnerable to Spectre v1 branch misprediction, leading to speculative execution of freed `__init` code and speculative reading of freed `__initdata`.
--

--- Patch [1]: [PATCH v5 1/2] module: Extend module_blacklist parameter to built-in modules ---
commit befcdb365a33b92bbd551da11437b39b818bdea2
Author: Aaron Tomlin <atomlin@atomlin.com>
Subject: module: Extend module_blacklist parameter to built-in modules

This patch extends the module_blacklist command-line parameter to apply to
built-in modules by checking an .initcall.modnames table during early boot.
It moves the blacklisting logic to init/main.c and intercepts initcalls in
do_one_initcall().

> diff --git a/init/main.c b/init/main.c
> --- a/init/main.c
> +++ b/init/main.c
[ ... ]
> @@ -1365,12 +1365,58 @@ static const char *__init get_builtin_modname(initcall_t fn)
>  {
>  	struct initcall_modname *p;
>  
>  	for (p = __start_initcall_modnames; p < __stop_initcall_modnames; p++) {
>  		if (dereference_function_descriptor(p->initcall_fn) ==
>  		    dereference_function_descriptor(fn))
>  			return p->modname;
>  	}
>  	return NULL;
>  }
>  
>  int __init_or_module do_one_initcall(initcall_t fn)
>  {
>  	int count = preempt_count();
>  	char msgbuf[64];
> +	const char *modname = NULL;
>  	int ret;
>  
> +	if (system_state < SYSTEM_FREEING_INITMEM && module_blacklist) {

[Severity: High]
Can this unsynchronized system_state check race with concurrent module loading?

If a driver or subsystem schedules an asynchronous task that triggers
request_module(), a worker thread could evaluate this system_state check as
true. Concurrently, another CPU could advance the state and execute
free_initmem(), unmapping and freeing all .init.text and .init.data pages.

If this happens, the worker thread would then call get_builtin_modname(),
executing freed .init.text instructions and scanning the freed
.initcall.modnames array.


[Severity: High]
Is this temporal check vulnerable to Spectre v1 branch misprediction?

During early boot, do_one_initcall() is heavily executed for built-in
modules, training the CPU branch predictor to predict this condition as
true. At runtime (system_state == SYSTEM_RUNNING), an unprivileged user
triggering a module load might cause the CPU to speculatively mispredict
the branch as true.

If the branch is mispredicted, the CPU would speculatively execute a direct
call to the freed .init.text section in get_builtin_modname(), potentially
creating an arbitrary speculative execution gadget if an attacker has
reallocated that memory.

> +		modname = get_builtin_modname(fn);

[Severity: High]
Does this introduce a modpost section mismatch warning when CONFIG_MODULES=y?

When module support is enabled, __init_or_module evaluates to an empty macro,
placing do_one_initcall() in the persistent .text section. Because it
directly calls get_builtin_modname(), which is annotated with __init, modpost
will statically flag this cross-section reference since it doesn't perform
control flow analysis to evaluate the runtime system_state guard.

> +		if (modname && module_is_blacklisted(modname)) {
> +			pr_info("Skipping initcall for blacklisted built-in module %s\n",
> +				modname);
> +			return 0;
> +		}
> +	}
> +
>  	if (initcall_blacklisted(fn))
>  		return -EPERM;

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