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: <20250326100327.GY892515@horms.kernel.org>
Date: Wed, 26 Mar 2025 10:03:27 +0000
From: Simon Horman <horms@...nel.org>
To: Thangaraj Samynathan <thangaraj.s@...rochip.com>
Cc: netdev@...r.kernel.org, bryan.whitehead@...rochip.com,
	UNGLinuxDriver@...rochip.com, andrew+netdev@...n.ch,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] net: ethernet: microchip: lan743x: Fix memory
 allocation failure

On Tue, Mar 25, 2025 at 04:26:53PM +0530, Thangaraj Samynathan wrote:
> The driver allocates ring elements using GFP_DMA flags. There is
> no dependency from LAN743x hardware on memory allocation should be
> in DMA_ZONE. Hence modifying the flags to use only GFP_ATOMIC
> 
> Fixes: e8684db191e4 ("net: ethernet: microchip: lan743x: Fix skb allocation failure")
> Signed-off-by: Thangaraj Samynathan <thangaraj.s@...rochip.com>

Hi Thangaraj,

As per the discussion of v1 I agree that dropping GFP_DMA and keeping
GFP_ATOMIC makes sense. So the code change looks good to me.

However, I am not clear that this is fixing a bug, which is
the main pre-requisite for patches for 'net'.

If not, perhaps it should be targeted at 'net-next' instead.
In which case the Fixes tag should be dropped, but you can refer to the
commit that introduced this problem in the commit message using this
syntax if you wish.

   commit e8684db191e4 ("net: ethernet: microchip: lan743x: Fix skb
   allocation failure")

e.g.:

   The driver allocates ring elements using GFP_DMA flags. There is
   no dependency from LAN743x hardware on memory allocation should be
   in DMA_ZONE. Hence modifying the flags to use only GFP_ATOMIC

   Introduced by commit e8684db191e4 ("net: ethernet: microchip: lan743x:
   Fix skb allocation failure").

   Signed-off-by: ...

If you do post for net-next, keep in mind that net-next is currently closed
for the merge window. I expect is should re-open around the 14th April.
So please post any patches for net-next after it re-opens.

> ---
> v0
> -Initial Commit
> 
> v1
> -Modified GFP flags from GFP_KERNEL to GFP_ATOMIC
> -added fixes tag

Link to v1: https://lore.kernel.org/all/20250314070227.24423-1-thangaraj.s@microchip.com/

> 
> ---
>  drivers/net/ethernet/microchip/lan743x_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index 23760b613d3e..8b6b9b6efe18 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -2495,8 +2495,7 @@ static int lan743x_rx_process_buffer(struct lan743x_rx *rx)
>  
>  	/* save existing skb, allocate new skb and map to dma */
>  	skb = buffer_info->skb;
> -	if (lan743x_rx_init_ring_element(rx, rx->last_head,
> -					 GFP_ATOMIC | GFP_DMA)) {
> +	if (lan743x_rx_init_ring_element(rx, rx->last_head, GFP_ATOMIC)) {
>  		/* failed to allocate next skb.
>  		 * Memory is very low.
>  		 * Drop this packet and reuse buffer.
> -- 
> 2.25.1
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