Re: [PATCH v6 02/10] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Race condition in switchtec_dma_remove allows concurrent channel allocations on a broken channel.
--

commit f369eaa9794c069cc909c5cee24ba55c0cfa8c48
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources

This commit adds an error-unwind path to switchtec_dma_alloc_chan_resources()
to prevent resource leaks when channel initialization fails. It ensures
descriptor rings are freed and the channel is disabled and marked inactive
on error.

Link: https://lore.kernel.org/dmaengine/20260707165555.350951F000E9@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index a10818efba4e5..e55fe9ff7e2c5 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c

[ ... ]

> @@ -1007,7 +1007,8 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
>  	rcu_read_lock();
>  	if (!rcu_dereference(swdma_dev->pdev)) {

[Severity: High]
This isn't a bug introduced by this patch, but is there a pre-existing race
condition here between channel allocation and device removal?

During device unbind or hot-unplug, switchtec_dma_remove() calls
switchtec_dma_chans_release(), which frees the channel IRQs and kills
tasklets, then clears swdma_dev->pdev:

drivers/dma/switchtec_dma.c:switchtec_dma_remove() {
    ...
    switchtec_dma_chans_release(pdev, swdma_dev);
    rcu_assign_pointer(swdma_dev->pdev, NULL);
    synchronize_rcu();
    pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
    pci_free_irq_vectors(pdev);
    dma_async_device_unregister(&swdma_dev->dma_dev);
}

Because dma_async_device_unregister() happens much later, a concurrent
client calling dma_request_chan() can still invoke
switchtec_dma_alloc_chan_resources(). If this executes before
swdma_dev->pdev is set to NULL, it successfully allocates the channel and
sets ring_active = true, unaware that the IRQ and tasklet were already
destroyed.

Could this cause the client to hang waiting for a freed IRQ, or trigger a
system crash due to unmapped memory access when switchtec_dma_remove()
subsequently unmaps the hardware registers?

>  		rcu_read_unlock();
> -		return -ENODEV;
> +		rc = -ENODEV;
> +		goto err_ring_inactive;
>  	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902062153.31048-1-logang@deltatee.com?part=2