linux-kernel - RE: [PATCH net] net: xilinx: axienet: Increment Rx skb ring head pointer after BD is successfully allocated in dmaengine flow

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:
 <BL3PR12MB65712291B55DD8D535BAE667C92EA@BL3PR12MB6571.namprd12.prod.outlook.com>
Date: Sat, 9 Aug 2025 20:31:40 +0000
From: "Gupta, Suraj" <Suraj.Gupta2@....com>
To: Jakub Kicinski <kuba@...nel.org>
CC: "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "Simek, Michal"
	<michal.simek@....com>, "sean.anderson@...ux.dev" <sean.anderson@...ux.dev>,
	"Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>, "horms@...nel.org"
	<horms@...nel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Katakam, Harini" <harini.katakam@....com>
Subject: RE: [PATCH net] net: xilinx: axienet: Increment Rx skb ring head
 pointer after BD is successfully allocated in dmaengine flow

[Public]

Hi Jakub,
> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Saturday, August 9, 2025 12:36 AM
> To: Gupta, Suraj <Suraj.Gupta2@....com>
> Cc: andrew+netdev@...n.ch; davem@...emloft.net;
> edumazet@...gle.com; pabeni@...hat.com; Simek, Michal
> <michal.simek@....com>; sean.anderson@...ux.dev; Pandey, Radhey
> Shyam <radhey.shyam.pandey@....com>; horms@...nel.org;
> netdev@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org; Katakam, Harini <harini.katakam@....com>
> Subject: Re: [PATCH net] net: xilinx: axienet: Increment Rx skb ring head
> pointer after BD is successfully allocated in dmaengine flow
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On Wed, 6 Aug 2025 00:49:58 +0530 Suraj Gupta wrote:
> > In DMAengine flow, AXI DMA driver invokes callback before freeing BD
> > in irq handling path.
> > In Rx callback (axienet_dma_rx_cb()), axienet driver tries to allocate
> > new BD after processing skb.
> > This will be problematic if both AXI-DMA and AXI ethernet have same BD
> > count as all Rx BDs will be allocated initially and it won't be able
> > to allocate new one after Rx irq. Incrementing head pointer w/o
> > checking for BD allocation will result in garbage values in skb BD and
> > cause the below kernel crash:
> >
> > Unable to handle kernel paging request at virtual address
> > fffffffffffffffa <snip> Internal error: Oops: 0000000096000006 [#1]
> > SMP pc : axienet_dma_rx_cb+0x78/0x150 lr :
> > axienet_dma_rx_cb+0x78/0x150  Call trace:
> >   axienet_dma_rx_cb+0x78/0x150 (P)
> >   xilinx_dma_do_tasklet+0xdc/0x290
> >   tasklet_action_common+0x12c/0x178
> >   tasklet_action+0x30/0x3c
> >   handle_softirqs+0xf8/0x230
> > <snip>
>
> Do you mean that we're incrementing lp->rx_ring_head before we know that
> the submission will succeed? Potentially leaving an uninitialized entry (say at
> index n), next attempt will try to use the next entry (n + 1) but the completion
> will not know about the skip so it will try to complete entry n ?
>
> This is really not coming thru in your explanation.
>
You're right, I only explained the issue I faced while running perf test with same BD count in axienet and AXI DMA. Above is more generic explanation of the situation here. I'll modify the description.

> The fix itself seems incomplete. Even if we correctly skip the increment we
> will never try to catch up with the allocations, the ring will have fewer
> outstanding Rx skbs until reset, right? Worst case we drop all the skbs and the
> ring will be empty, no Rx will happen until reset.
> The shutdown path seems to be checking for skb = NULL so I guess it's correct
> but good to double check..
> --
> pw-bot: cr

I agree that Rx ring will have fewer outstanding skbs. But I think that difference won't exceed one anytime as descriptors submission will fail only once due to insufficient space in AXIDMA BD ring. Rest of the time we already will have an extra entry in AXIDMA BD ring. Also, invoking callback (where Rx skb ring hp is filled in axienet)and freeing AXIDMA BD are part of same tasklet in AXIDMA driver so next callback will only be called after freeing a BD.
I tested running stress tests (Both UPD and TCP netperf).
Please let me know your thoughts if I'm missing something.

Thanks,
Suraj