Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Calling `drain_workqueue()` on a globally shared workqueue drops non-chained works from all other devices, leading to resource leaks. Pre-existing issues: - [High] The patch fails to prevent the Use-After-Free because `drain_workqueue()` does not permanently prevent new works from being queued after unplug. -- commit a8d1c4b2d56a32fc02ddadc07ead22f7dc20b168 Author: Boris Brezillon <[email protected]> drm/panthor: Fix UAF on works queued to panthor_cleanup_wq This patch adds a drain_workqueue() call in panthor_device_unplug() to ensure cleanup works are executed before the panthor device is destroyed. Closes: https://sashiko.dev/#/patchset/[email protected]?part=2 > diff --git a/drivers/gpu/drm/panthor/panthor_device.c > b/drivers/gpu/drm/panthor/panthor_device.c > index 0b25abebb803c..a0774e28aa94d 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -101,6 +101,11 @@ void panthor_device_unplug(struct panthor_device *ptdev) > panthor_gpu_unplug(ptdev); > panthor_pwr_unplug(ptdev); > > + /* Make sure works queued to panthor_cleanup_wq are executed > + * before the device is destroyed. > + */ > + drain_workqueue(panthor_cleanup_wq); [Severity: High] Since panthor_cleanup_wq is a global workqueue shared by all panthor devices (allocated once in panthor_init()), does calling drain_workqueue() here cause issues for other active panthor devices? drain_workqueue() temporarily sets the __WQ_DRAINING flag on the workqueue. If any other active panthor device attempts to queue work while this flag is set, __queue_work() will hit a WARN_ONCE and silently discard the non-chained work. Will this break cleanup for all other panthor devices system-wide, causing resource leaks (e.g., VMs, BOs, groups) that can lead to system degradation? [Severity: High] This is a pre-existing issue, but does this actually prevent the Use-After-Free? drain_workqueue() only waits for currently queued works to finish and then clears the __WQ_DRAINING flag before returning. If a userspace process holds an open DRM file descriptor during a device hot-unplug, the ptdev isn't freed immediately. When the process later closes the FD, drm_release() will destroy the group pool and call group_release(). At this point: drivers/gpu/drm/panthor/panthor_sched.c:group_release() { ... queue_work(panthor_cleanup_wq, &group->release_work); ... } This successfully queues new asynchronous cleanup works because __WQ_DRAINING has already been cleared. When drm_release() then drops the final device reference, synchronously freeing ptdev, the newly queued group_release_work will execute asynchronously and access the freed ptdev, triggering the exact Use-After-Free this was intended to fix. > + > pm_runtime_dont_use_autosuspend(ptdev->base.dev); > pm_runtime_put_sync_suspend(ptdev->base.dev); > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
