Re: [PATCH net v3] ethernet: atl1: Add missing DMA mapping error checks

[Alexander Lobakin]

From: Thomas Fourier <fourier.thomas@xxxxxxxxx>
Date: Wed, 18 Jun 2025 16:22:16 +0200

> The `dma_map_XXX()` functions can fail and must be checked using
> `dma_mapping_error()`.  This patch adds proper error handling for all
> DMA mapping calls.
> 
> In `atl1_alloc_rx_buffers()`, if DMA mapping fails, the buffer is
> deallocated and marked accordingly.
> 
> In `atl1_tx_map()`, previously mapped buffers are unmapped and the
> packet is dropped on failure.
> 
> Fixes: f3cc28c79760 ("Add Attansic L1 ethernet driver.")
> Signed-off-by: Thomas Fourier <fourier.thomas@xxxxxxxxx>
> ---
>  drivers/net/ethernet/atheros/atlx/atl1.c | 50 +++++++++++++++++++++---
>  1 file changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
> index cfdb546a09e7..9b53d87bf6ab 100644
> --- a/drivers/net/ethernet/atheros/atlx/atl1.c
> +++ b/drivers/net/ethernet/atheros/atlx/atl1.c
> @@ -1861,14 +1861,21 @@ static u16 atl1_alloc_rx_buffers(struct atl1_adapter *adapter)
>  			break;
>  		}
>  
> -		buffer_info->alloced = 1;
> -		buffer_info->skb = skb;
> -		buffer_info->length = (u16) adapter->rx_buffer_len;
>  		page = virt_to_page(skb->data);
>  		offset = offset_in_page(skb->data);
>  		buffer_info->dma = dma_map_page(&pdev->dev, page, offset,
>  						adapter->rx_buffer_len,
>  						DMA_FROM_DEVICE);
> +		if (dma_mapping_error(&pdev->dev, buffer_info->dma)) {
> +			kfree_skb(skb);
> +			adapter->soft_stats.rx_dropped++;
> +			break;
> +		}
> +
> +		buffer_info->alloced = 1;
> +		buffer_info->skb = skb;
> +		buffer_info->length = (u16)adapter->rx_buffer_len;
> +
>  		rfd_desc->buffer_addr = cpu_to_le64(buffer_info->dma);
>  		rfd_desc->buf_len = cpu_to_le16(adapter->rx_buffer_len);
>  		rfd_desc->coalese = 0;
> @@ -2183,8 +2190,8 @@ static int atl1_tx_csum(struct atl1_adapter *adapter, struct sk_buff *skb,
>  	return 0;
>  }
>  
> -static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> -	struct tx_packet_desc *ptpd)
> +static int atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> +		       struct tx_packet_desc *ptpd)

Since it only returns either 0 or -ENOMEM, it can be boolean instead
(true for success, false for mapping fail).

Thanks,
Olek