Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Potential Use-After-Free of `sched` and `sched->sched_rq` if `drm_sched_fini()` is called with pending jobs and `own_submit_wq` is false. -- commit f30738456b70961d8d90c374f4f4803079a2fecd Author: Donggeun Yoo <[email protected]> drm/sched: Free the run queues at the end of drm_sched_fini() This commit moves the freeing of the scheduler run queues from the top of drm_sched_fini() to the bottom. This prevents a use-after-free where the timeout handler might still be walking the run queues before cancel_delayed_work_sync() completes. Link: https://lore.kernel.org/dri-devel/[email protected]/ > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 6cb6f95464938..fec04c944c5e7 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c [ ... ] > @@ -1226,6 +1223,9 @@ void drm_sched_fini(struct drm_gpu_scheduler *sched) > if (sched->own_submit_wq) > destroy_workqueue(sched->submit_wq); [Severity: Critical] This is a pre-existing issue, but does skipping the workqueue drain when own_submit_wq is false leave us exposed to a use-after-free here? If a job timeout occurs concurrently with context teardown, drm_sched_fini() calls cancel_delayed_work_sync(&sched->work_tdr), which waits for the timeout handler to complete. Drivers like Panthor and PVR implement the timeout handler by resetting the hardware and calling drm_sched_start(), which calls drm_sched_wqueue_start(). This unconditionally queues the work item back onto the shared workqueue: drivers/gpu/drm/scheduler/sched_main.c:drm_sched_wqueue_start() { ... queue_work(sched->submit_wq, &sched->work_run_job); ... } After the timeout handler returns, cancel_delayed_work_sync() finishes. Because these drivers use a shared workqueue, own_submit_wq is false, so the workqueue is neither drained nor destroyed. When drm_sched_fini() then frees sched->sched_rq and returns, the driver can subsequently free the memory containing the scheduler. Can the shared submit_wq then execute the re-queued work_run_job, accessing the freed sched and sched->sched_rq? > sched->ready = false; > + > + for (i = DRM_SCHED_PRIORITY_KERNEL; i < sched->num_rqs; i++) > + kfree(sched->sched_rq[i]); > kfree(sched->sched_rq); > sched->sched_rq = NULL; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
