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

["Gary Guo" <gary@garyguo.net>]

On Fri Sep 4, 2026 at 3:34 PM BST, Aaron Tomlin wrote:
> On Thu, Sep 03, 2026 at 07:13:02PM +0000, sashiko-bot@kernel.org wrote:
>> commit 828a4fa4f29a0a3c5aba9065f4ba35eb845dddef
>> Author: Aaron Tomlin <atomlin@atomlin.com>
>> 
>> module: Extend module_blacklist parameter to built-in modules
>> 
>> This commit extends the module_blacklist command-line parameter to apply to
>> built-in modules. It achieves this by creating a new .initcall.modnames
>> section to map initcalls to module names, intercepting blacklisted builtin
>> initcalls during early boot.
>> 
>> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> > index b2988aa12f664..7490278b7a2df 100644
>> > --- a/include/asm-generic/vmlinux.lds.h
>> > +++ b/include/asm-generic/vmlinux.lds.h
>> > @@ -734,7 +734,8 @@
>> >  	EARLYCON_TABLE()						\
>> >  	LSM_TABLE()							\
>> >  	EARLY_LSM_TABLE()						\
>> > -	KUNIT_INIT_TABLE()
>> > +	KUNIT_INIT_TABLE()						\
>> > +	BOUNDED_SECTION_BY(.initcall.modnames, _initcall_modnames)
>> 
>> [Severity: High]
>> Does this section declaration need an alignment constraint? Without a preceding
>> . = ALIGN(8); or similar, the __start_initcall_modnames label might capture
>> the unaligned location counter before padding is inserted by the linker.
>> 
>> When iterating over this section during early boot in do_initcall_level(),
>> could the kernel read padding bytes as garbage pointers, breaking the
>> builtin module blacklist feature or causing an unaligned memory access panic?
>
> Hi Petr,
>
> I forgot to restore ". = ALIGN(8)" in this iteration.
>
> The following ensures the location counter . within .init.data is aligned
> to an 8-byte boundary before __start_initcall_modnames = . is captured,
> preventing any linker padding from preceding the first element of the
> table.

Have you observed cases where this as an actual problem?

I suppose this could be fixed in the BOUNDED_SECTION_BY instead by doing

    . = ALIGN(ALIGNOF(NEXT_SECTION));

>
> Are you okay with the following?
>
>     --- a/include/asm-generic/vmlinux.lds.h
>     +++ b/include/asm-generic/vmlinux.lds.h
>     @@ -735,6 +735,7 @@
>          LSM_TABLE()                            \
>          EARLY_LSM_TABLE()                        \
>          KUNIT_INIT_TABLE()                        \
>     +    . = ALIGN(8);                            \
>          BOUNDED_SECTION_BY(.initcall.modnames, _initcall_modnames)
>
>      #define INIT_TEXT                            \
>
>
>> > diff --git a/rust/macros/module.rs b/rust/macros/module.rs
>> > index bc7027f8dbb2c..a96157598197f 100644
>> > --- a/rust/macros/module.rs
>> > +++ b/rust/macros/module.rs
>> [ ... ]
>> > @@ -591,6 +594,20 @@ pub extern "C" fn cleanup_module() {
>> >                  #[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)]
>> >                  ::core::arch::global_asm!(#global_asm);
>> >  
>> > +                #[cfg(not(MODULE))]
>> > +                #[used(compiler)]
>> > +                #[link_section = ".init.rodata"]
>> > +                static #ident_modname_str: [u8; #name_len] = *#name_cstr.to_bytes_with_nul();
>> 
>> [Severity: Low]
>> Will this cause a compilation failure for built-in Rust modules?
>> 
>> It attempts to initialize a fixed-size array by dereferencing a dynamically
>> sized slice returned by to_bytes_with_nul(). This could break the build when
>> CONFIG_MODULES=n or when a Rust module is configured as built-in.
>
> Hi Gary,
>
> Calling .to_bytes_with_nul() on &CStr returns a slice reference (&[u8]), so
> dereferencing it yields an unsized slice ([u8]).
>
> To resolve this, we can follow the pattern already used in
> rust/macros/module.rs for emitting '.modinfo' entries construct a byte
> string literal at macro-expansion time using Literal::byte_string(). In
> Rust, a byte string literal (b"...\0") has type &'static [u8; N], which can
> be cleanly dereferenced with '*' into a fixed-size array [u8; N].
>
> Would the following be appropriate?

Looks reasonable.

Best,
Gary

>
> diff --git a/rust/macros/module.rs b/rust/macros/module.rs
> index a96157598197..67e3df8d4389 100644
> --- a/rust/macros/module.rs
> +++ b/rust/macros/module.rs
> @@ -493,7 +493,9 @@ pub(crate) fn module(info: ModuleInfo) -> Result<TokenStream> {
>      );
>
>      let name_cstr = CString::new(name.value()).expect("name contains NUL-terminator");
> -    let name_len = name_cstr.to_bytes_with_nul().len();
> +    let name_bytes = name_cstr.to_bytes_with_nul();
> +    let name_len = name_bytes.len();
> +    let name_byte_literal = Literal::byte_string(name_bytes);
>
>      Ok(quote! {
>          /// The module name.
> @@ -596,7 +598,7 @@ pub extern "C" fn cleanup_module() {
>                  #[cfg(not(MODULE))]
>                  #[used(compiler)]
>                  #[link_section = ".init.rodata"]
> -                static #ident_modname_str: [u8; #name_len] = *#name_cstr.to_bytes_with_nul();
> +                static #ident_modname_str: [u8; #name_len] = *#name_byte_literal;
>
>                  #[cfg(not(MODULE))]
>                  #[used(compiler)]
>
>
> Kind regards,