Re: [PATCH 02/10] dmaengine: ae4dma: Unregister DMA engine in ae4_pci_remove()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dmaengine: ae4dma: slab use-after-free in ae4_pci_remove()
  teardown order
--

commit d190c73fccb945a036c5abd1d18c37ce271e68d8
Author: Lee Jones <lee@kernel.org>

dmaengine: ae4dma: Unregister DMA engine in ae4_pci_remove()

This commit exports pt_dmaengine_unregister() and calls it in
ae4_pci_remove() to unregister the DMA device and destroy its descriptor
slab cache on removal.

> diff --git a/drivers/dma/amd/ae4dma/ae4dma-pci.c b/drivers/dma/amd/ae4dma/ae4dma-pci.c
> index 8ca34d4457cc0..7f48ebc5b10c6 100644
> --- a/drivers/dma/amd/ae4dma/ae4dma-pci.c
> +++ b/drivers/dma/amd/ae4dma/ae4dma-pci.c
> @@ -132,6 +132,7 @@ static void ae4_pci_remove(struct pci_dev *pdev)
>  {
>  	struct ae4_device *ae4 = dev_get_drvdata(&pdev->dev);
>  
> +	pt_dmaengine_unregister(&ae4->pt);
>  	ae4_destroy_work(ae4);
>  	ae4_deinit(ae4);
>  }

[Severity: Medium]
Can calling pt_dmaengine_unregister() before ae4_destroy_work() cause a
use-after-free on pt->dma_desc_cache?

pt_dmaengine_unregister() calls dma_async_device_unregister() and then
immediately destroys pt->dma_desc_cache, while the per-queue delayed work
items on ae4cmd_q->pws are only stopped afterwards by ae4_destroy_work().
dma_async_device_unregister() does not drain ae4cmd_q->pws, and
pt_synchronize() only synchronizes the vchan tasklet.  If ae4_pending_work()
is still running when the device is unbound, it can free a completed
descriptor into pt->dma_desc_cache after the cache is destroyed:

CPU0                                  kworker
ae4_pci_remove()                      ae4_pending_work()
  pt_dmaengine_unregister()             cmd->pt_cmd_callback()
    dma_async_device_unregister()         pt_cmd_callback_work()
    kmem_cache_destroy(                     ...
      pt->dma_desc_cache)                   vchan_vdesc_fini()
                                              pt_do_cleanup()
                                                kmem_cache_free(
                                                  pt->dma_desc_cache, desc)
                                                  <-- use-after-free
  ae4_destroy_work()

Stopping the workqueues before pt_dmaengine_unregister(), or destroying
pt->dma_desc_cache after ae4_destroy_work(), would avoid this race.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111215349.1983909-1-lee@kernel.org?part=2