BPF List
 help / color / mirror / Atom feed
* [PATCH net v2] xsk: set network header on skb at generic transmit
@ 2026-09-01  8:33 Jason Xing
  2026-09-02  8:33 ` sashiko-bot
  2026-09-03 23:34 ` netdev-bot+sashiko
  0 siblings, 2 replies; 6+ messages in thread
From: Jason Xing @ 2026-09-01  8:33 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
	john.fastabend, aleksander.lobakin
  Cc: bpf, netdev, Jason Xing, syzbot+f16c1b285c9f63994eec

When constructing an skb in the XSK generic transmit path, neither
xsk_build_skb() nor xsk_build_skb_zerocopy() calls
skb_reset_network_header(). After __finalize_skb_around(), the
network_header offset defaults to 0 (pointing to skb->head). The
subsequent skb_reserve(skb, hr) moves skb->data forward by hr bytes,
but leaves network_header still at 0. This makes skb_network_offset()
return a negative value (-hr).

When such an skb reaches a tunnel driver like ipgre_xmit(), the call
chain pskb_inet_may_pull() -> pskb_network_may_pull_reason() computes:

    pskb_may_pull_reason(skb, skb_network_offset(skb) + nhlen)

The negative int from skb_network_offset() is implicitly promoted to
unsigned int, wrapping around to a huge value. This triggers the
DEBUG_NET_WARN_ON_ONCE(len > INT_MAX) in pskb_may_pull_reason().

Call trace from syzkaller:
  ipgre_xmit+0x958/0xcd0 net/ipv4/ip_gre.c:658
  __netdev_start_xmit include/linux/netdevice.h:5400
  netdev_start_xmit include/linux/netdevice.h:5409
  __dev_direct_xmit+0x4b6/0x730 net/core/dev.c:4942
  __xsk_generic_xmit net/xdp/xsk.c:1079
  xsk_generic_xmit+0x277a/0x40a0 net/xdp/xsk.c:1120

Reset the network header so skb_network_offset() is valid (0). Note this
anchors it at the start of the frame (L2 for an ethernet device), which
is the same convention AF_PACKET uses on TX. Unlike AF_PACKET, AF_XDP
does not need to locate the real L2/L3 boundary or probe the transport
header: an AF_XDP frame is always a complete, opaque L2 blob from
userspace. AF_PACKET only cares about the boundary because it can cook an
L2 header itself (SOCK_DGRAM) and supports GSO/checksum offload; none of
that applies here, where the frame is sent as-is via __dev_direct_xmit().
So resetting the network header is enough to restore the invariant and
avoid the crash.

