Re: [PATCH 1/2] spi: atmel: fix resource leak on DMA buffer allocation failure

[Claudiu Beznea]

Hi, Felix,

On 5/15/26 20:20, Felix Gu wrote:
> The original code set use_dma to false when dma_alloc_coherent() fails,
> so DMA channels allocated earlier were never freed, causing a resource
> leak.
>
> Fix by moving the bounce buffer allocation into
> atmel_spi_configure_dma() and extending atmel_spi_release_dma() to
> also free the bounce buffers. Any allocation failure in the DMA
> configuration path now rolls back both channels and buffers through
> the same release function.
>
> Fixes: a9889ed62d06 ("spi: atmel: Implements transfers with bounce buffer")
> Signed-off-by: Felix Gu<ustc.gu@xxxxxxxxx>
> ---
>   drivers/spi/spi-atmel.c | 113 ++++++++++++++++++++++++------------------------
>   1 file changed, 57 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 25aa294631c8..e519a86a2b45 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -559,6 +559,34 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as, u8 bits_per_word)
>   	return err;
>   }
>   
> +static void atmel_spi_release_dma(struct spi_controller *host,
> +				  struct atmel_spi *as)
> +{
> +	if (host->dma_rx) {
> +		dma_release_channel(host->dma_rx);
> +		host->dma_rx = NULL;
> +	}
> +	if (host->dma_tx) {
> +		dma_release_channel(host->dma_tx);
> +		host->dma_tx = NULL;
> +	}
> +
> +	if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
> +		if (as->addr_tx_bbuf) {
> +			dma_free_coherent(&as->pdev->dev, SPI_MAX_DMA_XFER,
> +					  as->addr_tx_bbuf,
> +					  as->dma_addr_tx_bbuf);
> +			as->addr_tx_bbuf = NULL;
> +		}
> +		if (as->addr_rx_bbuf) {
> +			dma_free_coherent(&as->pdev->dev, SPI_MAX_DMA_XFER,
> +					  as->addr_rx_bbuf,
> +					  as->dma_addr_rx_bbuf);
> +			as->addr_rx_bbuf = NULL;
> +		}
> +	}
> +}
> +
>   static int atmel_spi_configure_dma(struct spi_controller *host,
>   				   struct atmel_spi *as)
>   {
> @@ -569,7 +597,8 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
>   	if (IS_ERR(host->dma_tx)) {
>   		err = PTR_ERR(host->dma_tx);
>   		dev_dbg(dev, "No TX DMA channel, DMA is disabled\n");
> -		goto error_clear;
> +		host->dma_tx = NULL;
> +		return err;
>   	}
>   
>   	host->dma_rx = dma_request_chan(dev, "rx");
> @@ -580,12 +609,31 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
>   		 * requested tx channel.
>   		 */
>   		dev_dbg(dev, "No RX DMA channel, DMA is disabled\n");
> -		goto error;
> +		host->dma_rx = NULL;
> +		goto err_release_dma;
>   	}
>   
>   	err = atmel_spi_dma_slave_config(as, 8);
>   	if (err)
> -		goto error;
> +		goto err_release_dma;
> +
> +	if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
> +		as->addr_tx_bbuf = dma_alloc_coherent(dev, SPI_MAX_DMA_XFER,
> +						      &as->dma_addr_tx_bbuf,
> +						      GFP_KERNEL | GFP_DMA);
You could use dmam_alloc_coherent() and avoid bulking the failure path.

Thank you,
Claudiu