Re: [PATCH net v2] xsk: set network header on skb at generic transmit

[netdev-bot+sashiko@kernel.org]

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