Re: [PATCH net v3 3/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Incorrect completion address reporting to user-space and leaked descriptor on `-EOVERFLOW` in multi-buffer packets.
- [High] Packet stream corruption when dropping oversized multi-buffer packets.
--

commit 38322c7d2a85e9ef029404cbf4ae750b54520d27
Author: Jason Xing <kerneljasonxing@gmail.com>
xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path

This patch fixes a use-after-free in the xsk_build_skb() error path
by ensuring that in-progress multi-buffer socket buffers are not
freed if they are still referenced by the xdp_sock.

Link: https://lore.kernel.org/all/20260415082654.21026-4-kerneljasonxing@gmail.com/

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -969,7 +969,7 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
>  	return skb;
>  
>  free_err:
> -	if (skb && !skb_shinfo(skb)->nr_frags)
> +	if (skb && !xs->skb)
>  		kfree_skb(skb);
>  
>  	if (err == -EOVERFLOW) {

I noticed a couple of pre-existing regressions in this error path.

Do the error handling paths for -EOVERFLOW leak the original descriptor
address and submit an invalid address to user-space?

When a continuation descriptor exceeds the fragment limit, the code
jumps to the free_err label before the current descriptor address is
stored in the xsk_addr array.

However, the error path still increments the descriptor count:

        if (xs->skb) {
            /* Drop the packet */
            xsk_inc_num_desc(xs->skb);
            xsk_drop_skb(xs->skb);
        }

When xsk_destruct_skb() loops up to the new descriptor count, could it
read past the initialized entries, find a zero address from the earlier
kmem_cache_zalloc(), and submit address 0 to the completion queue instead
of the actual dropped address?

Does dropping the socket buffer here cause packet stream corruption?

When a multi-buffer packet is dropped due to an -EOVERFLOW error,
the partial packet is discarded and the socket buffer pointer is cleared.

Because there is no state tracking to discard the remaining descriptors
of the dropped packet, will the transmission loop process the next
descriptor (which is just a continuation of the dropped packet) as the
beginning of a completely new packet?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260422033650.68457-1-kerneljasonxing@gmail.com?part=3