Re: [PATCH 2/5] module/dups: Inform duplicate requests about the result directly
From: sashiko-bot@kernel.org
Date: 2026-07-20 12:28:13
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] Dropping `kmod_dup_mutex` prematurely without acquiring a reference leads to a Use-After-Free and data races. Pre-existing issues: - [High] A `request_module_nowait` call can erroneously complete a concurrent `request_module` duplicate tracking object prematurely. --
--- Patch [2]: [PATCH 2/5] module/dups: Inform duplicate requests about the result directly ---commit 5b4a732b8aa814f2e4a78fd3e59bb299a50f8a41 Author: Petr Pavlu [off-list ref] module/dups: Inform duplicate requests about the result directly When kmod_dup_request_announce() announces the completion of a request_module() call to duplicate waiters, it queues a work item to invoke kmod_dup_request_complete(), and only that function calls complete_all(). This adds an arbitrary delay that is unnecessary and provides little benefit. Call complete_all() directly from kmod_dup_request_announce() instead. Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
quoted hunk ↗ jump to hunk
diff --git a/kernel/module/dups.c b/kernel/module/dups.c index e1828e865edde..f0bdd7560d7e7 100644 --- a/kernel/module/dups.c +++ b/kernel/module/dups.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -246,23 +221,22 @@ void kmod_dup_request_announce(char *module_name, int ret) * that is owned by the current task. */ kmod_req = kmod_dup_request_lookup(module_name); + mutex_unlock(&kmod_dup_mutex); + if (!kmod_req) - goto out; + return; kmod_req->dup_ret = ret;
[Severity: High] Does unlocking kmod_dup_mutex here leave kmod_req unprotected? If a thread is preempted after unlocking the mutex but before writing to kmod_req->dup_ret, could delete_work execute, drop the refcount to zero, and free kmod_req? This would result in a use-after-free when the preempted thread resumes and attempts to write to kmod_req->dup_ret or call complete_all(). Should we hold the mutex longer or acquire a reference?
+ /* Inform all the duplicate waiters to check the return value. */ + complete_all(&kmod_req->first_req_done);
[Severity: High] This is a pre-existing issue, but can this prematurely complete a concurrent request_module() call? When request_module_nowait() is called, it skips adding a tracking object to the dup_kmod_reqs list, but continues to execute call_modprobe() and unconditionally calls kmod_dup_request_announce(). If a concurrent request_module() call had created a tracking object for the same module, the nowait thread's lookup will find it and call complete_all(), prematurely waking up all duplicate waiters before the original modprobe has actually finished. Does this function need to verify the caller owns the request before completing it? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260720121621.750661-1-petr.pavlu@suse.com?part=2