Fixes: 3914d88f7608 ("xsk: Respect device's headroom and tailroom on generic xmit path")
Reported-by: syzbot+f16c1b285c9f63994eec@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f16c1b285c9f63994eec
Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
---
V2
Link: https://lore.kernel.org/all/20260826012052.51028-1-kerneljasonxing@gmail.com/
1. add more clarification about why we don't probe the header like packet at
the end of the commit message (Jakub)
---
 net/xdp/xsk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 33475b180ea6..2077bb6299ca 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -947,6 +947,8 @@ static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
 {
 	int err;
 
+	skb_reset_network_header(skb);
+
 	err = xsk_skb_destructor_set_addr(skb, addr);
 	if (unlikely(err))
 		return err;
-- 
2.43.5


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

* Re: [PATCH net v2] xsk: set network header on skb at generic transmit
  2026-09-01  8:33 [PATCH net v2] xsk: set network header on skb at generic transmit Jason Xing
@ 2026-09-02  8:33 ` sashiko-bot
  2026-09-02 11:55   ` Jason Xing
  2026-09-03 23:34 ` netdev-bot+sashiko
  1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-09-02  8:33 UTC (permalink / raw)
  To: Jason Xing; +Cc: bpf

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

New issues:
- [Medium] The patch incorrectly anchors the network header at the L2 boundary (MAC header), which violates standard SKB semantics and contradicts the AF_PACKET convention it claims to follow.

Pre-existing issues:
- [High] Missing initialization of `skb->mac_header` in the AF_XDP generic transmit path leaves it at `~0U` (0xFFFF), causing out-of-bounds memory accesses in network drivers that unconditionally read the MAC header.
--

commit 8222314e7d9b572394dd3fc2b9acb0b3706abd3e
Author: Jason Xing <kerneljasonxing@gmail.com>

xsk: set network header on skb at generic transmit

This commit fixes an issue in the XSK generic transmit path where the network
header was left uninitialized. It adds a call to skb_reset_network_header(skb)
in xsk_skb_init_misc() to ensure the network header is correctly anchored to
the start of the frame. This prevents a crash when the skb is handled by
tunnel drivers.

Closes: https://syzkaller.appspot.com/bug?extid=f16c1b285c9f63994eec
Link: https://lore.kernel.org/all/20260826012052.51028-1-kerneljasonxing@gmail.com/

>     Reset the network header so skb_network_offset() is valid (0). Note this
>     anchors it at the start of the frame (L2 for an ethernet device), which
>     is the same convention AF_PACKET uses on TX.

[Severity: Medium]
Does anchoring the network header at the L2 boundary (MAC header) here
violate SKB semantics and contradict the AF_PACKET convention?

In AF_PACKET's packet_snd(), skb_reset_network_header(skb) is explicitly
called before skb_reserve(skb, -reserve) prepends the L2 header, correctly
anchoring the network header at the L3 boundary (after hard_header_len bytes).

By anchoring the network header directly at skb->data (the L2 header) in
xsk_skb_init_misc(), won't any subsystem, offload logic, or tunneling driver
that relies on skb_network_header(skb) erroneously parse the Ethernet header
as an IP header?

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 33475b180ea6f..2077bb6299ca9 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -947,6 +947,8 @@ static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
>  {
>  	int err;
>  
> +	skb_reset_network_header(skb);
> +

[Severity: High]
This isn't a bug introduced by this patch, but is it also necessary to call
skb_reset_mac_header(skb) here?

The AF_XDP generic TX path allocates packets via __alloc_skb(), which leaves
mac_header uninitialized at ~0U (0xFFFF). Because AF_XDP generic TX bypasses
standard egress paths (like __dev_queue_xmit()) that would normally set the
MAC header, the SKB is handed to __dev_direct_xmit() with an invalid
mac_header.

When physical NIC drivers unconditionally read eth->h_proto via
skb_mac_header(skb) during transmit, this evaluates to skb->head + 0xFFFF.

For example, in drivers/net/ethernet/intel/ice/ice_txrx.c:ice_xmit_frame_ring():

	eth = (struct ethhdr *)skb_mac_header(skb);

	if ((ice_is_switchdev_running(vsi->back) ||
	     ice_lag_is_switchdev_running(vsi->back)) &&
	    vsi->type != ICE_VSI_SF)
		ice_eswitch_set_target_vsi(skb, &offload);
	else if (unlikely((skb->priority == TC_PRIO_CONTROL ||
			   eth->h_proto == htons(ETH_P_LLDP)) &&
			   vsi->type == ICE_VSI_PF &&
			   vsi->port_info->qos_cfg.is_sw_lldp))

Dereferencing this out-of-bounds pointer crosses page boundaries and causes
a paging violation (kernel panic). Should skb_reset_mac_header(skb) be added
alongside the network header reset to prevent this out-of-bounds read?

>  	err = xsk_skb_destructor_set_addr(skb, addr);
>  	if (unlikely(err))
>  		return err;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901083325.3445-1-kerneljasonxing@gmail.com?part=1

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

* Re: [PATCH net v2] xsk: set network header on skb at generic transmit
  2026-09-02  8:33 ` sashiko-bot
@ 2026-09-02 11:55   ` Jason Xing
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Xing @ 2026-09-02 11:55 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: bpf

On Wed, Sep 2, 2026 at 4:33 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Medium] The patch incorrectly anchors the network header at the L2 boundary (MAC header), which violates standard SKB semantics and contradicts the AF_PACKET convention it claims to follow.
>
> Pre-existing issues:
> - [High] Missing initialization of `skb->mac_header` in the AF_XDP generic transmit path leaves it at `~0U` (0xFFFF), causing out-of-bounds memory accesses in network drivers that unconditionally read the MAC header.
> --
>
> commit 8222314e7d9b572394dd3fc2b9acb0b3706abd3e
> Author: Jason Xing <kerneljasonxing@gmail.com>
>
> xsk: set network header on skb at generic transmit
>
> This commit fixes an issue in the XSK generic transmit path where the network
> header was left uninitialized. It adds a call to skb_reset_network_header(skb)
> in xsk_skb_init_misc() to ensure the network header is correctly anchored to
> the start of the frame. This prevents a crash when the skb is handled by
> tunnel drivers.
>
> Closes: https://syzkaller.appspot.com/bug?extid=f16c1b285c9f63994eec
> Link: https://lore.kernel.org/all/20260826012052.51028-1-kerneljasonxing@gmail.com/
>
> >     Reset the network header so skb_network_offset() is valid (0). Note this
> >     anchors it at the start of the frame (L2 for an ethernet device), which
> >     is the same convention AF_PACKET uses on TX.
>
> [Severity: Medium]
> Does anchoring the network header at the L2 boundary (MAC header) here
> violate SKB semantics and contradict the AF_PACKET convention?
>
> In AF_PACKET's packet_snd(), skb_reset_network_header(skb) is explicitly
> called before skb_reserve(skb, -reserve) prepends the L2 header, correctly
> anchoring the network header at the L3 boundary (after hard_header_len bytes).

I will remove this in V3. I misunderstood the boundary in AF_PACKET logic.

>
> By anchoring the network header directly at skb->data (the L2 header) in
> xsk_skb_init_misc(), won't any subsystem, offload logic, or tunneling driver
> that relies on skb_network_header(skb) erroneously parse the Ethernet header
> as an IP header?

No, at least until now. That's why resetting is sufficient.

>
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 33475b180ea6f..2077bb6299ca9 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -947,6 +947,8 @@ static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
> >  {
> >       int err;
> >
> > +     skb_reset_network_header(skb);
> > +
>
> [Severity: High]
> This isn't a bug introduced by this patch, but is it also necessary to call
> skb_reset_mac_header(skb) here?

I think it should be merged as a fix:
https://lore.kernel.org/all/20260817153957.3177627-1-poros@redhat.com/

Thanks,
Jason

>
> The AF_XDP generic TX path allocates packets via __alloc_skb(), which leaves
> mac_header uninitialized at ~0U (0xFFFF). Because AF_XDP generic TX bypasses
> standard egress paths (like __dev_queue_xmit()) that would normally set the
> MAC header, the SKB is handed to __dev_direct_xmit() with an invalid
> mac_header.
>
> When physical NIC drivers unconditionally read eth->h_proto via
> skb_mac_header(skb) during transmit, this evaluates to skb->head + 0xFFFF.
>
> For example, in drivers/net/ethernet/intel/ice/ice_txrx.c:ice_xmit_frame_ring():
>
>         eth = (struct ethhdr *)skb_mac_header(skb);
>
>         if ((ice_is_switchdev_running(vsi->back) ||
>              ice_lag_is_switchdev_running(vsi->back)) &&
>             vsi->type != ICE_VSI_SF)
>                 ice_eswitch_set_target_vsi(skb, &offload);
>         else if (unlikely((skb->priority == TC_PRIO_CONTROL ||
>                            eth->h_proto == htons(ETH_P_LLDP)) &&
>                            vsi->type == ICE_VSI_PF &&
>                            vsi->port_info->qos_cfg.is_sw_lldp))
>
> Dereferencing this out-of-bounds pointer crosses page boundaries and causes
> a paging violation (kernel panic). Should skb_reset_mac_header(skb) be added
> alongside the network header reset to prevent this out-of-bounds read?
>
> >       err = xsk_skb_destructor_set_addr(skb, addr);
> >       if (unlikely(err))
> >               return err;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260901083325.3445-1-kerneljasonxing@gmail.com?part=1

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

* Re: [PATCH net v2] xsk: set network header on skb at generic transmit
  2026-09-01  8:33 [PATCH net v2] xsk: set network header on skb at generic transmit Jason Xing
  2026-09-02  8:33 ` sashiko-bot
@ 2026-09-03 23:34 ` netdev-bot+sashiko
  2026-09-04  3:53   ` Jason Xing
  1 sibling, 1 reply; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-09-03 23:34 UTC (permalink / raw)
  To: kerneljasonxing
  Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
	john.fastabend, aleksander.lobakin, bpf, netdev,
	syzbot+f16c1b285c9f63994eec

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] net/xdp/xsk.c: the new skb_reset_network_header(skb) in
  xsk_skb_init_misc() (net/xdp/xsk.c:950) anchors the *network* header…

