Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] A pre-existing use-after-free exists in `kmod_dup_request_complete()` due to `kmod_dup_request_announce()` queuing `complete_work` without holding a reference. -- --- Patch [1]: [PATCH 1/5] module/dups: Fix use-after-free in kmod_dup_req lifetime handling --- commit c16274209ebc300f334c8ae6c8a5bec3494aa562 Author: Petr Pavlu <[email protected]> module/dups: Fix use-after-free in kmod_dup_req lifetime handling The kmod dups code uses RCU to ensure that a kmod_dup_req instance is freed only after it is no longer referenced. When releasing an instance, the kmod_dup_request_delete() function removes the kmod_dup_req from the dup_kmod_reqs list, waits via synchronize_rcu() and finally frees it. However, this doesn't work correctly because parallel users referencing the instance in kmod_dup_request_exists_wait() don't enter an RCU read-side critical section. This can result in a use-after-free. Fix the issue by changing the lifecycle management of kmod_dup_req to use reference counting. > diff --git a/kernel/module/dups.c b/kernel/module/dups.c > index 1d720a5311ba0..e1828e865edde 100644 > --- a/kernel/module/dups.c > +++ b/kernel/module/dups.c [ ... ] > @@ -224,6 +241,10 @@ void kmod_dup_request_announce(char *module_name, int > ret) > > mutex_lock(&kmod_dup_mutex); > > + /* > + * Find the entry previously added in kmod_dup_request_exists_wait() > + * that is owned by the current task. > + */ > kmod_req = kmod_dup_request_lookup(module_name); > if (!kmod_req) > goto out; [Severity: Critical] This is a pre-existing issue, but is there still a use-after-free risk for complete_work even with the refcounting switch? Looking further down in kmod_dup_request_announce(), it queues the asynchronous work item without incrementing the object's reference count: queue_work(system_dfl_wq, &kmod_req->complete_work); Because no reference is held for the work item, kmod_req could be freed by delete_work before kmod_dup_request_complete() finishes executing. When the work executes in kmod_dup_request_complete() and dereferences kmod_req to call complete_all(&kmod_req->first_req_done), this could access freed memory. Can concurrent auto-loads trigger this? For instance, if multiple tasks execute __request_module() concurrently: Task 1 (wait=false) Task 2 (wait=true) kmod_dup_request_announce() queue_work(complete_work) kmod_dup_request_announce() queue_work(complete_work) Could the second complete_work execution be queued just before the 60-second timer expires, and run right after delete_work frees the object? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
