Re: [PATCH net v2 2/2] veth: fix skb length accounting after XDP frag adjustment
sun jian <[email protected]> Mon, 3 Aug 2026 09:32:56 +0800
| Newsgroups | org.kernel.vger.bpf,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kernel.vger.stable |
|---|---|
| Message-ID | <CABFUUZGTJptpOKKAeU3ThibmPpXize4Eud2=EBX-vqaX3YH4pg@mail.gmail.com> |
On Sat, Aug 1, 2026 at 12:14 AM Mohsin Bashir <[email protected]> wrote: > > > > On 7/30/26 8:23 PM, Sun Jian wrote: > > veth exposes non-linear skb fragments through an xdp_buff. If an XDP > > program adjusts the fragment area, veth_xdp_rcv_skb() 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. > > > > The fragment accounting must run before the linear tail adjustment: > > when bpf_xdp_adjust_tail() shrinks the packet into the linear area it > > releases all fragments, and __skb_put() requires skb->data_len == 0 > > by that point. > > > > 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: 718a18a0c8a6 ("veth: Rework veth_xdp_rcv_skb in order to accept non-linear skb") > > Cc: [email protected] > > Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/ > > Signed-off-by: Sun Jian <[email protected]> > > --- > > drivers/net/veth.c | 23 +++++++++++++++-------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/veth.c b/drivers/net/veth.c > > index 00e34afd858e..348391e87e14 100644 > > --- a/drivers/net/veth.c > > +++ b/drivers/net/veth.c > > @@ -865,18 +865,25 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, > > > > skb_reset_mac_header(skb); > > > > - /* check if bpf_xdp_adjust_tail was used */ > > - off = xdp->data_end - orig_data_end; > > - if (off != 0) > > - __skb_put(skb, off); /* positive on grow, negative on shrink */ > > - > > /* 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). Remove the old fragment contribution > > + * from skb->len before updating data_len, then add the new one back. > > + * This must precede the linear tail adjustment below: a changed > > + * data_end implies that no fragments remain, and __skb_put() requires > > + * a linear skb. > > */ > > - if (xdp_buff_has_frags(xdp)) > > + skb->len -= skb->data_len; > > + if (xdp_buff_has_frags(xdp)) { > > skb->data_len = skb_shinfo(skb)->xdp_frags_size; > > - else > > + skb->len += skb->data_len; > > + } else { > > skb->data_len = 0; > > + } > > + > > + /* check if bpf_xdp_adjust_tail was used */ > > + off = xdp->data_end - orig_data_end; > > + if (off != 0) > > + __skb_put(skb, off); /* positive on grow, negative on shrink */ > > > > skb->protocol = eth_type_trans(skb, rq->dev); > > > > I am most likely missing something here but what happens if we have > frags and we attempt to advance data_end while leaving some frags > present (e.g., bpf_xdp_pull_data())? Looks like, in that case we would > issue __skb_put(skb, off) with off > 0 and we would hit > SKB_LINEAR_ASSERT() because skb is still non-linear? Hi Mohsin, Thanks for catching this. You're right that bpf_xdp_pull_data() can advance data_end while leaving frags present. I'll investigate this path and follow up. Thanks, Sun Jian