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 <[email protected]> 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/[email protected]?part=1
