Re: [PATCH v3 05/17] drm/panthor: Flush the cleanup_wq in the unplug path

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Flushing a global, module-wide workqueue inside a device-specific unplug path stalls the unplug process for all active devices.

Pre-existing issues:
- [High] The patch adds `flush_workqueue(panthor_cleanup_wq)` to prevent Use-After-Free (UAF), but it does not prevent new work items from being queued after the flush completes, leaving a race window that still triggers UAFs.
--

--- Patch [5]: [PATCH v3 05/17] drm/panthor: Flush the cleanup_wq in the unplug path ---
commit ab033ca0df2b4b2d5ba70d4e669044aa6bb08bda
Author: Boris Brezillon <[email protected]>

drm/panthor: Flush the cleanup_wq in the unplug path

If we don't do that, we might face various UAFs, because the resource
referenced by these work items might be gone by the time they get
executed.

Closes: https://sashiko.dev/#/patchset/[email protected]?part=2

> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 0182b72f19320..0b862d3c36058 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -3365,6 +3365,12 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
>  		}
>  	}
>  	mutex_unlock(&ptdev->mmu->as.slots_lock);
> +
> +	/* Ensure any pending job cleanup work are executed before returning,
> +	 * otherwise those might access objects that are gone if the work is
> +	 * executed after other components are unplugged.
> +	 */
> +	flush_workqueue(panthor_cleanup_wq);
>  }

[Severity: Medium]
Since panthor_cleanup_wq is allocated globally in panthor_drv_init() as a
module-wide workqueue, will flushing it here stall the teardown for all
active devices?

If the system has multiple Panthor GPUs and another device has a backlog of
jobs or a stuck work item, it seems this could block the current device's
unplug thread until all unrelated work items complete.

> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5832dccfc0932..f18b2e03f2fd8 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
[ ... ]
> @@ -4074,6 +4074,12 @@ void panthor_sched_unplug(struct panthor_device *ptdev)
>  		sched->pm.has_ref = false;
>  	}
>  	mutex_unlock(&sched->lock);
> +
> +	/* Ensure any pending group release work are executed before returning,
> +	 * otherwise those might access objects that are gone if the work is
> +	 * executed after other components are unplugged.
> +	 */
> +	flush_workqueue(panthor_cleanup_wq);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does this flush actually
prevent the UAF if userspace still holds an open DRM file descriptor?

When userspace closes the file descriptor after the unplug completes, it
triggers the teardown path:
drm_file_free() -> panthor_postclose() -> panthor_group_pool_destroy()

This drops group references, calling group_release(), which will call:
queue_work(panthor_cleanup_wq, &group->release_work);

Once drm_file_free() completes, the last reference drops and drm_dev_release()
synchronously frees the panthor_device memory. Then group->release_work executes
asynchronously on the workqueue, accessing the freed group->ptdev and its
resources.

A similar race seems to exist in panthor_vm_bind_job_release() in
panthor_mmu.c, where dropped VM bind jobs queue cleanup_op_ctx_work after
the MMU flush. Can an unprivileged user trigger a GPU reset, wait for the
unplug, and then close the file descriptor to reliably hit this UAF?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5