Re: [PATCH v3] xsk: fix memory corruptions in net/core/xdp.c

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Fri, Oct 16, 2026 at 10:15:42PM +0200, Björn Töpel wrote:
> Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:
> 
> > On Fri, Oct 16, 2026 at 01:19:35PM +0200, Björn Töpel wrote:
> >> Waking a dead thread: RISE FROM YOUR GRAVE!
> >> 
> >> Clement Lecigne <clecigne@google.com> writes:
> >> 
> >> > From: Clément Lecigne <clecigne@google.com>
> >> >
> >> > Commit 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
> >> > introduced a vulnerability in the handling of XDP_PASS for AF_XDP zero-copy
> >> > frames.
> >> 
> >> Hmm, I acually hit this in my WIP for UMEM page pool provider hack.
> >> 
> >> > Note: Currently, this specific AF_XDP zero-copy conversion path is only
> >> > reachable from the drivers/net/ethernet/intel/ice and
> >> > drivers/net/ethernet/intel/idpf drivers.
> >> >
> >> > When building an skb, xdp_build_skb_from_zc() uses the chunk size
> >> > (xdp->frame_sz) for the allocation. However, napi_build_skb() automatically
> >> > reserves space at the end of the allocation for the skb_shared_info
> >> > structure. 
> >> >
> >> > Most high performance UMEM applications use 4K chunks, where the
> >> > corruption cannot happen.
> >> 
> >> I think this can also happen with 4K UMEM chunks.
> >> 
> >> For example, on ice, use MTU 3800, 4K chunks, and no extra UMEM
> >> headroom. Attach an xdp.frags program that returns XDP_PASS, and bind
> >> the socket without XDP_USE_SG.
> >> 
> >> A 3814-byte Ethernet frame fits in the UMEM chunk:
> >> 
> >>   256 headroom + 3814 packet = 4070 bytes
> >> 
> >> But the copied skb also needs space for skb_shared_info. Using
> >> the 320-byte size from your example:
> >> 
> >>   256 headroom + 3814 packet + 320 shared info = 4390 bytes
> >> 
> >> The old code allocates only 4096 bytes, so the packet copy
> >> overwrites skb_shared_info.
> >> 
> >> Am I missing something?
> >
> > I was pushing back on relying on scenario above as we respect the tailroom
> > now since some time:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a315e022a72d95ef5f1d4e58e903cb492b0ad931
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1ee1605138fc94cc8f8f273321dd2471c64977f9
> 
> Hmm, isn't that only with XDP_USE_SG? My example binds without it.
> 
> Then xsk_pool_get_rx_frame_size() returns 3840. XDP_PASS still builds an
> skb with skb_shared_info, even if it's linear. So we only have 4096 -
> 256 - 320 = 3520 bytes for the packet.
> 
> Or am I missing another check?

I missed your trick with xdp.frags on prog side. Get this fixed then! :D I
had a thought like 'who cares if we overwrite struct not used in linear
traffic' but as Clement said somewhere destructor_arg overwrite would make
a mess.

Side note is ice currently still only allows 3k buffers for linear XDP
progs but it currently works with page_pool not with magic page split
mechanism, so the limit should be lifted.

I might take this as an AR but anyone reading this in future feel free to
pick it up.

Bjorn, let us see if Clement responses in some nearby future, otherwise
please go forward and submit a standalone fix out of your series.

Thanks!