--

xsk: set network header on skb at generic transmit

The XSK generic transmit path never reset the network header, so after
skb_reserve(hr) the stored offset was left behind skb->data and
skb_network_offset() returned -hr, which tripped the
DEBUG_NET_WARN_ON_ONCE(len > INT_MAX) in pskb_may_pull_reason() when the
skb reached a tunnel device. This adds skb_reset_network_header() in
xsk_skb_init_misc() so the offset is 0.

Closes: https://syzkaller.appspot.com/bug?extid=f16c1b285c9f63994eec
Link: https://lore.kernel.org/all/20260826012052.51028-1-kerneljasonxing@gmail.com/

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 33475b180ea6f..2077bb6299ca9 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -947,6 +947,8 @@ static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
>  {
>  	int err;
>  
> +	skb_reset_network_header(skb);
> +

[Severity: Medium]
The negative offset does look closed by this: xsk_skb_init_misc() runs
after skb_reserve(hr) in xsk_build_skb() and xsk_build_skb_zerocopy(), so
skb_network_offset() becomes 0. Two parts of the reasoning look off
though.

First, the commit message says:

  "Note this anchors it at the start of the frame (L2 for an ethernet
   device), which is the same convention AF_PACKET uses on TX."

Is that what AF_PACKET does? packet_parse_headers() anchors the MAC
header, not the network header:

