[PATCH net v3 0/2] fix skb length accounting after XDP frag adjustment

[PATCH net v3 0/2] fix skb length accounting after XDP frag adjustment

[Sun Jian]

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/20260731032357.6114-1-sun.jian.kdev@gmail.com/

v1:
https://lore.kernel.org/bpf/20260727032535.13469-1-sun.jian.kdev@gmail.com/

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.

Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/

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

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

[Sun Jian]

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/bpf/al9T9Eto%2FhRIzP5W@boxer/
Reviewed-by: Mohsin Bashir <hmohsin@meta.com>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 net/core/dev.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 5933c5dab09e..5c37cf6c4aa1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Preexisting bug: bpf_xdp_adjust_tail for generic XDP incorrectly frees system_page_pool fragment pages using the driver's RX queue memory model.
- [High] The exact length accounting vulnerability fixed in `bpf_prog_run_generic_xdp` remains unpatched in the native XDP receive path for veth devices (`veth_xdp_rcv_skb`).
--

--- Patch [1]: [PATCH net v3 1/2] net: fix skb length accounting after generic XDP frag adjustment ---
commit e03da2f78f026fb5c5139f386cdfe9ac7e62b48f
Author: Sun Jian <sun.jian.kdev@gmail.com>
Subject: net: fix skb length accounting after generic XDP frag adjustment

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.

This patch subtracts the old data_len before replacing it and adds the new
data_len afterwards, keeping skb->len and skb->data_len synchronized.

Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5933c5dab09ee..5c37cf6c4aa10 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -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

[Severity: High]
This is a pre-existing issue, but since this code path relates to fragment
shrinking via bpf_xdp_adjust_tail(), does it incorrectly free system_page_pool
fragment pages using the driver's RX queue memory model?

In generic XDP, skb fragments are reallocated using system_page_pool:

net/core/dev.c:netif_skb_check_for_xdp() {
    ...
    err = skb_cow_data_for_xdp(this_cpu_read(system_page_pool.pool), pskb, prog);
    ...
}

However, bpf_prog_run_generic_xdp() initializes the xdp_buff with the device
driver's rxqueue:

net/core/dev.c:bpf_prog_run_generic_xdp() {
    ...
    xdp_init_buff(xdp, frame_sz, &rxqueue->xdp_rxq);
    ...
}

When an XDP program shrinks fragments, bpf_xdp_shrink_data() retrieves the
driver's mem_type and passes it to __xdp_return():

net/core/filter.c:bpf_xdp_shrink_data() {
    ...
    enum xdp_mem_type mem_type = xdp->rxq->mem.type;
    ...
    if (release) {
        __xdp_return(netmem, mem_type, false, zc_frag);
    ...
}

If mem_type is MEM_TYPE_PAGE_SHARED, __xdp_return() routes to page_frag_free(),
bypassing the required page_pool_put_full_netmem() hooks for system_page_pool
pages. Can this leak DMA mappings and corrupt page structures when these
pages reach the buddy allocator?

> -	 * (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;

[Severity: High]
This isn't a bug introduced by this patch, but does the exact length
accounting vulnerability fixed here remain unpatched in the native XDP
receive path for veth devices?

Looking at veth_xdp_rcv_skb() in drivers/net/veth.c:

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

If an XDP program shrinks an skb's fragment area, skb->data_len is updated
but skb->len is not adjusted, which would artificially inflate skb_headlen()
and potentially leak adjacent kernel memory to userspace.

Additionally, if the fragments are completely dropped, xdp->data_end shrinks,
resulting in a negative off passed to __skb_put(). Since skb->data_len is
still non-zero during the __skb_put() call, would SKB_LINEAR_ASSERT() trigger
a BUG_ON and cause a kernel panic?

> +	} else {
>  		skb->data_len = 0;
> +	}
>  
>  	/* check if XDP changed eth hdr such SKB needs update */
>  	eth = (struct ethhdr *)xdp->data;

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

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

[Sun Jian]

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.

Fixes: 718a18a0c8a6 ("veth: Rework veth_xdp_rcv_skb in order to accept non-linear skb")
Cc: stable@vger.kernel.org
Reported-by: Mohsin Bashir <mohsin.bashr@gmail.com>
Link: https://lore.kernel.org/bpf/80687d9c-9c27-494c-b3f2-efd0230b1895@gmail.com/
Suggested-by: Lorenzo Bianconi <lorenzo@kernel.org>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/net/veth.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 00e34afd858e..f80f6584a6ff 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling __skb_pull() before updating skb->len and skb->data_len triggers a BUG_ON() panic if the XDP program expands the linear area and shrinks the head by a large amount.
--

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

veth: fix skb length accounting after XDP frag adjustment

This commit synchronizes skb->len and skb->data_len after an XDP program
adjusts the fragment area in veth_xdp_rcv_skb(). It also uses
skb_set_tail_pointer() instead of __skb_put() to handle non-linear skbs
safely.

Link: https://lore.kernel.org/bpf/80687d9c-9c27-494c-b3f2-efd0230b1895@gmail.com/

> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 00e34afd858e7..f80f6584a6ff9 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -865,18 +865,24 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
>  
>  	skb_reset_mac_header(skb);

[Severity: High]
This is a pre-existing issue, but does calling __skb_pull() just above this
line risk triggering a BUG() panic before the skb length synchronization logic
executes?

If an XDP program calls bpf_xdp_pull_data() to pull bytes into the linear
area (increasing xdp->data_end), and then calls bpf_xdp_adjust_head() to
advance the head by a large amount within the newly expanded linear area,
veth_xdp_rcv_skb() will call __skb_pull(skb, -off).

Because this executes before the new skb->len and skb->data_len
synchronization logic below, __skb_pull() subtracts the large adjustment
from the original skb->len. 

If this causes skb->len to drop below the original skb->data_len, wouldn't
this trigger the BUG() assertion in __skb_pull()?

> -	/* 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;

[ ... ]

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