On Sun, Sep 06, 2026 at 02:39:12PM +0100, Gary Guo wrote:
> On Fri Sep 4, 2026 at 3:34 PM BST, Aaron Tomlin wrote:
> > On Thu, Sep 03, 2026 at 07:13:02PM +0000, [email protected] wrote:
> >> commit 828a4fa4f29a0a3c5aba9065f4ba35eb845dddef
> >> Author: Aaron Tomlin <[email protected]>
> >> 
> >> 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));

Hi Gary,

To answer your question, no. I have not observed it causing a real failure
in our current x86_64 build. This is because KUNIT_INIT_TABLE() happens to
precede it in INIT_DATA, and KUNIT_INIT_TABLE() explicitly sets '. = ALIGN(8);'
and contains 8-byte pointers, so the location counter coincidentally
arrived at an 8-byte aligned address.

However, relying on whichever section happens to precede it to maintain
8-byte alignment across all architectures and config combinations feels
fragile.

Regarding using ALIGNOF(sec): in GNU ld, ALIGNOF() only works on output
sections. Because .initcall.modnames is an input section within the
.init.data output section, GNU ld rejects it with: undefined section
.initcall.modnames referenced in expression

Placing '. = ALIGN(8);' directly before the section matches how other
8-byte aligned tables in vmlinux.lds.h (such as LSM_TABLE, EARLY_LSM_TABLE,
and KUNIT_INIT_TABLE) ensure their start labels are properly aligned.


> > 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.


Thank you Gary!


Kind regards,
-- 
Aaron Tomlin

Reply via email to