netdev - Re: [PATCH net v3] netpoll: Fix deadlock in memory allocation under spinlock

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: <20251016162323.176561bd@kernel.org>
Date: Thu, 16 Oct 2025 16:23:23 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Breno Leitao <leitao@...ian.org>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
 <horms@...nel.org>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 kernel-team@...a.com
Subject: Re: [PATCH net v3] netpoll: Fix deadlock in memory allocation under
 spinlock

On Tue, 14 Oct 2025 09:37:50 -0700 Breno Leitao wrote:
> +	while (1) {
> +		spin_lock_irqsave(&skb_pool->lock, flags);
> +		if (skb_pool->qlen >= MAX_SKBS)
> +			goto unlock;
> +		spin_unlock_irqrestore(&skb_pool->lock, flags);

No need for the lock here:

	if (READ_ONCE(..) >= MAX_SKBS)

>  		skb = alloc_skb(MAX_SKB_SIZE, GFP_ATOMIC);
>  		if (!skb)
> -			break;
> +			return;
>  
> +		spin_lock_irqsave(&skb_pool->lock, flags);
> +		if (skb_pool->qlen >= MAX_SKBS)
> +			/* Discard if len got increased (TOCTOU) */
> +			goto discard;

Not sure this is strictly needed, the number 32 (MAX_SKBS) was not
chosen super scientifically anyway, doesn't matter if we go over a
little. But if we care I think we can:

	if (skb_pool->qlen < MAX_SKBS)
		__skb_queue_tail(skb_pool, skb);
	else
		dev_kfree_skb_any(skb);

and there's no need for the gotos

>  		__skb_queue_tail(skb_pool, skb);
> +		spin_unlock_irqrestore(&skb_pool->lock, flags);
>  	}
-- 
pw-bot: cr