Re: [PATCH v1 2/6] 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:
- [High] The error path in switchtec_dma_alloc_chan_resources() frees descriptor rings without halting the hardware channel or clearing DMA base addresses, leading to a potential DMA use-after-free.
--

--- Patch [2]: [PATCH v1 2/6] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources ---
commit eaf75ddb364eba320691aad49bebc8922067a94b
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 handle failures during initialization. It disables the channel and frees
the descriptor rings to prevent resource leaks when setup fails.

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

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c

[ ... ]

> @@ -988,15 +988,15 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
>  	rc = reset_channel(swdma_chan);
>  	if (rc)
> -		return rc;
> +		goto err_disable_channel;
>  
>  	rc = unhalt_channel(swdma_chan);
>  	if (rc)
> -		return rc;
> +		goto err_disable_channel;

[ ... ]

> +err_ring_inactive:
> +	spin_lock_bh(&swdma_chan->submit_lock);
> +	swdma_chan->ring_active = false;
> +	spin_unlock_bh(&swdma_chan->submit_lock);
> +
> +	spin_lock_bh(&swdma_chan->complete_lock);
> +	swdma_chan->comp_ring_active = false;
> +	spin_unlock_bh(&swdma_chan->complete_lock);
> +err_disable_channel:
> +	disable_channel(swdma_chan);
> +err_free_desc:
> +	switchtec_dma_free_desc(swdma_chan);
> +	return rc;
>  }

[Severity: High]
If unhalt_channel() times out and returns an error, we jump here and free
the descriptor rings. Does this risk a DMA use-after-free?

Looking at disable_channel(), it merely clears an enable bit via a posted,
unflushed PCIe write. Because this unwind path skips switchtec_dma_chan_stop()
(which is used in the normal teardown path to synchronously halt the channel
and clear the base registers), couldn't the hardware complete the unhalt
command and access the descriptor rings right after they are freed?

Should this error path ensure the channel is fully stopped and DMA base
addresses are cleared before freeing the coherent DMA memory?

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