Re: [PATCH v4] module: Extend module_blacklist parameter to built-in modules

[Aaron Tomlin]

On Tue, Jul 14, 2026 at 04:46:08PM +0200, Petr Pavlu wrote:
> > @@ -734,7 +734,9 @@
> >  	EARLYCON_TABLE()						\
> >  	LSM_TABLE()							\
> >  	EARLY_LSM_TABLE()						\
> > -	KUNIT_INIT_TABLE()
> > +	KUNIT_INIT_TABLE()						\
> > +	. = ALIGN(8);							\
> 
> Is ALIGN(8) sufficient, or is STRUCT_ALIGN() needed instead?

Hi Petr,

Thank you for your feedback.

I can use STRUCT_ALIGN() instead of ALIGN(8) to match structure alignment.

> > +	BOUNDED_SECTION_PRE_LABEL(.initcall.modnames, initcall_modnames, __start_, __stop_)
> 
> Nit: I think this can be shortened to:
> 
> BOUNDED_SECTION_BY(.initcall.modnames, _initcall_modnames)

Acknowledged.

> >  #define INIT_TEXT							\
> >  	*(.init.text .init.text.*)					\
> > diff --git a/include/linux/init.h b/include/linux/init.h
> > index 40331923b9f4..9c78b6c30361 100644
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -271,8 +271,30 @@ extern struct module __this_module;
> >  		__initcall_name(initcall, __iid, id),		\
> >  		__initcall_section(__sec, __iid))
> >  
> > -#define ___define_initcall(fn, id, __sec)			\
> > -	__unique_initcall(fn, id, __sec, __initcall_id(fn))
> > +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> > +#define __initcall_fn_ptr(fn, __iid, id)	__initcall_stub(fn, __iid, id)
> > +#else
> > +#define __initcall_fn_ptr(fn, __iid, id)	fn
> > +#endif
> > +
> > +struct initcall_modname {
> > +	initcall_t initcall_fn;
> > +	const char *modname;
> > +};
> > +
> > +#define ____define_initcall_modname(fn, id, __sec, __iid)		\
> > +	__unique_initcall(fn, id, __sec, __iid)				\
> > +	static const char __initstr_##fn[] __used __aligned(1)		\
> > +		__section(".init.rodata") = KBUILD_MODNAME;		\
> > +	static const struct initcall_modname __modname_##fn __used	\
> > +		__section(".initcall.modnames") = {			\
> > +			.initcall_fn = __initcall_fn_ptr(fn, __iid, id), \
> > +			.modname = __initstr_##fn			\
> > +		};
> 
> All initcalls are ultimately defined via ____define_initcall_modname(),
> including initcalls in vmlinux code that can never be built as modules.
> In such a case, KBUILD_MODNAME is set to the target's basename (same as
> KBUILD_BASENAME).
> 
> This wastes memory and also allows the module_blacklist parameter to
> match initcalls that are unrelated to built-in modules.

Agreed.

In the next iteration, I restored the default ___define_initcall definition
in include/linux/init.h back to using the standard __unique_initcall
without mapping.

To restrict the mapping strictly to actual built-in modules, I redefined
module_init() under #ifndef MODULE in include/linux/module.h to expand to
____define_initcall_modname().        
  
Indeed since core vmlinux code that can never be built as modules does not
use module_init(), the next iteration ensures that:
  
    1.  No mapping metadata is generated for core subsystem initcalls

    2.  The module_blacklist (or module_denylist) parameters can no longer
        accidentally match or skip unrelated core kernel initcalls

> > +
> > +#define ___define_initcall(fn, id, __sec)				\
> > +	____define_initcall_modname(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..bc2968c225e1 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -883,6 +883,8 @@ static inline void module_for_each_mod(int(*func)(struct module *mod, void *data
> >  }
> >  #endif /* CONFIG_MODULES */
> >  
> > +extern bool module_is_denylisted(const char *module_name);
> > +
> 
> Nit: The extern keyword in function declarations is unnecessary. See
> Documentation/process/coding-style.rst.

Acknowledged.

> 
> >  #ifdef CONFIG_SYSFS
> >  extern struct kset *module_kset;
> >  extern const struct kobj_type module_ktype;
> > diff --git a/init/main.c b/init/main.c
> > index e363232b428b..af71811d24e3 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -1334,12 +1334,65 @@ static inline void do_trace_initcall_level(const char *level)
> >  }
> >  #endif /* !TRACEPOINTS_ENABLED */
> >  
> > +extern struct initcall_modname __start_initcall_modnames[];
> > +extern struct initcall_modname __stop_initcall_modnames[];
> > +
> > +/* module_denylist is a comma-separated list of module names */
> > +static char *module_denylist;
> > +bool module_is_denylisted(const char *module_name)
> 
> module_is_denylisted() should be __init_or_module.

Acknowledged.

> > +{
> > +	const char *p;
> > +	size_t len;
> > +
> > +	if (!module_denylist)
> > +		return false;
> > +
> > +	for (p = module_denylist; *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_denylist, module_denylist, charp, 0400);
> > +core_param(module_blacklist, module_denylist, charp, 0400);
> > +
> > +static const char *initcall_get_modname(initcall_t fn)
> 
> initcall_get_modname() should ideally be __init.

Acknowledged.

> 
> Nit: I think a better name would be something like get_builtin_modname()
> to make clear that the function returns a module name only for built-in
> modules, not regular ones.

Agreed.

> > +{
> > +	struct initcall_modname *p;
> > +	unsigned long addr = (unsigned long)dereference_function_descriptor(fn);
> > +
> > +	if (system_state >= SYSTEM_FREEING_INITMEM)
> > +		return NULL;
> > +
> > +	if (!is_kernel_text(addr) &&
> > +	    !is_kernel_inittext(addr))
> > +		return NULL;
> 
> I believe these checks could be avoided if the code was reorganized to
> avoid reaching this logic on the do_init_module() path in the first
> place.

Agreed.


Kind regards,
-- 
Aaron Tomlin