Re: [PATCH net v2 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures

On Tue, Apr 21, 2026 at 5:40 PM <[email protected]> wrote:
>
> Sashiko AI review found 1 potential issue(s):
> - [High] On 32-bit systems, xsk_drop_untrans_skb() will call kmem_cache_free() on an uninitialized destructor_arg pointer in early allocation failure paths, causing slab corruption.
> --
>
> commit cf643670755512ad76d19d6d0707e86a2a05c671
> Author: Jason Xing <[email protected]>
>
> xsk: fix u64 descriptor address truncation on 32-bit architectures
>
> This commit fixes a truncation issue of the 64-bit descriptor address
> on 32-bit architectures by allocating an xsk_addrs struct to store the
> full address instead of packing it into the destructor_arg pointer.
> It also updates the related cleanup and initialization functions to
> handle the newly allocated struct.
>
> Link: https://lore.kernel.org/all/[email protected]/
>
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 9c06a4e868725..0232559d5348f 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> [ ... ]
> > @@ -799,6 +825,12 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
> >
> >  static void xsk_drop_untrans_skb(struct sk_buff *skb)
> >  {
> > +     if (!IS_ENABLED(CONFIG_64BIT) && !xsk_skb_destructor_is_addr(skb)) {
> > +             struct xsk_addrs *xsk_addr;
> > +
> > +             xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
> > +             kmem_cache_free(xsk_tx_generic_cache, xsk_addr);
> > +     }
>
> Could this pass an uninitialized pointer to kmem_cache_free() and cause slab
> corruption?
>
> When a new socket buffer is allocated via sock_alloc_send_skb(), the
> networking core's __alloc_skb() function only zeroes the skb_shared_info
> struct up to the dataref field. The destructor_arg field resides after
> dataref and contains uninitialized garbage from the slab.
>
> If an error occurs before destructor_arg is explicitly initialized, such as
> when xsk_skb_init_misc() returns -ENOMEM:
>
> xsk_build_skb_zerocopy()
>     ...
>     skb = sock_alloc_send_skb(&xs->sk, hr, 1, &err);
>     ...
>     err = xsk_skb_init_misc(skb, xs, desc->addr);
>     if (unlikely(err)) {
>         xsk_drop_untrans_skb(skb);
>         return ERR_PTR(err);
>     }
>
> In xsk_drop_untrans_skb() on 32-bit systems, xsk_skb_destructor_is_addr(skb)
> evaluates to !skb_shinfo(skb)->destructor_arg. Because destructor_arg
> contains non-zero garbage, the function returns false. The check in
> xsk_drop_untrans_skb() (!xsk_skb_destructor_is_addr(skb)) then succeeds,
> passing the garbage pointer to kmem_cache_free().

Yes, you're right. After I removed the untrans_skb code block as
suggested by Stan, the problem is gone :)

>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8