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

On Mon, Aug 03, 2026 at 05:55:28PM +0200, Petr Pavlu wrote:
> > Design Considerations and Trade-offs:
> > 
> >     1.  LTO and CFI Compatibility vs. PREL32
> > 
> >         Previous iterations of this patch attempted to use top-level
> >         inline assembly to generate 32-bit relative offsets (PREL32) to
> >         save memory. However, raw inline assembly operates blindly
> >         outside of the C compiler's visibility. When compiled with
> >         CONFIG_LTO_CLANG or CONFIG_CFI_CLANG, the compiler applies
> >         symbol renaming and generates Control Flow Integrity stubs.
> >         The raw assembly string-matching fails to track these changes,
> >         resulting in undefined references or runtime address mismatches.
> > 
> >         To resolve this, we strictly use standard C structures to hold
> >         the function pointers. This natively allows the compiler to
> >         resolve LTO renaming and map CFI stubs correctly. We trade the
> >         minor spatial optimisation of PREL32 (using absolute 64-bit
> >         pointers instead) to guarantee architectural safety under modern
> >         compiler protections. Because this metadata is placed in an
> >         ".init" section and freed entirely after boot, the temporary
> >         memory overhead is negligible.
> > 
> >     2.  Architectural Safety and Elimination of Runtime Vulnerabilities:
> > 
> >         By embedding the boot-time blacklist check inside the
> >         do_one_initcall_builtin() __init wrapper function, we ensure
> >         the metadata lookup logic is exclusively invoked during early
> >         boot. This approach provides strict structural guarantees:
> >         - It inherently eliminates Use-After-Free (UAF) and race conditions
> >           since loadable modules (which execute post-boot and invoke
> >           do_one_initcall() directly) bypass this __init wrapper entirely.
> >         - It prevents modpost section mismatch warnings since the __init
> >           metadata is strictly accessed by other __init functions.
> >         - It mitigates Spectre v1 speculative execution vulnerabilities
> >           by guaranteeing the unprivileged runtime module loading path
> >           cannot speculatively branch into reclaimed .init.text instructions.
> 
> I suggest moving these design notes below the --- separator, together
> with the diffstat.

Acknowledged.

> > Signed-off-by: Aaron Tomlin <[email protected]>
> > ---
> >  include/asm-generic/vmlinux.lds.h |  4 ++-
> >  include/linux/init.h              | 27 +++++++++++++++-
> >  include/linux/module.h            |  4 ++-
> >  init/main.c                       | 54 +++++++++++++++++++++++++++++--
> >  kernel/module/main.c              | 22 +------------
> 
> The Rust module support in rust/macros/module.rs should be updated
> similarly to maintain feature parity.

Acknowledged.

> > +#define ___define_initcall_modname(fn, id, __sec)			\
> > +	____define_initcall_modname(fn, id, __sec, __initcall_id(fn))
> > +
> > +#define __define_initcall_modname(fn, id)				\
> > +	___define_initcall_modname(fn, id, .initcall##id)
> 
> These two macros can be merged into:

Acknowledged.

> #define __define_initcall_modname(fn, id)				\
> 	___define_initcall_modname(fn, id, .initcall##id, __initcall_id(fn))
> 
> > +
> > +#define __builtin_module_initcall(fn)	__define_initcall_modname(fn, 6)
> > +
> > +#define ___define_initcall(fn, id, __sec)				\
> >  	__unique_initcall(fn, id, __sec, __initcall_id(fn))
> >  
> >  #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 7566815fabbe..ecc4db15ff4a 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -86,7 +86,7 @@ extern void cleanup_module(void);
> >   * builtin) or at module insertion time (if a module).  There can only
> >   * be one per module.
> >   */
> > -#define module_init(x)	__initcall(x);
> > +#define module_init(initfn)	__builtin_module_initcall(initfn);
> 
> Renaming the macro parameter from `x` to `initfn` is unnecessary in this
> patchset and also makes the preceding comment inconsistent, since it
> still refers to `x`.

Acknowledged.

> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index 46dd8d25a605..5c90ebedbf68 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -2919,26 +2919,6 @@ int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
> >  	return 0;
> >  }
> >  
> > -/* module_blacklist is a comma-separated list of module names */
> > -static char *module_blacklist;
> > -static bool blacklisted(const char *module_name)
> > -{
> > -	const char *p;
> > -	size_t len;
> > -
> > -	if (!module_blacklist)
> > -		return false;
> > -
> > -	for (p = module_blacklist; *p; p += len) {
> > -		len = strcspn(p, ",");
> > -		if (strlen(module_name) == len && !memcmp(module_name, p, len))
> > -			return true;
> > -		if (p[len] == ',')
> > -			len++;
> > -	}
> > -	return false;
> > -}
> > -core_param(module_blacklist, module_blacklist, charp, 0400);
> >  
> 
> This still leaves two blank lines between module_frob_arch_sections()
> and layout_and_allocate(), instead of just one.

Acknowledged.


Kind regards,
-- 
Aaron Tomlin