Re: [PATCH 04/12] drm/panthor: Flush the cleanup_wq before destroying the drm_device

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-After-Free during device teardown due to incorrect `drmm` action registration order for `panthor_cleanup_wq`.
--

--- Patch [4]: [PATCH 04/12] drm/panthor: Flush the cleanup_wq before destroying the drm_device ---
commit 87cbd87d54cc32db384d8c3b01846063a45650f2
Author: Boris Brezillon <[email protected]>

drm/panthor: Flush the cleanup_wq before destroying the drm_device

This commit adds a drmm action to flush the panthor_cleanup_wq workqueue
during device teardown. It registers this action before initializing
subcomponents, intending to ensure that any cleanup work queued during
initialization or execution is flushed.

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 7d336f160d1fd..b7c55a6f4f081 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
[ ... ]
> @@ -220,6 +228,10 @@ int panthor_device_init(struct panthor_device *ptdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = drmm_add_action(&ptdev->base, panthor_device_flush_cleanup_wq, NULL);
> +	if (ret)
> +		return ret;
> +

[Severity: High]
Does this execute the flush in the intended order?

Since drmm release actions execute in Last-In-First-Out (LIFO) order,
registering the flush action before subcomponent initializations (like
panthor_mmu_init() and panthor_fw_init()) means the flush will run after
those subcomponents have already been freed.

If there is pending work on panthor_cleanup_wq, could this lead to a
use-after-free? When panthor_device_flush_cleanup_wq() executes, pending jobs
could attempt to access the already-freed subcomponents. For example, tearing
down a BO accesses freed memory here:

panthor_device_flush_cleanup_wq()
  flush_workqueue()
    ...
    panthor_kernel_bo_destroy()
      if (vm == panthor_fw_vm(ptdev))  <-- ptdev->fw is already freed

>  	ret = panthor_clk_init(ptdev);
>  	if (ret)
>  		return ret;

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