netdev - Re: [PATCH net 3/4] net: ti: icssg-prueth: Fix race condition for traffic from different network sockets

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250501075615.34573158@kernel.org>
Date: Thu, 1 May 2025 07:56:15 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Meghana Malladi <m-malladi@...com>
Cc: <dan.carpenter@...aro.org>, <john.fastabend@...il.com>,
 <hawk@...nel.org>, <daniel@...earbox.net>, <ast@...nel.org>,
 <pabeni@...hat.com>, <edumazet@...gle.com>, <davem@...emloft.net>,
 <andrew+netdev@...n.ch>, <bpf@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
 <linux-arm-kernel@...ts.infradead.org>, <srk@...com>, Vignesh Raghavendra
 <vigneshr@...com>, Roger Quadros <rogerq@...nel.org>, <danishanwar@...com>
Subject: Re: [PATCH net 3/4] net: ti: icssg-prueth: Fix race condition for
 traffic from different network sockets

On Mon, 28 Apr 2025 17:34:58 +0530 Meghana Malladi wrote:
> When dealing with transmitting traffic from different network
> sockets to a single Tx channel, freeing the DMA descriptors can lead
> to kernel panic with the following error:
> 
> [  394.602494] ------------[ cut here ]------------
> [  394.607134] kernel BUG at lib/genalloc.c:508!
> [  394.611485] Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
> 
> logs: https://gist.github.com/MeghanaMalladiTI/ad1d1da3b6e966bc6962c105c0b1d0b6
> 
> The above error was reproduced when sending XDP traffic from XSK
> socket along with network traffic from BSD socket. This causes
> a race condition leading to corrupted DMA descriptors. Fix this
> by adding spinlock protection while accessing the DMA descriptors
> of a Tx ring.

IDK how XSK vs normal sockets matters after what is now patch 4.
The only possible race you may be protecting against is pushing 
work vs completion. Please double check this is even needed, 
and if so fix the commit msg.

> Fixes: 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support")
> Signed-off-by: Meghana Malladi <m-malladi@...com>
> ---
>  drivers/net/ethernet/ti/icssg/icssg_common.c | 7 +++++++
>  drivers/net/ethernet/ti/icssg/icssg_prueth.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 4f45f2b6b67f..a120ff6fec8f 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -157,7 +157,9 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
>  	tx_chn = &emac->tx_chns[chn];
>  
>  	while (true) {
> +		spin_lock(&tx_chn->lock);
>  		res = k3_udma_glue_pop_tx_chn(tx_chn->tx_chn, &desc_dma);
> +		spin_unlock(&tx_chn->lock);
>  		if (res == -ENODATA)
>  			break;
>  
> @@ -325,6 +327,7 @@ int prueth_init_tx_chns(struct prueth_emac *emac)
>  		snprintf(tx_chn->name, sizeof(tx_chn->name),
>  			 "tx%d-%d", slice, i);
>  
> +		spin_lock_init(&tx_chn->lock);
>  		tx_chn->emac = emac;
>  		tx_chn->id = i;
>  		tx_chn->descs_num = PRUETH_MAX_TX_DESC;
> @@ -627,7 +630,9 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
>  	cppi5_hdesc_set_pktlen(first_desc, xdpf->len);
>  	desc_dma = k3_cppi_desc_pool_virt2dma(tx_chn->desc_pool, first_desc);
>  
> +	spin_lock_bh(&tx_chn->lock);
>  	ret = k3_udma_glue_push_tx_chn(tx_chn->tx_chn, first_desc, desc_dma);
> +	spin_unlock_bh(&tx_chn->lock);

I'm afraid this needs to be some form of spin_lock_irq
The completions may run from hard irq context when netpoll/netconsole
is used.
-- 
pw-bot: cr