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

[sashiko-bot@kernel.org]

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