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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <80e0d302-7c43-4435-9d74-f1950878216d@intel.com>
Date: Fri, 20 Jun 2025 16:26:28 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Thomas Fourier <fourier.thomas@...il.com>
CC: Chris Snook <chris.snook@...il.com>, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "Ingo
 Molnar" <mingo@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Jeff Garzik
	<jeff@...zik.org>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net v3] ethernet: atl1: Add missing DMA mapping error
 checks

From: Thomas Fourier <fourier.thomas@...il.com>
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@...il.com>
> ---
>  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