Re: [PATCH bpf v2] veth: convert frag_list skbs before running XDP

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] veth_xdp_rcv_skb() fails to adjust skb->len when an XDP program modifies fragment lengths, causing state desynchronization.
--

--- Patch [1]: [PATCH bpf v2] veth: convert frag_list skbs before running XDP ---
commit c3ece024a1663f1b10d53a17f59581af7993ec1f
Author: Matt Fleming <mfleming@cloudflare.com>

veth: convert frag_list skbs before running XDP
    
A frag_list skb can reach veth with data_len set but nr_frags zero.
This patch routes non-linear skbs through skb_pp_cow_data() before exposing
them to XDP, preventing a crash in AF_XDP copy mode.

> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 1c51421491753..00e34afd858e7 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c

[ ... ]

> @@ -771,7 +771,7 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
>  	xdp_prepare_buff(xdp, skb->head, skb_headroom(skb),
>  			 skb_headlen(skb), true);
>  
> -	if (skb_is_nonlinear(skb)) {
> +	if (skb_shinfo(skb)->nr_frags) {
>  		skb_shinfo(skb)->xdp_frags_size = skb->data_len;
>  		xdp_buff_set_frags_flag(xdp);
>  	} else {

[Severity: Critical]
This is a pre-existing issue, but while reviewing the XDP fragment changes, I
noticed a potential state desynchronization in veth_xdp_rcv_skb() when handling
XDP tail adjustments.

When a fragmented packet is received and an attached XDP program calls
bpf_xdp_adjust_tail() to shrink or grow the packet, veth_xdp_rcv_skb() updates
skb->data_len:

drivers/net/veth.c:veth_xdp_rcv_skb() {
    ...
	/* 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.
	 */
	if (xdp_buff_has_frags(xdp))
		skb->data_len = skb_shinfo(skb)->xdp_frags_size;
	else
		skb->data_len = 0;
    ...
}

Does skb->len also need to be updated with the corresponding delta when
skb->data_len is modified?

If skb->len isn't updated alongside skb->data_len, could skb_headlen()
evaluate to a size larger than the actual linear buffer? If the tail is
shrunk, this desynchronization might lead to out-of-bounds reads or writes
past the allocated linear memory during subsequent network stack processing.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720140545.461747-1-matt@readmodwrite.com?part=1