[PATCH module v2] module: Fix uninitialized count read of unloaded taint tracking record

[PATCH module v2] module: Fix uninitialized count read of unloaded taint tracking record

[Binbin Deng]

try_add_tainted_module() allocates the tracking record without zeroing
it and initializes the fields in this order:

	mod_taint = kmalloc_obj(*mod_taint);
	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;

The record is published by list_add_rcu() before the count field is
written.  The readers, unloaded_tainted_modules_seq_show() (debugfs,
under rcu_read_lock()) and print_unloaded_tainted_modules() (panic
path, under guard(rcu)), traverse the list under RCU and read
mod_taint->count.  module_mutex, which the writer holds, does not
exclude RCU readers.

A reader that observes the new node between the list_add_rcu() and
the count store reads an uninitialized u64 left over from previous
slab use.  This is a data race and leaks kernel heap residue through
the debugfs file and the panic message.

The race window is two adjacent instructions; it is reachable when a
tainted module is unloaded (rmmod) concurrently with a read of
/sys/kernel/debug/module/unloaded_tainted, or with the module list
printout of an oops on another CPU.

Fix this by initializing count before the record is published.

Found during a static-analysis-assisted audit: candidate sites were
surfaced by a Coccinelle rule, and the reader/writer paths were
confirmed by LLM-assisted (GLM-5.3) code inspection.  The issue was
not observed at runtime; it is identified by code inspection only.

Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
Signed-off-by: Binbin Deng <18983559317@163.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..9e8902600c32 100644
--- a/kernel/module/tracking.c
+++ b/kernel/module/tracking.c
@@ -38,8 +38,8 @@ int try_add_tainted_module(struct module *mod)
 		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;
+	list_add_rcu(&mod_taint->list, &unloaded_tainted_modules);
 out:
 	return 0;
 }
-- 
2.43.0

Re: [PATCH module v2] module: Fix uninitialized count read of unloaded taint tracking record

[Petr Pavlu]

On 10/10/26 10:03 AM, Binbin Deng wrote:
> try_add_tainted_module() allocates the tracking record without zeroing
> it and initializes the fields in this order:
> 
> 	mod_taint = kmalloc_obj(*mod_taint);
> 	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;
> 
> The record is published by list_add_rcu() before the count field is
> written.  The readers, unloaded_tainted_modules_seq_show() (debugfs,
> under rcu_read_lock()) and print_unloaded_tainted_modules() (panic
> path, under guard(rcu)), traverse the list under RCU and read
> mod_taint->count.  module_mutex, which the writer holds, does not
> exclude RCU readers.
> 
> A reader that observes the new node between the list_add_rcu() and
> the count store reads an uninitialized u64 left over from previous
> slab use.  This is a data race and leaks kernel heap residue through
> the debugfs file and the panic message.
> 
> The race window is two adjacent instructions; it is reachable when a
> tainted module is unloaded (rmmod) concurrently with a read of
> /sys/kernel/debug/module/unloaded_tainted, or with the module list
> printout of an oops on another CPU.
> 
> Fix this by initializing count before the record is published.
> 
> Found during a static-analysis-assisted audit: candidate sites were
> surfaced by a Coccinelle rule, and the reader/writer paths were
> confirmed by LLM-assisted (GLM-5.3) code inspection.  The issue was
> not observed at runtime; it is identified by code inspection only.
> 
> Fixes: 99bd9956551b ("module: Introduce module unload taint tracking")
> Signed-off-by: Binbin Deng <18983559317@163.com>

Looks ok, I've queued the patch on modules-next for v7.4-rc1.

-- 
Thanks,
Petr