netdev - Re: [PATCH v1] net: stmmac: Disable queues before tx

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: <46190ba0-9cb3-4488-abdb-977fd8fda381@intel.com>
Date: Wed, 4 Feb 2026 14:50:18 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: <muhammad.nazim.amirul.nazle.asmade@...era.com>, <netdev@...r.kernel.org>
CC: <andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <ast@...nel.org>,
	<daniel@...earbox.net>, <hawk@...nel.org>, <john.fastabend@...il.com>,
	<sdf@...ichev.me>, <bpf@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] net: stmmac: Disable queues before tx_disable

On 2/3/2026 6:15 PM, muhammad.nazim.amirul.nazle.asmade@...era.com wrote:
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@...era.com>
> 
> Disable all queues and disconnect before tx_disable in stmmac_xdp_release
> to prevent a corner case where packet may be still queued at the same time
> tx_disable is called resulting in kernel panic if some packet still has to
> be processed.
> 

It is helpful to include a (trimmed) copy of the panic if you have one.

> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@...era.com>

This is a net fix which should have the net tag in the subject and a 
Fixes commit. If it is a regression you should tag as the commit which 
introduced it. Otherwise, indicate the first commit which added 
stmmac_xdp_release().

I saw that there is also __sttmac_release which calls 
stmmac_disable_all_queues() first before calling netif_tx_disable(), so 
this aligns the XDP case with the non-XDP case, so this seems like the 
right approach.

The stmmac_disable_all_queue essentially just calls napi_disable on all 
of the appropriate queues. The netif_tx_disable() calls 
netif_tx_stop_queue() on each queue....

I don't actually know the proper sequencing here. I see several other 
drivers seem to have their napi_disable happening after the call to 
netif_tx_disable. It seems to me like the existing code should be safe, 
at least based on many other driver examples.

I think we need to see the supposed panic to understand the problem and 
whether this fix is correct.

> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index da206b24aaed..7f5a6f30c231 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6967,15 +6967,15 @@ void stmmac_xdp_release(struct net_device *dev)
>   	struct stmmac_priv *priv = netdev_priv(dev);
>   	u32 chan;
>   
> -	/* Ensure tx function is not running */
> -	netif_tx_disable(dev);
> -
>   	/* Disable NAPI process */
>   	stmmac_disable_all_queues(priv);
>   
>   	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
>   		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
>   

The only other action here is to cancel some tx_queue hrtimer? Could 
this have an impact?

> +	/* Ensure tx function is not running */
> +	netif_tx_disable(dev);
> +
>   	/* Free the IRQ lines */
>   	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
>