Re: [PATCH v2 1/6] module/dups: Inform duplicate requests about the result directly
From: sashiko-bot@kernel.org
Date: 2026-07-23 08:41:13
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Lack of reference counting on `kmod_dup_req` allows Use-After-Free vulnerabilities when duplicate waiters or concurrent announcers race with the 60-second deletion timer. --
--- Patch [1]: [PATCH v2 1/6] module/dups: Inform duplicate requests about the result directly ---commit a68253df788049311e521e9217563df8d2aeb41e
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.
quoted hunk ↗ jump to hunk
diff --git a/kernel/module/dups.c b/kernel/module/dups.c index 1d720a5311ba0..b063bc423aa81 100644 --- a/kernel/module/dups.c +++ b/kernel/module/dups.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -230,17 +205,17 @@ void kmod_dup_request_announce(char *module_name, int ret) kmod_req->dup_ret = ret; + /* Inform all duplicate waiters to check the return value. */ + complete_all(&kmod_req->first_req_done); + /* - * If we complete() here we may allow duplicate threads - * to continue before the first one that submitted the - * request. We're in no rush also, given that each and - * every bounce back to userspace is slow we avoid that - * with a slight delay here. So queueue up the completion - * and let duplicates suffer, just wait a tad bit longer. - * There is no rush. But we also don't want to hold the - * caller up forever or introduce any boot delays. + * Now that we have allowed prior request_module() calls to go on + * with life, let's schedule deleting this entry. We don't have + * to do it right away, but we *eventually* want to do it so to not + * let this linger forever as this is just a boot optimization for + * possible abuses of vmalloc() incurred by finit_module() thrashing. */ - queue_work(system_dfl_wq, &kmod_req->complete_work); + queue_delayed_work(system_dfl_wq, &kmod_req->delete_work, 60 * HZ);
[Severity: High] This is a pre-existing issue, but could this delayed work cause a use-after-free since kmod_dup_req lacks reference counting? If a duplicate request thread waits on first_req_done in kmod_dup_request_exists_wait() without holding a reference, and is preempted for over 60 seconds after waking up, the deletion worker could call kfree() on kmod_req in kmod_dup_request_delete(). When the waiter resumes, reading kmod_req->dup_ret would result in a use-after-free. Additionally, could queue_delayed_work() here race with an actively executing teardown? If two threads concurrently call kmod_dup_request_announce() for the same module, and the second thread calls it exactly when the 60-second timer fires, it could re-arm the timer while the first worker thread clears the pending bit but blocks on kmod_dup_mutex. The second thread would release the mutex, the worker would proceed to free kmod_req, and the re-armed timer would later fire on freed memory.
out: mutex_unlock(&kmod_dup_mutex);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260723-module-stats-rcu-v2-0-bd582261fae3@suse.com?part=1