BPF List
 help / color / mirror / Atom feed
* [PATCH net 0/2] xdp: fix skb length accounting after frag adjustment
@ 2026-07-27  3:25 Sun Jian
  2026-07-27  3:25 ` [PATCH net 1/2] net: fix skb length accounting after generic XDP " Sun Jian
  2026-07-27  3:25 ` [PATCH net 2/2] veth: fix skb length accounting after " Sun Jian
  0 siblings, 2 replies; 8+ messages in thread
From: Sun Jian @ 2026-07-27  3:25 UTC (permalink / raw)
  To: netdev
  Cc: bpf, stable, sun.jian.kdev, davem, edumazet, kuba, pabeni,
	andrew+netdev, horms, ast, daniel, hawk, john.fastabend, sdf,
	lorenzo, toke, maciej.fijalkowski, matt

Both generic XDP and veth restore skb fragment accounting after running
an XDP program by copying xdp_frags_size into skb->data_len. skb->len is
only adjusted by the linear tail delta, so the stale fragment
contribution is left in place. When an XDP program shrinks only the
fragment area, skb_headlen() therefore exceeds 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.

Maciej suggested assigning xdp_get_buff_len(xdp) to skb->len. That works
for veth, where the skb and XDP views both include the MAC header at this
point, but not for generic XDP. bpf_prog_run_generic_xdp() builds the XDP
view with skb_headlen(skb) + mac_len while the skb has already been
pulled past the MAC header, so that assignment would overcount skb->len
by mac_len.

Both patches instead replace the old data_len contribution in skb->len
with the updated one. This is independent of the current packet view and
keeps the accounting sequence identical at both sites.

Tested with a 60000-byte UDP datagram over a veth pair with MTU 64000. An
XDP program shortened the fragment area by 1024 bytes. Before the fixes,
both generic and native XDP produced corrupted payloads in 10/10 runs.
After the fixes, both paths matched the expected payload exactly in 10/10
runs.

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 | 4 +++-
 net/core/dev.c     | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)


base-commit: 53658c6f3682967a5e76ed4bc7462c4bdcddaec3
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net 1/2] net: fix skb length accounting after generic XDP frag adjustment
  2026-07-27  3:25 [PATCH net 0/2] xdp: fix skb length accounting after frag adjustment Sun Jian
@ 2026-07-27  3:25 ` Sun Jian
  2026-07-27 19:06   ` Maciej Fijalkowski
  2026-07-28  3:26   ` sashiko-bot
  2026-07-27  3:25 ` [PATCH net 2/2] veth: fix skb length accounting after " Sun Jian
  1 sibling, 2 replies; 8+ messages in thread
From: Sun Jian @ 2026-07-27  3:25 UTC (permalink / raw)
  To: netdev
  Cc: bpf, stable, sun.jian.kdev, davem, edumazet, kuba, pabeni,
	andrew+netdev, horms, ast, daniel, hawk, john.fastabend, sdf,
	lorenzo, toke, maciej.fijalkowski, matt

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/r/20260720141859.19FF41F000E9@smtp.kernel.org
Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 net/core/dev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 5933c5dab09e..94204702cb02 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5517,12 +5517,14 @@ 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), 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;
 
 	/* check if XDP changed eth hdr such SKB needs update */
 	eth = (struct ethhdr *)xdp->data;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net 2/2] veth: fix skb length accounting after XDP frag adjustment
  2026-07-27  3:25 [PATCH net 0/2] xdp: fix skb length accounting after frag adjustment Sun Jian
  2026-07-27  3:25 ` [PATCH net 1/2] net: fix skb length accounting after generic XDP " Sun Jian
@ 2026-07-27  3:25 ` Sun Jian
  2026-07-27  6:54   ` Lorenzo Bianconi
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Sun Jian @ 2026-07-27  3:25 UTC (permalink / raw)
  To: netdev
  Cc: bpf, stable, sun.jian.kdev, davem, edumazet, kuba, pabeni,
	andrew+netdev, horms, ast, daniel, hawk, john.fastabend, sdf,
	lorenzo, toke, maciej.fijalkowski, matt

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.

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: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20260720141859.19FF41F000E9@smtp.kernel.org
Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/net/veth.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 00e34afd858e..a956498a073b 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 */
 
 	/* 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);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net 2/2] veth: fix skb length accounting after XDP frag adjustment
  2026-07-27  3:25 ` [PATCH net 2/2] veth: fix skb length accounting after " Sun Jian
