linux-kernel - Re: Re: Re: Re: Re: [PATCH bpf v2] xsk: Fix out of order segment free in __xsk_generic

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aEmANxmvCLMXdQpi@boxer>
Date: Wed, 11 Jun 2025 15:10:15 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Eryk Kubanski <e.kubanski@...tner.samsung.com>
CC: Stanislav Fomichev <stfomichev@...il.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "bjorn@...nel.org" <bjorn@...nel.org>,
	"magnus.karlsson@...el.com" <magnus.karlsson@...el.com>,
	"jonathan.lemon@...il.com" <jonathan.lemon@...il.com>
Subject: Re: Re: Re: Re: Re: [PATCH bpf v2] xsk: Fix out of order segment
 free in __xsk_generic_xmit()

On Tue, Jun 10, 2025 at 11:11:25AM +0200, Eryk Kubanski wrote:
> > I've come with something as below. Idea is to embed addr at the end of
> > linear part of skb/at the end of page frag.
> 
> Are you sure that this is safe for other components?
> 
> So instead of storing entire array at the skb_shared_info (skb->end),
> we store It 8-bytes per PAGE fragment and 8-byte at skb->end.
> Technically noone should edit skb past-the-end, it
> looks good to me.
> 
> In xsk_cq_submit_locked() you use only xskq_prod_write_addr.
> I think that this may cause synchronization issues on reader side.
> You don't perform ATOMIC_RELEASE, so this producer incrementation
> is atomic (u32) but it doesn't synchronize address write.
> 
> I think that you should accumulate local producer and
> store it with ATOMIC_RELEASE after writing descriptors.
> In current situation someone may see producer incrementation,
> but address stored in this bank doesn't need to be synchronized yet.

Hi Eryk, yes I missed the smp_store_release that __xskq_prod_submit does -
magic of late night refactors :) but main point was to share the approach
in terms of addr storage.

As you also said IFF_TX_SKB_NO_LINEAR case needs to be addressed as well
as it uses the same skb destructor. Since these pages come directly from
umem we don't need this quirk, we should be able to get the addr from
page.