netdev - Re: [PATCH v5 bpf 1/2] xdp: introduce xdp_convert_skb_to

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: <20251030190511.62575480@kernel.org>
Date: Thu, 30 Oct 2025 19:05:11 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: <bpf@...r.kernel.org>, <ast@...nel.org>, <daniel@...earbox.net>,
 <hawk@...nel.org>, <netdev@...r.kernel.org>, <magnus.karlsson@...el.com>,
 <aleksander.lobakin@...el.com>, <ilias.apalodimas@...aro.org>,
 <toke@...hat.com>, <lorenzo@...nel.org>,
 <syzbot+ff145014d6b0ce64a173@...kaller.appspotmail.com>, Ihor Solodrai
 <ihor.solodrai@...ux.dev>, Octavian Purdila <tavip@...gle.com>
Subject: Re: [PATCH v5 bpf 1/2] xdp: introduce xdp_convert_skb_to_buff()

On Thu, 30 Oct 2025 21:22:34 +0100 Maciej Fijalkowski wrote:
> > > Why do you say so?
> > > 
> > > netif_receive_generic_xdp()
> > > 	netif_skb_check_for_xdp()
> > > 	skb_cow_data_for_xdp() failed
> > > 		go through skb linearize path
> > > 			returned skb data is backed by kmalloc, not page_pool,
> > > 			means mem type for this particular xdp_buff has to be
> > > 			MEM_TYPE_PAGE_SHARED
> > > 
> > > Are we on the same page now?  
> > 
> > No, I think I already covered this, maybe you disagreed and I missed it.
> > 
> > The mem_type set here is expected to be used only for freeing pages. 
> > XDP can only free fagments (when pkt is trimmed), it cannot free the
> > head from under the skb. So only fragments matter here, we can ignore
> > the head.  
> 
> ...and given that linearize path would make skb a frag-less one...okay -
> I'm buying this! :D I have some other thoughts, but I would like to
> finally close this pandora's box, you probably have similar feelings.
> 
> So plain assignment like:
> xdp->rxq->mem.type = MEM_TYPE_PAGE_POOL;

Yes, LGTM.

> would be fine for you? Plus AI reviewer has kicked me in the nuts on veth
> patch so have to send v6 anyways.

The veth side unfortunately needs more work than Mr Robot points out.
For some reason veth tries to turn skb into an xdp_frame..

Either we have to make it not do that - we could probably call
xdp_do_generic_redirect() and for Tx .. figure out the right incantation 
to give the frame back to the peer veth.

Or, if we didn't hit CoW, you need to actually add the incantation we
removed here, there:

	xdp->rxq->mem.type = skb->pp_recycle && page_pool_page_is_pp(..) ?
		MEM_TYPE_PAGE_POOL : MEM_TYPE_PAGE_SHARED;

Or CoW the head retroactively if we hit the Tx/Redir path.

My intuition is that first option (making the handling as similar to
XDP generic as possible) is going to be least buggy long term.