Re: [PATCH net 1/2] net: fix skb length accounting after generic XDP frag adjustment

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

On Sun, Jul 26, 2026 at 08:25:34PM -0700, Sun Jian wrote:
> Generic XDP exposes non-linear skb fragments through an xdp_buff. If an
> XDP program adjusts the fragment area, bpf_prog_run_generic_xdp() copies
> xdp_frags_size back to skb->data_len but leaves skb->len containing the
> old fragment contribution.
> 
> After a fragment shrink, this makes skb_headlen() larger than the actual
> linear area. In the reproduced UDP receive path, __skb_datagram_iter()
> copied 1024 bytes past the actual linear tail to userspace, starting at
> struct skb_shared_info. The copied bytes included the affected skb's
> nr_frags, xdp_frags_size and a kernel pointer from
> skb_shinfo(skb)->frags[0]. Real packet data was displaced by the same
> amount and truncated at the end.
> 
> Subtract the old data_len before replacing it and add the new data_len
> afterwards, keeping skb->len and skb->data_len synchronized.
> 
> A 60000-byte UDP datagram on a veth pair with MTU 64000 was shortened by
> 1024 bytes from its fragment area. Before the fix, all 10 runs produced
> corrupted payloads. After the fix, all 10 runs matched the expected
> payload exactly.
> 
> Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/20260720141859.19FF41F000E9@smtp.kernel.org
> Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  net/core/dev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5933c5dab09e..94204702cb02 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5517,12 +5517,14 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
>  	}
>  
>  	/* XDP frag metadata (e.g. nr_frags) are updated in eBPF helpers
> -	 * (e.g. bpf_xdp_adjust_tail), we need to update data_len here.
> +	 * (e.g. bpf_xdp_adjust_tail), update skb length fields here.
>  	 */
> +	skb->len -= skb->data_len;
>  	if (xdp_buff_has_frags(xdp))
>  		skb->data_len = skb_shinfo(skb)->xdp_frags_size;
>  	else
>  		skb->data_len = 0;
> +	skb->len += skb->data_len;

hm that reads a bit weird to me. maybe we could follow the existing
semantics and reuse @off ? that would be a nicer way to update ::len only
by a delta.

if we would stick with existing approach i would appreciate comment being
more elaborative, where we could explain that it is simpler to wipe out
data_len altogether from len and then just add it back.

>  
>  	/* check if XDP changed eth hdr such SKB needs update */
>  	eth = (struct ethhdr *)xdp->data;
> -- 
> 2.43.0
>