On Wed, Aug 26, 2026 at 10:29:20AM +0000, [email protected] wrote:
> 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 <[email protected]>
>
> 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.
Ah, missed this, will do a v2 with this fixed, my bad.
> > #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, ...).
Oops, missed this one too, I only tested this with a module, not a
built-in driver. Will fix it up for v2.
