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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect ordering of SKB length synchronization and manipulation helpers causes BUG_ON() panics in __skb_put and __skb_pull.
--

--- Patch [2]: [PATCH net 2/2] veth: fix skb length accounting after XDP frag adjustment ---
commit 7bca8b2841500975db8d1e89e6bd51e147aa4ad3
Author: Sun Jian <sun.jian.kdev@gmail.com>

veth: fix skb length accounting after XDP frag adjustment

This commit fixes an issue where veth_xdp_rcv_skb() left skb->len containing
stale fragment sizes after an XDP program adjusted the fragment area. It
subtracts the old data_len before replacing it and adds the new data_len
afterwards to keep skb->len and skb->data_len synchronized.

Link: https://lore.kernel.org/r/20260720141859.19FF41F000E9@smtp.kernel.org
Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/

> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 00e34afd858e7..a956498a073be 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -871,12 +871,14 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
>  		__skb_put(skb, off); /* positive on grow, negative on shrink */

[Severity: High]
This is a pre-existing issue, but can this sequence of SKB length
synchronizations trigger a kernel panic due to the order of operations?

If an XDP program modifies a fragmented packet, veth_xdp_rcv_skb() adjusts
the head and tail before syncing skb->data_len:

    if (off < 0)
        __skb_pull(skb, -off);
    ...
    if (off != 0)
        __skb_put(skb, off); /* positive on grow, negative on shrink */

If the XDP program shrinks the tail past the fragments, __skb_put() is called
with a negative offset while skb->data_len is still non-zero. Doesn't this
trigger SKB_LINEAR_ASSERT if CONFIG_DEBUG_SG is enabled?

Likewise, if the program shrinks the head (triggering __skb_pull() before
updating skb->data_len), could skb->len drop below the stale skb->data_len
value? This would hit the unconditional BUG_ON(skb->len < skb->data_len)
inside __skb_pull() and cause a denial of service.

Would it make sense to synchronize skb->data_len and skb->len before calling
__skb_pull() and __skb_put()?

>  
>  	/* 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;
>  
>  	skb->protocol = eth_type_trans(skb, rq->dev);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727032535.13469-1-sun.jian.kdev@gmail.com?part=2