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

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

Pre-existing issues:
- [High] Improper teardown order in `switchtec_dma_remove` enables race conditions and use-after-free with dmaengine clients.
- [High] The driver holds a spinlock across dmaengine API boundaries, causing guaranteed deadlocks if multiple descriptors are batched.
--

--- Patch [2]: [PATCH v4 02/12] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources ---
commit c17dbf5966e1848ec7a26210950a24038c80d7e5
Author: Logan Gunthorpe <[email protected]>

dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources

This commit fixes a resource leak in switchtec_dma_alloc_chan_resources()
where descriptor rings and coherent memory weren't freed on failure. It adds
an error-unwind path to properly disable the channel and clean up active
flags.

Link: https://lore.kernel.org/dmaengine/[email protected]

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index a4a7d66d042de..f77da31aeb65e 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but does this teardown order
allow active dmaengine clients to access destroyed hardware state or wait
on freed IRQs?

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 switchtec_dma_remove() disables channels and frees IRQs before
calling dma_async_device_unregister(), a concurrent client could attempt
to allocate or submit jobs to the exposed device before unregistration
completes.

[Severity: High]
This is also a pre-existing issue, but does returning to a generic API
client with a spinlock held cause deadlocks?

drivers/dma/switchtec_dma.c:switchtec_dma_prep_desc() {
...
	/* return with the lock held, it will be released in tx_submit */

	return &desc->txd;
}

If a client batches multiple descriptors (e.g., calling
dmaengine_prep_dma_memcpy() twice for the same channel before submit),
wouldn't the second call attempt to recursively acquire the same
non-recursive submit_lock?

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