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. > #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/[email protected]?part=1