@ 2026-07-27  6:54   ` Lorenzo Bianconi
  2026-07-27  7:28   ` Lorenzo Bianconi
  2026-07-28  3:26   ` sashiko-bot
  2 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-07-27  6:54 UTC (permalink / raw)
  To: Sun Jian
  Cc: netdev, bpf, stable, davem, edumazet, kuba, pabeni, andrew+netdev,
	horms, ast, daniel, hawk, john.fastabend, sdf, toke,
	maciej.fijalkowski, matt

[-- Attachment #1: Type: text/plain, Size: 2422 bytes --]

On Jul 26, 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.
> 
> 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: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/20260720141859.19FF41F000E9@smtp.kernel.org
> Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  drivers/net/veth.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 00e34afd858e..a956498a073b 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 */
>  
>  	/* 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;

nit: I guess you can move this one just in the if () branch.

Regards,
Lorenzo

>  
>  	skb->protocol = eth_type_trans(skb, rq->dev);
>  
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net 2/2] veth: fix skb length accounting after XDP frag adjustment
  2026-07-27  3:25 ` [PATCH net 2/2] veth: fix skb length accounting after " Sun Jian
  2026-07-27  6:54   ` Lorenzo Bianconi
@ 2026-07-27  7:28   ` Lorenzo Bianconi
  2026-07-28  3:26   ` sashiko-bot
  2 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-07-27  7:28 UTC (permalink / raw)
  To: Sun Jian
  Cc: netdev, bpf, stable, davem, edumazet, kuba, pabeni, andrew+netdev,
	horms, ast, daniel, hawk, john.fastabend, sdf, toke,
	maciej.fijalkowski, matt

[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]

On Jul 26, 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.
> 
> 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: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/20260720141859.19FF41F000E9@smtp.kernel.org
> Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  drivers/net/veth.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 00e34afd858e..a956498a073b 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 */
>  
>  	/* 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;

nit: same here, you can this one just in the if () branch.

Regards,
Lorenzo

>  
>  	skb->protocol = eth_type_trans(skb, rq->dev);
>  
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net 1/2] net: fix skb length accounting after generic XDP frag adjustment
  2026-07-27  3:25 ` [PATCH net 1/2] net: fix skb length accounting after generic XDP " Sun Jian
@ 2026-07-27 19:06   ` Maciej Fijalkowski
  2026-07-28  3:26   ` sashiko-bot
  1 sibling, 0 replies; 8+ messages in thread
From: Maciej Fijalkowski @ 2026-07-27 19:06 UTC (permalink / raw)
  To: Sun Jian
  Cc: netdev, bpf, stable, davem, edumazet, kuba, pabeni, andrew+netdev,
	horms, ast, daniel, hawk, john.fastabend, sdf, lorenzo, toke,
	matt

On Sun, Jul 26, 2026 at 08:25:34PM -0700, Sun Jian wrote:
> 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/r/20260720141859.19FF41F000E9@smtp.kernel.org
> Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  net/core/dev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5933c5dab09e..94204702cb02 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5517,12 +5517,14 @@ 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), 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;

hm that reads a bit weird to me. maybe we could follow the existing
semantics and reuse @off ? that would be a nicer way to update ::len only
by a delta.

if we would stick with existing approach i would appreciate comment being
more elaborative, where we could explain that it is simpler to wipe out
data_len altogether from len and then just add it back.

>  
>  	/* check if XDP changed eth hdr such SKB needs update */
>  	eth = (struct ethhdr *)xdp->data;
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net 1/2] net: fix skb length accounting after generic XDP frag adjustment
  2026-07-27  3:25 ` [PATCH net 1/2] net: fix skb length accounting after generic XDP " Sun Jian
  2026-07-27 19:06   ` Maciej Fijalkowski
@ 2026-07-28  3:26   ` sashiko-bot
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-28  3:26 UTC (permalink / raw)
  To: Sun Jian; +Cc: bpf

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

Pre-existing issues:
- [High] A generic XDP program can trigger a kernel panic via BUG_ON() in __skb_pull() by dropping all fragments and then manipulating head and tail lengths.
--

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

net: fix skb length accounting after generic XDP frag adjustment

This commit fixes skb length accounting after generic XDP fragment
adjustments by keeping skb->len and skb->data_len synchronized. It
subtracts the old data_len before replacing it and adds the new
data_len afterwards.

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

> diff --git a/net/core/dev.c b/net/core/dev.c
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
[ ... ]
> @@ -5497,12 +5517,14 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
>  
>  	/* check if bpf_xdp_adjust_head was used */
>  	off = xdp->data - orig_data;
>  	if (off) {
>  		if (off > 0)
>  			__skb_pull(skb, off);

[Severity: High]
This is a pre-existing issue, but can this sequence trigger a kernel panic
via BUG_ON() in __skb_pull()?

If an XDP program drops all fragments via bpf_xdp_adjust_tail() with a
negative offset, grows the linear tailroom, and shrinks the linear head by
consuming the newly added space, it seems __skb_pull() is called while
skb->data_len still holds the original large fragment size.

Since the fragment adjustments to skb->data_len are synchronized further down
in this function, could skb->len drop below skb->data_len during this pull
and trigger the BUG() inside __skb_pull()?

>  		else if (off < 0)
>  			__skb_push(skb, -off);
>  
>  		skb->mac_header += off;
>  		skb_reset_network_header(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.
> +	 * (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;
>  
>  	/* check if XDP changed eth hdr such SKB needs update */
>  	eth = (struct ethhdr *)xdp->data;

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net 2/2] veth: fix skb length accounting after XDP frag adjustment
  2026-07-27  3:25 ` [PATCH net 2/2] veth: fix skb length accounting after " Sun Jian
  2026-07-27  6:54   ` Lorenzo Bianconi
  2026-07-27  7:28   ` Lorenzo Bianconi
@ 2026-07-28  3:26   ` sashiko-bot
  2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-28  3:26 UTC (permalink / raw)
  To: Sun Jian; +Cc: bpf

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-07-28  3:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27  3:25 [PATCH net 0/2] xdp: fix skb length accounting after frag adjustment Sun Jian
2026-07-27  3:25 ` [PATCH net 1/2] net: fix skb length accounting after generic XDP " Sun Jian
2026-07-27 19:06   ` Maciej Fijalkowski
2026-07-28  3:26   ` sashiko-bot
2026-07-27  3:25 ` [PATCH net 2/2] veth: fix skb length accounting after " Sun Jian
2026-07-27  6:54   ` Lorenzo Bianconi
2026-07-27  7:28   ` Lorenzo Bianconi
2026-07-28  3:26   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox