fix skb length accounting after XDP frag adjustment

This series fixes skb length accounting after an XDP program adjusts its fragment area, in both the generic XDP path (net/core/dev.c) and the veth native path (drivers/net/veth.c). When the fragment area is resized, skb->len and skb->data_len can go out of sync, and in the reproduced UDP receive path this leaked skb_shared_info contents (including a kernel pointer) to userspace while truncating real payload.

v2:
https://lore.kernel.org/bpf/[email protected]/

v1:
https://lore.kernel.org/bpf/[email protected]/

Changes since v2:
- 2/2: replace __skb_put() with skb_set_tail_pointer() and explicit skb->len accounting. As Mohsin pointed out, bpf_xdp_pull_data() can advance data_end while leaving frags present; __skb_put() would then hit SKB_LINEAR_ASSERT() on a still-nonlinear skb. Following Lorenzo's suggestion, use the same approach as bpf_prog_run_generic_xdp():
skb_set_tail_pointer() carries no linearity requirement. The v2 comment claiming a changed data_end implies no remaining frags was incorrect and has been dropped.
- 1/2: add Mohsin Bashir's Reviewed-by and Lorenzo Bianconi's Acked-by.

Tested with a local 8-case test harness (generic and native, each covering bpf_xdp_adjust_tail shrink variants plus a bpf_xdp_pull_data partial pull), all pass. A forced-tailroom debug reproducer confirms that v2's __skb_put() path panics with SKB_LINEAR_ASSERT() when bpf_xdp_pull_data() leaves frags while advancing data_end, and that this series does not.

Sun Jian (2):
net: fix skb length accounting after generic XDP frag adjustment
veth: fix skb length accounting after XDP frag adjustment

 drivers/net/veth.c | 14 ++++++++++----
 net/core/dev.c     | 10 +++++++---
 2 files changed, 17 insertions(+), 7 deletions(-)

2.43.0

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.

---
 net/core/dev.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

index 5933c5dab09e..5c37cf6c4aa1 100644
@@ -5517,12 +5517,16 @@ 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). Remove the old fragment contribution
+	 * from skb->len before updating data_len, then add the new one back.
 	 */
-	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 XDP changed eth hdr such SKB needs update */
 	eth = (struct ethhdr *)xdp->data;

2.43.0

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.

Additionally, bpf_xdp_pull_data() can advance data_end while leaving frags present. The skb is then still non-linear, so the old __skb_put(skb, off) triggers SKB_LINEAR_ASSERT().

Use skb_set_tail_pointer() and update skb->len explicitly instead, following bpf_prog_run_generic_xdp(). Unlike __skb_put(), skb_set_tail_pointer() does not require a linear skb.

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. A forced-tailroom reproducer also exercises bpf_xdp_pull_data() with frags still present; the old code triggers SKB_LINEAR_ASSERT(), while this fix passes 10/10 runs.

---
 drivers/net/veth.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

index 00e34afd858e..f80f6584a6ff 100644
@@ -865,18 +865,24 @@ 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.
 	 */
-	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;
+	}
+
+	/* Synchronize the skb tail with XDP's updated linear area. */
+	off = xdp->data_end - orig_data_end;
+	if (off != 0) {
+		skb_set_tail_pointer(skb, xdp->data_end - xdp->data);
+		skb->len += off; /* positive on grow, negative on shrink */
+	}
 
 	skb->protocol = eth_type_trans(skb, rq->dev);
 

2.43.0