* [PATCH 1/3] module: Fix missing taint flags for unloaded modules
2026-10-20 14:05 [PATCH 0/3] module: Fixes to tracking taint flags for unloaded modules Petr Pavlu
@ 2026-10-20 14:05 ` Petr Pavlu
2026-10-20 15:10 ` Aaron Tomlin
2026-10-20 14:05 ` [PATCH 2/3] module: Fix data race on mod_unload_taint::count Petr Pavlu
2026-10-20 14:05 ` [PATCH 3/3] module: Move mod_unload_taint to tracking.c Petr Pavlu
2 siblings, 1 reply; 7+ messages in thread
From: Petr Pavlu @ 2026-10-20 14:05 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: Aaron Tomlin, linux-modules, linux-kernel
When recording taint flags for an unloaded module, try_add_tainted_module()
can fail to record some of the module's flags if a record for a module with
the same name already exists and the two have at least one flag in common.
This happens because the function uses 'mod_taint->taints & mod->taints' to
determine whether to merge the new module with the existing record. As
a result, the final report may not provide accurate information about the
taint flags of unloaded modules.
Fix the duplicate check by using the stricter equality comparison.
Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/tracking.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
index 9033ff54c4e2..5bcd1c2701f0 100644
--- a/kernel/module/tracking.c
+++ b/kernel/module/tracking.c
@@ -27,7 +27,7 @@ int try_add_tainted_module(struct module *mod)
list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules, list,
lockdep_is_held(&module_mutex)) {
if (!strcmp(mod_taint->name, mod->name) &&
- mod_taint->taints & mod->taints) {
+ mod_taint->taints == mod->taints) {
mod_taint->count++;
goto out;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] module: Fix missing taint flags for unloaded modules
2026-10-20 14:05 ` [PATCH 1/3] module: Fix missing " Petr Pavlu
@ 2026-10-20 15:10 ` Aaron Tomlin
0 siblings, 0 replies; 7+ messages in thread
From: Aaron Tomlin @ 2026-10-20 15:10 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
On Tue, Oct 20, 2026 at 04:05:19PM +0200, Petr Pavlu wrote:
> When recording taint flags for an unloaded module, try_add_tainted_module()
> can fail to record some of the module's flags if a record for a module with
> the same name already exists and the two have at least one flag in common.
> This happens because the function uses 'mod_taint->taints & mod->taints' to
> determine whether to merge the new module with the existing record. As
> a result, the final report may not provide accurate information about the
> taint flags of unloaded modules.
>
> Fix the duplicate check by using the stricter equality comparison.
>
> Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> kernel/module/tracking.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
> index 9033ff54c4e2..5bcd1c2701f0 100644
> --- a/kernel/module/tracking.c
> +++ b/kernel/module/tracking.c
> @@ -27,7 +27,7 @@ int try_add_tainted_module(struct module *mod)
> list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules, list,
> lockdep_is_held(&module_mutex)) {
> if (!strcmp(mod_taint->name, mod->name) &&
> - mod_taint->taints & mod->taints) {
> + mod_taint->taints == mod->taints) {
> mod_taint->count++;
> goto out;
> }
>
> --
> 2.55.0
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] module: Fix data race on mod_unload_taint::count
2026-10-20 14:05 [PATCH 0/3] module: Fixes to tracking taint flags for unloaded modules Petr Pavlu
2026-10-20 14:05 ` [PATCH 1/3] module: Fix missing " Petr Pavlu
@ 2026-10-20 14:05 ` Petr Pavlu
2026-10-20 14:13 ` sashiko-bot
2026-10-20 14:05 ` [PATCH 3/3] module: Move mod_unload_taint to tracking.c Petr Pavlu
2 siblings, 1 reply; 7+ messages in thread
From: Petr Pavlu @ 2026-10-20 14:05 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: Aaron Tomlin, linux-modules, linux-kernel
Updates to mod_unload_taint::count are serialized by module_mutex. However,
readers may access this value concurrently with these updates. Since the
counter is a u64, this can result in torn reads on 32-bit architectures.
Fix the issue by changing mod_unload_taint::count to 'unsigned int' and
using WRITE_ONCE()/READ_ONCE() to access it.
Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/internal.h | 8 +++++++-
kernel/module/tracking.c | 12 ++++++++----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 061161cc79d9..70ef20b97c84 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -257,7 +257,13 @@ struct mod_unload_taint {
struct list_head list;
char name[MODULE_NAME_LEN];
unsigned long taints;
- u64 count;
+
+ /*
+ * Number of times a module with this name and taint combination has
+ * been unloaded. Updates are serialized by module_mutex and use
+ * WRITE_ONCE(). Readers that may run concurrently use READ_ONCE().
+ */
+ unsigned int count;
};
int try_add_tainted_module(struct module *mod);
diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
index 5bcd1c2701f0..873fceb30526 100644
--- a/kernel/module/tracking.c
+++ b/kernel/module/tracking.c
@@ -28,7 +28,10 @@ int try_add_tainted_module(struct module *mod)
lockdep_is_held(&module_mutex)) {
if (!strcmp(mod_taint->name, mod->name) &&
mod_taint->taints == mod->taints) {
- mod_taint->count++;
+ unsigned int count = mod_taint->count;
+
+ if (count != UINT_MAX)
+ WRITE_ONCE(mod_taint->count, count + 1);
goto out;
}
}
@@ -57,8 +60,8 @@ void print_unloaded_tainted_modules(void)
l = module_flags_taint(mod_taint->taints, buf);
buf[l++] = '\0';
- pr_cont(" %s(%s):%llu", mod_taint->name, buf,
- mod_taint->count);
+ pr_cont(" %s(%s):%u", mod_taint->name, buf,
+ READ_ONCE(mod_taint->count));
}
}
}
@@ -92,7 +95,8 @@ static int unloaded_tainted_modules_seq_show(struct seq_file *m, void *p)
l = module_flags_taint(mod_taint->taints, buf);
buf[l++] = '\0';
- seq_printf(m, "%s (%s) %llu", mod_taint->name, buf, mod_taint->count);
+ seq_printf(m, "%s (%s) %u", mod_taint->name, buf,
+ READ_ONCE(mod_taint->count));
seq_puts(m, "\n");
return 0;
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] module: Fix data race on mod_unload_taint::count
2026-10-20 14:05 ` [PATCH 2/3] module: Fix data race on mod_unload_taint::count Petr Pavlu
@ 2026-10-20 14:13 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-20 14:13 UTC (permalink / raw)
To: Petr Pavlu; +Cc: linux-modules
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] module: data race and uninitialized read of count in try_add_tainted_module()
--
commit 742b3a34fee074246edb757213ccfc148f6e7300
Author: Petr Pavlu <petr.pavlu@suse.com>
module: Fix data race on mod_unload_taint::count
This commit fixes a data race on mod_unload_taint::count between updates
serialized by module_mutex and concurrent RCU readers by changing the
field to unsigned int and using WRITE_ONCE()/READ_ONCE() accesses.
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 061161cc79d90..70ef20b97c84a 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -257,7 +257,13 @@ struct mod_unload_taint {
> struct list_head list;
> char name[MODULE_NAME_LEN];
> unsigned long taints;
> - u64 count;
> +
> + /*
> + * Number of times a module with this name and taint combination has
> + * been unloaded. Updates are serialized by module_mutex and use
> + * WRITE_ONCE(). Readers that may run concurrently use READ_ONCE().
> + */
> + unsigned int count;
> };
>
> int try_add_tainted_module(struct module *mod);
> diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
> index 5bcd1c2701f07..873fceb305268 100644
> --- a/kernel/module/tracking.c
> +++ b/kernel/module/tracking.c
> @@ -28,7 +28,10 @@ int try_add_tainted_module(struct module *mod)
> lockdep_is_held(&module_mutex)) {
> if (!strcmp(mod_taint->name, mod->name) &&
> mod_taint->taints == mod->taints) {
> - mod_taint->count++;
> + unsigned int count = mod_taint->count;
> +
> + if (count != UINT_MAX)
> + WRITE_ONCE(mod_taint->count, count + 1);
> goto out;
> }
> }
[Severity: Medium]
Does try_add_tainted_module() in kernel/module/tracking.c still have a
data race and potential uninitialized read on mod_taint->count when
allocating a new entry?
Further down in try_add_tainted_module(), a newly allocated entry is
published via list_add_rcu() before mod_taint->count is initialized:
mod_taint = kmalloc_obj(*mod_taint);
if (unlikely(!mod_taint))
return -ENOMEM;
strscpy(mod_taint->name, mod->name, MODULE_NAME_LEN);
mod_taint->taints = mod->taints;
list_add_rcu(&mod_taint->list, &unloaded_tainted_modules);
mod_taint->count = 1;
Because kmalloc_obj() does not zero-initialize the allocation and
list_add_rcu() publishes the entry to unloaded_tainted_modules first,
concurrent RCU readers in print_unloaded_tainted_modules() or
unloaded_tainted_modules_seq_show() can observe mod_taint and execute
READ_ONCE(mod_taint->count) while mod_taint->count still contains
uninitialized slab memory, or race with the plain store setting it to 1.
Should mod_taint->count = 1 be initialized before calling list_add_rcu()?
[ ... ]
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261020-taint-unload-v1-0-d1c764889afc@suse.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] module: Move mod_unload_taint to tracking.c
2026-10-20 14:05 [PATCH 0/3] module: Fixes to tracking taint flags for unloaded modules Petr Pavlu
2026-10-20 14:05 ` [PATCH 1/3] module: Fix missing " Petr Pavlu
2026-10-20 14:05 ` [PATCH 2/3] module: Fix data race on mod_unload_taint::count Petr Pavlu
@ 2026-10-20 14:05 ` Petr Pavlu
2026-10-20 15:09 ` Aaron Tomlin
2 siblings, 1 reply; 7+ messages in thread
From: Petr Pavlu @ 2026-10-20 14:05 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: Aaron Tomlin, linux-modules, linux-kernel
Move the definition of mod_unload_taint from internal.h to tracking.c, as
it doesn't need to be exposed to other module code, and remove the
redundant declaration of mod_debugfs_root from tracking.c, as it is already
declared in internal.h.
This is a non-functional change.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/internal.h | 13 -------------
kernel/module/tracking.c | 14 +++++++++++++-
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 70ef20b97c84..3aca22253296 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -253,19 +253,6 @@ static inline void kmod_dup_request_announce(char *module_name, int ret)
#endif
#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
-struct mod_unload_taint {
- struct list_head list;
- char name[MODULE_NAME_LEN];
- unsigned long taints;
-
- /*
- * Number of times a module with this name and taint combination has
- * been unloaded. Updates are serialized by module_mutex and use
- * WRITE_ONCE(). Readers that may run concurrently use READ_ONCE().
- */
- unsigned int count;
-};
-
int try_add_tainted_module(struct module *mod);
void print_unloaded_tainted_modules(void);
#else /* !CONFIG_MODULE_UNLOAD_TAINT_TRACKING */
diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
index 873fceb30526..46840dc22dcb 100644
--- a/kernel/module/tracking.c
+++ b/kernel/module/tracking.c
@@ -14,8 +14,20 @@
#include <linux/rculist.h>
#include "internal.h"
+struct mod_unload_taint {
+ struct list_head list;
+ char name[MODULE_NAME_LEN];
+ unsigned long taints;
+
+ /*
+ * Number of times a module with this name and taint combination has
+ * been unloaded. Updates are serialized by module_mutex and use
+ * WRITE_ONCE(). Readers that may run concurrently use READ_ONCE().
+ */
+ unsigned int count;
+};
+
static LIST_HEAD(unloaded_tainted_modules);
-extern struct dentry *mod_debugfs_root;
int try_add_tainted_module(struct module *mod)
{
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] module: Move mod_unload_taint to tracking.c
2026-10-20 14:05 ` [PATCH 3/3] module: Move mod_unload_taint to tracking.c Petr Pavlu
@ 2026-10-20 15:09 ` Aaron Tomlin
0 siblings, 0 replies; 7+ messages in thread
From: Aaron Tomlin @ 2026-10-20 15:09 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
On Tue, Oct 20, 2026 at 04:05:21PM +0200, Petr Pavlu wrote:
> Move the definition of mod_unload_taint from internal.h to tracking.c, as
> it doesn't need to be exposed to other module code, and remove the
> redundant declaration of mod_debugfs_root from tracking.c, as it is already
> declared in internal.h.
>
> This is a non-functional change.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> kernel/module/internal.h | 13 -------------
> kernel/module/tracking.c | 14 +++++++++++++-
> 2 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 70ef20b97c84..3aca22253296 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -253,19 +253,6 @@ static inline void kmod_dup_request_announce(char *module_name, int ret)
> #endif
>
> #ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING
> -struct mod_unload_taint {
> - struct list_head list;
> - char name[MODULE_NAME_LEN];
> - unsigned long taints;
> -
> - /*
> - * Number of times a module with this name and taint combination has
> - * been unloaded. Updates are serialized by module_mutex and use
> - * WRITE_ONCE(). Readers that may run concurrently use READ_ONCE().
> - */
> - unsigned int count;
> -};
> -
> int try_add_tainted_module(struct module *mod);
> void print_unloaded_tainted_modules(void);
> #else /* !CONFIG_MODULE_UNLOAD_TAINT_TRACKING */
> diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
> index 873fceb30526..46840dc22dcb 100644
> --- a/kernel/module/tracking.c
> +++ b/kernel/module/tracking.c
> @@ -14,8 +14,20 @@
> #include <linux/rculist.h>
> #include "internal.h"
>
> +struct mod_unload_taint {
> + struct list_head list;
> + char name[MODULE_NAME_LEN];
> + unsigned long taints;
> +
> + /*
> + * Number of times a module with this name and taint combination has
> + * been unloaded. Updates are serialized by module_mutex and use
> + * WRITE_ONCE(). Readers that may run concurrently use READ_ONCE().
> + */
> + unsigned int count;
> +};
> +
> static LIST_HEAD(unloaded_tainted_modules);
> -extern struct dentry *mod_debugfs_root;
>
> int try_add_tainted_module(struct module *mod)
> {
>
> --
> 2.55.0
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 7+ messages in thread