net/packet/af_packet.c:packet_parse_headers() {
	/* On TX skb->data is the L2 header; anchor it for all socket types. */
	skb_reset_mac_header(skb);

	if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
	    sock->type == SOCK_RAW)
		skb->protocol = dev_parse_header_protocol(skb);
	...
}

and packet_snd() for SOCK_RAW resets network_header before moving
skb->data back over the hard header, so network_header stays at the L3
boundary:

	skb_reset_network_header(skb);
	...
	} else if (reserve) {
		skb_reserve(skb, -reserve);

So after this patch an AF_XDP skb differs from an AF_PACKET one by
hard_header_len in network_header, and mac_header is still at the unset
sentinel left by __finalize_skb_around(), i.e. skb_mac_header_was_set()
is false.

Second, the commit message says AF_PACKET "supports GSO/checksum
offload; none of that applies here". Does checksum offload really not
apply? xsk_skb_metadata() in the same file does:

net/xdp/xsk.c:xsk_skb_metadata() {
		skb->csum_start = hr + csum_start;
		skb->csum_offset = csum_offset;
		skb->ip_summed = CHECKSUM_PARTIAL;

		if (unlikely(pool->tx_sw_csum)) {
			...
			err = skb_checksum_help(skb);
	...
}

The software fallback only happens when the user opted in via
XDP_UMEM_TX_SW_CSUM, so a CHECKSUM_PARTIAL AF_XDP skb does reach the
driver through __dev_direct_xmit(), and drivers take the L2/L3 split
from network_header:

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:ixgbe_tx_csum() {
	vlan_macip_lens = skb_checksum_start_offset(skb) -
			  skb_network_offset(skb);
no_csum:
	/* vlan_macip_lens: MACLEN, VLAN tag */
	vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT;
	...
}

With skb_network_offset() now 0 on an ethernet frame, does this program
MACLEN == 0 and an IPLEN that swallows the ethernet header, so the NIC
inserts a checksum derived from the wrong header layout?

Would it make sense to also call skb_reset_mac_header() and place
network_header at the real L3 boundary here? Alternatively, if the
minimal reset is the intent, could the commit message describe
network_header as a placeholder and drop the claims about the AF_PACKET
convention and the absence of checksum offload?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260901083325.3445-1-kerneljasonxing%40gmail.com

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

* Re: [PATCH net v2] xsk: set network header on skb at generic transmit
  2026-09-03 23:34 ` netdev-bot+sashiko
@ 2026-09-04  3:53   ` Jason Xing
  2026-09-04  4:10     ` Jason Xing
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Xing @ 2026-09-04  3:53 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
	john.fastabend, aleksander.lobakin, bpf, netdev,
	syzbot+f16c1b285c9f63994eec

On Fri, Sep 4, 2026 at 7:34 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> - [Medium] net/xdp/xsk.c: the new skb_reset_network_header(skb) in
>   xsk_skb_init_misc() (net/xdp/xsk.c:950) anchors the *network* header…
>
> --
>
> xsk: set network header on skb at generic transmit
>
> The XSK generic transmit path never reset the network header, so after
> skb_reserve(hr) the stored offset was left behind skb->data and
> skb_network_offset() returned -hr, which tripped the
> DEBUG_NET_WARN_ON_ONCE(len > INT_MAX) in pskb_may_pull_reason() when the
> skb reached a tunnel device. This adds skb_reset_network_header() in
> xsk_skb_init_misc() so the offset is 0.
>
> Closes: https://syzkaller.appspot.com/bug?extid=f16c1b285c9f63994eec
> Link: https://lore.kernel.org/all/20260826012052.51028-1-kerneljasonxing@gmail.com/
>
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 33475b180ea6f..2077bb6299ca9 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -947,6 +947,8 @@ static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
> >  {
> >       int err;
> >
> > +     skb_reset_network_header(skb);
> > +
>
> [Severity: Medium]
> The negative offset does look closed by this: xsk_skb_init_misc() runs
> after skb_reserve(hr) in xsk_build_skb() and xsk_build_skb_zerocopy(), so
> skb_network_offset() becomes 0. Two parts of the reasoning look off
> though.
>
> First, the commit message says:
>
>   "Note this anchors it at the start of the frame (L2 for an ethernet
>    device), which is the same convention AF_PACKET uses on TX."
>
> Is that what AF_PACKET does? packet_parse_headers() anchors the MAC
> header, not the network header:
>
> net/packet/af_packet.c:packet_parse_headers() {
>         /* On TX skb->data is the L2 header; anchor it for all socket types. */
>         skb_reset_mac_header(skb);
>
>         if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
>             sock->type == SOCK_RAW)
>                 skb->protocol = dev_parse_header_protocol(skb);
>         ...
> }

I'm going to remove this part.

>
> and packet_snd() for SOCK_RAW resets network_header before moving
> skb->data back over the hard header, so network_header stays at the L3
> boundary:
>
>         skb_reset_network_header(skb);
>         ...
>         } else if (reserve) {
>                 skb_reserve(skb, -reserve);
>
> So after this patch an AF_XDP skb differs from an AF_PACKET one by
> hard_header_len in network_header, and mac_header is still at the unset
> sentinel left by __finalize_skb_around(), i.e. skb_mac_header_was_set()
> is false.
>
> Second, the commit message says AF_PACKET "supports GSO/checksum
> offload; none of that applies here". Does checksum offload really not
> apply? xsk_skb_metadata() in the same file does:
>
> net/xdp/xsk.c:xsk_skb_metadata() {
>                 skb->csum_start = hr + csum_start;
>                 skb->csum_offset = csum_offset;
>                 skb->ip_summed = CHECKSUM_PARTIAL;
>
>                 if (unlikely(pool->tx_sw_csum)) {
>                         ...
>                         err = skb_checksum_help(skb);
>         ...
> }
>
> The software fallback only happens when the user opted in via
> XDP_UMEM_TX_SW_CSUM, so a CHECKSUM_PARTIAL AF_XDP skb does reach the
> driver through __dev_direct_xmit(), and drivers take the L2/L3 split
> from network_header:
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:ixgbe_tx_csum() {
>         vlan_macip_lens = skb_checksum_start_offset(skb) -
>                           skb_network_offset(skb);
> no_csum:
>         /* vlan_macip_lens: MACLEN, VLAN tag */
>         vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT;
>         ...
> }

After digging into the probing logic, I strongly feel that Jakub is
right to consider whether to add the probing logic.

>
> With skb_network_offset() now 0 on an ethernet frame, does this program
> MACLEN == 0 and an IPLEN that swallows the ethernet header, so the NIC
> inserts a checksum derived from the wrong header layout?
>
> Would it make sense to also call skb_reset_mac_header() and place
> network_header at the real L3 boundary here? Alternatively, if the
> minimal reset is the intent, could the commit message describe

The minimal reset doesn't help with the checksum problem mentioned above.

Adding simple logic like resetting l2/l3 headers doesn't guarantee
that csum problem won't be triggered. Adding complete probing hurts
perfomance for sure.

Maybe we can cook two patches:
1. Fix the current bug syzbot reported as is.
2. Fix offload logic by adding complete probing

@Jakub, Maciej, Stan
Do we need a complex logic like af_packet to handle this?

Thanks,
Jason

> network_header as a placeholder and drop the claims about the AF_PACKET
> convention and the absence of checksum offload?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260901083325.3445-1-kerneljasonxing%40gmail.com

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

* Re: [PATCH net v2] xsk: set network header on skb at generic transmit
  2026-09-04  3:53   ` Jason Xing
@ 2026-09-04  4:10     ` Jason Xing
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Xing @ 2026-09-04  4:10 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
	john.fastabend, aleksander.lobakin, bpf, netdev,
	syzbot+f16c1b285c9f63994eec

On Fri, Sep 4, 2026 at 11:53 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> On Fri, Sep 4, 2026 at 7:34 AM <netdev-bot+sashiko@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider:
> >
> > Critical: 0 · High: 0 · Medium: 1 · Low: 0
> >
> > - [Medium] net/xdp/xsk.c: the new skb_reset_network_header(skb) in
> >   xsk_skb_init_misc() (net/xdp/xsk.c:950) anchors the *network* header…
> >
> > --
> >
> > xsk: set network header on skb at generic transmit
> >
> > The XSK generic transmit path never reset the network header, so after
> > skb_reserve(hr) the stored offset was left behind skb->data and
> > skb_network_offset() returned -hr, which tripped the
> > DEBUG_NET_WARN_ON_ONCE(len > INT_MAX) in pskb_may_pull_reason() when the
> > skb reached a tunnel device. This adds skb_reset_network_header() in
> > xsk_skb_init_misc() so the offset is 0.
> >
> > Closes: https://syzkaller.appspot.com/bug?extid=f16c1b285c9f63994eec
> > Link: https://lore.kernel.org/all/20260826012052.51028-1-kerneljasonxing@gmail.com/
> >
> > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > > index 33475b180ea6f..2077bb6299ca9 100644
> > > --- a/net/xdp/xsk.c
> > > +++ b/net/xdp/xsk.c
> > > @@ -947,6 +947,8 @@ static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
> > >  {
> > >       int err;
> > >
> > > +     skb_reset_network_header(skb);
> > > +
> >
> > [Severity: Medium]
> > The negative offset does look closed by this: xsk_skb_init_misc() runs
> > after skb_reserve(hr) in xsk_build_skb() and xsk_build_skb_zerocopy(), so
> > skb_network_offset() becomes 0. Two parts of the reasoning look off
> > though.
> >
> > First, the commit message says:
> >
> >   "Note this anchors it at the start of the frame (L2 for an ethernet
> >    device), which is the same convention AF_PACKET uses on TX."
> >
> > Is that what AF_PACKET does? packet_parse_headers() anchors the MAC
> > header, not the network header:
> >
> > net/packet/af_packet.c:packet_parse_headers() {
> >         /* On TX skb->data is the L2 header; anchor it for all socket types. */
> >         skb_reset_mac_header(skb);
> >
> >         if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
> >             sock->type == SOCK_RAW)
> >                 skb->protocol = dev_parse_header_protocol(skb);
> >         ...
> > }
>
> I'm going to remove this part.
>
> >
> > and packet_snd() for SOCK_RAW resets network_header before moving
> > skb->data back over the hard header, so network_header stays at the L3
> > boundary:
> >
> >         skb_reset_network_header(skb);
> >         ...
> >         } else if (reserve) {
> >                 skb_reserve(skb, -reserve);
> >
> > So after this patch an AF_XDP skb differs from an AF_PACKET one by
> > hard_header_len in network_header, and mac_header is still at the unset
> > sentinel left by __finalize_skb_around(), i.e. skb_mac_header_was_set()
> > is false.
> >
> > Second, the commit message says AF_PACKET "supports GSO/checksum
> > offload; none of that applies here". Does checksum offload really not
> > apply? xsk_skb_metadata() in the same file does:
> >
> > net/xdp/xsk.c:xsk_skb_metadata() {
> >                 skb->csum_start = hr + csum_start;
> >                 skb->csum_offset = csum_offset;
> >                 skb->ip_summed = CHECKSUM_PARTIAL;
> >
> >                 if (unlikely(pool->tx_sw_csum)) {
> >                         ...
> >                         err = skb_checksum_help(skb);
> >         ...
> > }
> >
> > The software fallback only happens when the user opted in via
> > XDP_UMEM_TX_SW_CSUM, so a CHECKSUM_PARTIAL AF_XDP skb does reach the
> > driver through __dev_direct_xmit(), and drivers take the L2/L3 split
> > from network_header:
> >
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:ixgbe_tx_csum() {
> >         vlan_macip_lens = skb_checksum_start_offset(skb) -
> >                           skb_network_offset(skb);
> > no_csum:
> >         /* vlan_macip_lens: MACLEN, VLAN tag */
> >         vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT;
> >         ...
> > }
>
> After digging into the probing logic, I strongly feel that Jakub is
> right to consider whether to add the probing logic.
>
> >
> > With skb_network_offset() now 0 on an ethernet frame, does this program
> > MACLEN == 0 and an IPLEN that swallows the ethernet header, so the NIC
> > inserts a checksum derived from the wrong header layout?
> >
> > Would it make sense to also call skb_reset_mac_header() and place
> > network_header at the real L3 boundary here? Alternatively, if the
> > minimal reset is the intent, could the commit message describe
>
> The minimal reset doesn't help with the checksum problem mentioned above.
>
> Adding simple logic like resetting l2/l3 headers doesn't guarantee
> that csum problem won't be triggered. Adding complete probing hurts
> perfomance for sure.
>
> Maybe we can cook two patches:
> 1. Fix the current bug syzbot reported as is.
> 2. Fix offload logic by adding complete probing
>
> @Jakub, Maciej, Stan
> Do we need a complex logic like af_packet to handle this?

Until now, it seems we've got three fixes in the queue:
1. reset mac header:
https://lore.kernel.org/all/20260817153957.3177627-1-poros@redhat.com/
(commit 35fcde7f8deb "xsk: support for Tx")
2. reset network header like this patch. (commit 3914d88f7608 "xsk:
Respect device's headroom and tailroom")
3. handle csum offload issue by adding probing code. (commit
48eb03dd2630 "xsk: Add TX ... checksum offload support")

Thanks,
Jason

>
> Thanks,
> Jason
>
> > network_header as a placeholder and drop the claims about the AF_PACKET
> > convention and the absence of checksum offload?
> >
> > --
> > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260901083325.3445-1-kerneljasonxing%40gmail.com

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

end of thread, other threads:[~2026-09-04  4:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01  8:33 [PATCH net v2] xsk: set network header on skb at generic transmit Jason Xing
2026-09-02  8:33 ` sashiko-bot
2026-09-02 11:55   ` Jason Xing
2026-09-03 23:34 ` netdev-bot+sashiko
2026-09-04  3:53   ` Jason Xing
2026-09-04  4:10     ` Jason Xing

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