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

[Clement Lecigne]

On Mon, Jun 29, 2026 at 12:34 PM Fijalkowski, Maciej
<maciej.fijalkowski@xxxxxxxxx> wrote:
>
> >
> > From: Clément Lecigne <clecigne@xxxxxxxxxx>
> >
> > 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.
> >
> > 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. However, if the UMEM is configured with 2KB
> > chunks (a very common configuration to maximize packet density in memory),
> > a standard 1500 MTU packet will trigger the corruption because the required
> > space exceeds the 2048 byte chunk size:
> >
> > Headroom (256) + Packet (1514) + skb_shared_info (320) = 2090 bytes
> >
> > Because 2090 bytes > 2048 bytes and __skb_put() does not perform bounds
> > checking, the memcpy() writes past the available linear data area and
> > corrupts the skb_shared_info structure. This can lead to arbitrary code
> > execution if pointers like destructor_arg are overwritten.
> >
> > Additionally, in xdp_copy_frags_from_zc(), the allocation size is set
> > strictly to the fragment size (len), but the subsequent memcpy() uses
> > LARGEST_ALIGN(len). This mismatch results in an out-of-bounds write of
> > up to 7 bytes, which triggers KASAN warnings and is unsafe despite typical
> > page pool allocator padding.
> >
> > Fix the skb allocation in xdp_build_skb_from_zc() by dynamically
> > calculating the exact truesize required using SKB_HEAD_ALIGN() to
> > properly account for the headroom, the LARGEST_ALIGN(len), and the
> > skb_shared_info overhead.
> >
> > Fix the out-of-bounds write in xdp_copy_frags_from_zc() by rounding up
> > the allocation request using LARGEST_ALIGN(len) to match the copy
> > operation.
> >
> > Fixes: 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
> > CC: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx>
> > CC: Eric Dumazet <edumazet@xxxxxxxxxx>
> > Signed-off-by: Clément Lecigne <clecigne@xxxxxxxxxx>
>
> Hi Clement,
>
> Do you have a reproducer for mentioned issue or is it only a fix from
> theoretical POV?
>
> To be clear, we were addressing headroom issues in this series:
> https://lore.kernel.org/bpf/20260402154958.562179-1-maciej.fijalkowski@xxxxxxxxx/
>
> so I wanted to ask if you are able to have this malformed setup for
> 2k chunk size. That series should not allow for that.

I didn't manage to build a malformed setup and only used a LKM to reproduce
the issue artificially. I shared some more details with you privately.

Thanks,
-clem

>
> I think this is the second time someone is trying to fix this area of code,
> so it is not a nack or something, let us fix this, but I wanted to have
> us on the same page.
>
> Thanks,
> Maciej
>
> > ---
> > Changes since v2:
> >  - Used LARGEST_ALIGN to calculate the len to account for the aligned
> > memcpy.
> >  - Fixed the commit message to include the idpf driver.
> >
> > Changes since v1:
> >  - Used SKB_HEAD_ALIGN to properly calculate the required allocation size
> >    including the skb_shared_info overhead.
> >  - Re-ordered variable declarations.
> >
> > ---
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index 9890a30584ba..7e39f17ad407 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -698,8 +698,8 @@ static noinline bool xdp_copy_frags_from_zc(struct
> > sk_buff *skb,
> >
> >       for (u32 i = 0; i < nr_frags; i++) {
> >               const skb_frag_t *frag = &xinfo->frags[i];
> > -             u32 len = skb_frag_size(frag);
> > -             u32 offset, truesize = len;
> > +             u32 offset, len = skb_frag_size(frag);
> > +             u32 truesize = LARGEST_ALIGN(len);
> >               struct page *page;
> >
> >               page = page_pool_dev_alloc(pp, &offset, &truesize);
> > @@ -738,9 +738,10 @@ static noinline bool xdp_copy_frags_from_zc(struct
> > sk_buff *skb,
> >   */
> >  struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
> >  {
> > +     u32 headroom = xdp->data_meta - xdp->data_hard_start;
> >       const struct xdp_rxq_info *rxq = xdp->rxq;
> > -     u32 len = xdp->data_end - xdp->data_meta;
> > -     u32 truesize = xdp->frame_sz;
> > +     u32 len = LARGEST_ALIGN(xdp->data_end - xdp->data_meta);
> > +     u32 truesize = SKB_HEAD_ALIGN(headroom + len);
> >       struct sk_buff *skb = NULL;
> >       struct page_pool *pp;
> >       int metalen;
> > @@ -762,7 +763,7 @@ struct sk_buff *xdp_build_skb_from_zc(struct
> > xdp_buff *xdp)
> >       }
> >
> >       skb_mark_for_recycle(skb);
> > -     skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
> > +     skb_reserve(skb, headroom);
> >
> >       memcpy(__skb_put(skb, len), xdp->data_meta,
> > LARGEST_ALIGN(len));
> >
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature