netdev - Re: [PATCH v6 bpf] xsk: fix immature cq descriptor production

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: <CAL+tcoD=Gn6ZmJ+_Y48vPRyHVHmP-7irsx=fRVRnyhDrpTrEtQ@mail.gmail.com>
Date: Tue, 26 Aug 2025 21:40:53 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Daniel Borkmann <daniel@...earbox.net>
Cc: Dan Carpenter <dan.carpenter@...aro.org>, 
	Maciej Fijalkowski <maciej.fijalkowski@...el.com>, bpf@...r.kernel.org, ast@...nel.org, 
	andrii@...nel.org, netdev@...r.kernel.org, magnus.karlsson@...el.com, 
	stfomichev@...il.com, aleksander.lobakin@...el.com, 
	Eryk Kubanski <e.kubanski@...tner.samsung.com>
Subject: Re: [PATCH v6 bpf] xsk: fix immature cq descriptor production

On Tue, Aug 26, 2025 at 8:24 PM Daniel Borkmann <daniel@...earbox.net> wrote:
>
> On 8/26/25 10:14 AM, Dan Carpenter wrote:
> > On Wed, Aug 20, 2025 at 05:44:16PM +0200, Maciej Fijalkowski wrote:
> >>                      return ERR_PTR(err);
> >>
> >>              skb_reserve(skb, hr);
> >> +
> >> +            addrs = kmem_cache_zalloc(xsk_tx_generic_cache, GFP_KERNEL);
> >> +            if (!addrs) {
> >> +                    kfree(skb);
> >
> > This needs to be kfree_skb(skb);
>
> Oh well, good catch! Maciej, given this commit did not land yet in Linus' tree,
> I can toss the commit from bpf tree assuming you send a v7?
>
> Also, looking at xsk_build_skb(), do we similarly need to free that allocated
> skb when we hit the ERR_PTR(-EOVERFLOW) ? Mentioned function has the following
> in the free_err path:
>
>          if (first_frag && skb)
>                  kfree_skb(skb);
>
> Pls double check.

Sorry to join the party late...

I have to bring bad news here. After I tested[1] on VM, I saw an
around 18.8% performance decrease (from 745,705 to 627,331) with this
patch applied.It can be reproduced stably :(

[1]: ./xdpsock -i eth1 -t  -S -s 64

Thanks,
Jason