Re: [PATCH 1/2] module: pull out add_taint_module() to be public

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Potential NULL pointer dereference in `add_taint_module` if called by built-in code passing `THIS_MODULE`.
- [Low] Syntax error in the `!CONFIG_MODULES` stub for `add_taint_module` causes an unconditional build failure due to an undeclared identifier.
--

commit 371e790be5ebdf80163e6326d6a8f79c05f469bb
Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

module: pull out add_taint_module() to be public

This commit makes add_taint_module() public so other kernel code can call it.
It also provides a fallback stub that simply calls add_taint() when modules
are not enabled.

> diff --git a/include/linux/module.h b/include/linux/module.h
> index 96cc98568eea5..25221539e6459 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
[ ... ]
> @@ -877,6 +880,11 @@ static inline bool module_is_coming(struct module *mod)
>  static inline void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data)
>  {
>  }
> +
> +static inline void add_taint_module(struct module *mod, unsigned flag, enum lockdep_ok)
> +{
> +	add_taint(flag, lockdep_ok);
> +}

[Severity: Low]
Is this a build regression when CONFIG_MODULES is disabled?

The third parameter in this stub specifies the type enum lockdep_ok but omits
a variable name. The function body then passes lockdep_ok to add_taint(),
which resolves to the enum tag rather than a local variable, causing the
compiler to halt with an undeclared identifier error.

>  #endif /* CONFIG_MODULES */
>  
>  #ifdef CONFIG_SYSFS
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d0e1e0bd2ad06..99cedb9618988 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
[ ... ]
> @@ -191,8 +191,17 @@ static inline int strong_try_module_get(struct module *mod)
>  		return -ENOENT;
>  }
>  
> -static inline void add_taint_module(struct module *mod, unsigned flag,
> -				    enum lockdep_ok lockdep_ok)
> +/**
> + * add_taint_module: add a taint flag if not already set for a specific module
> + * @mod: pointer to the module that caused the problem
> + * @flag: one of the TAINT_* constants.
> + * @lockdep_ok: whether lock debugging is still OK.
> + *
> + * If something bad has gone wrong, you'll want @lockdebug_ok = false, but for
> + * some noteworthy-but-not-corrupting cases, it can be set to true.
> + */
> +void add_taint_module(struct module *mod, unsigned flag,
> +		      enum lockdep_ok lockdep_ok)
>  {
>  	add_taint(flag, lockdep_ok);
>  	set_bit(flag, &mod->taints);

[Severity: High]
Is this a regression that could unconditionally dereference a NULL pointer?

If built-in code calls this function (for example, passing THIS_MODULE which
evaluates to NULL for built-in modules), mod->taints will be accessed without
a standard if (mod) safeguard. 

This could lead to a fatal page fault if, for instance, a local root user
writes to a built-in driver's bind or unbind sysfs file which then calls
add_taint_module(drv->owner, ...).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-bind_taint-v1-0-52b05f4a965c@linuxfoundation.org?part=1