Re: [PATCH v3 3/6] module/dups: Avoid unnecessary kmod_dup_req allocations
From: sashiko-bot@kernel.org
Date: 2026-08-06 09:42:02
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] A `request_module_nowait()` call can prematurely complete a concurrent `request_module(wait=true)` request, causing synchronous waiters to proceed before the module is actually loaded. --
--- Patch [3]: [PATCH v3 3/6] module/dups: Avoid unnecessary kmod_dup_req allocations ---commit c39253dbe61d24363b967a87af56bd7ef5c557ff Author: Petr Pavlu [off-list ref] module/dups: Avoid unnecessary kmod_dup_req allocations This commit defers the allocation of kmod_dup_req until it is actually needed, instead of preallocating it before taking the mutex. It introduces a helper alloc_kmod_req() to simplify the initialization process.
quoted hunk ↗ jump to hunk
diff --git a/kernel/module/dups.c b/kernel/module/dups.c --- a/kernel/module/dups.c +++ b/kernel/module/dups.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -138,7 +141,6 @@ bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret) * If a prior request to this one came through with * request_module() though, then a request_module_nowait() * would benefit from duplicate detection. */ if (!wait) { - kfree(new_kmod_req); pr_debug("New request_module_nowait() for %s -- cannot track duplicates for this request\n", module_name); mutex_unlock(&kmod_dup_mutex); return false;
[Severity: High]
This is a pre-existing issue, but does a request_module_nowait() call risk
prematurely completing a concurrent request_module() call that is waiting?
If Thread B calls request_module_nowait("foo"), it doesn't create a
tracking entry because wait is false. If Thread A subsequently calls
request_module("foo") with wait=true, it creates the tracking entry and
blocks for processing. If Thread C then calls request_module("foo") with
wait=true, it finds Thread A's entry and blocks waiting for it.
Because Thread B used nowait, its execution finishes quickly (waiting only
for the modprobe binary to exec) and returns 0. It then unconditionally
announces completion:
kernel/module/dups.c:kmod_dup_request_announce() {
...
kmod_req = kmod_dup_request_lookup(module_name);
if (!kmod_req || completion_done(&kmod_req->first_req_done)) {
mutex_unlock(&kmod_dup_mutex);
return;
}
kmod_req->dup_ret = ret;
/* Inform all duplicate waiters to check the return value. */
complete_all(&kmod_req->first_req_done);
...
}
Won't this find Thread A's tracking entry, assign dup_ret = 0, and wake up
Thread C prematurely while the module is still being loaded in the
background by Thread A?
If Thread C receives a success return value before the module is actually
fully loaded and initialized, could this cause drivers or subsystems to
attempt to use uninitialized module symbols or hardware features?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-module-stats-rcu-v3-0-2b0331106f00@suse.com?part=3