[PATCH net v3] net/packet: fix network header offset for non-VLAN raw packets on VLAN subinterfaces

[PATCH net v3] net/packet: fix network header offset for non-VLAN raw packets on VLAN subinterfaces

[Junnan Zhang]

AF_PACKET SOCK_RAW sets skb network_header to dev->hard_header_len in
packet_snd(). On VLAN subinterfaces with software tag insertion,
hard_header_len includes space for the VLAN tag (ETH_HLEN + VLAN_HLEN =
18) while min_header_len is the on-wire Ethernet header length
(ETH_HLEN = 14). A non-VLAN SOCK_RAW frame carries a standard 14-byte
Ethernet header, so its L3 header sits at min_header_len, not
hard_header_len.

packet_parse_headers() only corrects network_header for VLAN-tagged
frames. For non-VLAN frames it leaves network_header at hard_header_len,
so the IP header is found VLAN_HLEN bytes too late and
inet_gso_segment() fails with -EINVAL.

Observed on a virtio_net NIC (KVM guest) that advertises
NETIF_F_HW_VLAN_CTAG_FILTER but not NETIF_F_HW_VLAN_CTAG_TX, so VLAN
subinterfaces use software tag insertion (hard_header_len = 18). An
AF_PACKET SOCK_RAW socket bound to the VLAN subinterface with
PACKET_VNET_HDR enabled sends a large IPv4/TCP frame exceeding the path
MTU, with gso_type set in the virtio-net header. The user frame is a
plain [ethhdr][IP...] layout without a VLAN tag;
vlan_dev_hard_start_xmit() only attaches the tag metadata via
__vlan_hwaccel_put_tag(), and validate_xmit_vlan() on the physical
device later inserts the actual tag bytes via
__vlan_hwaccel_push_inside(). With network_header stuck at 18 while the
real IP header is at ETH_HLEN (14), inet_gso_segment() reads a
misaligned ip_hdr(skb) and returns -EINVAL.

For non-VLAN SOCK_RAW frames on VLAN subinterfaces, set network_header
to min_header_len so that the L3/L4 header positions match the actual
on-the-wire frame.

This fix is placed before skb_probe_transport_header() so that both the
transport header probe (which uses skb_network_offset() as nhoff) and
subsequent GSO see the right L3/L4 offsets. It complements
commit 01fdecc0480d ("net: packet: fix wrong transport_header when sending VLAN-tagged frame")
which only covers VLAN-tagged frames.

Fixes: dfed913e8b55 ("net/af_packet: add VLAN support for AF_PACKET SOCK_RAW GSO")
Signed-off-by: Junnan Zhang <zhangjn11@chinatelecom.cn>
Signed-off-by: Shouxin Sun <sunshx@chinatelecom.cn>
Signed-off-by: Junnan Zhang <zhangjn_dev@163.com>
---
v3:
- Use is_vlan_dev() instead of the min_header_len < hard_header_len
  heuristic to detect the affected device; the heuristic may have false
  positives for other protocols, and the ARPHRD_ETHER check excluded
  the other hard_header_len cases anyway, so scope the fix back to
  VLAN subinterfaces.
- Rename is_vlan to is_vlan_packet to disambiguate from is_vlan_dev().
- Correct where the tag is inserted: vlan_dev_hard_start_xmit() only
  attaches the tag metadata via __vlan_hwaccel_put_tag(); the tag
  bytes are inserted later by validate_xmit_vlan() on the physical
  device.
- Reword the subject and commit message back to the VLAN-subinterface
  scope.

v2: https://lore.kernel.org/all/20260831072034.40044-1-zhangjn_dev@163.com/
v1: https://lore.kernel.org/all/20260821085722.24036-1-zhangjn_dev@163.com/#t
---
 net/packet/af_packet.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 1168bd6b09cd..3bada38b3556 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1935,6 +1935,7 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
 static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
 {
 	int depth;
+	bool is_vlan_packet = false;
 
 	/* On TX skb->data is the L2 header; anchor it for all socket types. */
 	skb_reset_mac_header(skb);
@@ -1943,11 +1944,28 @@ static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
 	    sock->type == SOCK_RAW)
 		skb->protocol = dev_parse_header_protocol(skb);
 
+	if (likely(skb->dev->type == ARPHRD_ETHER)) {
+		is_vlan_packet = eth_type_vlan(skb->protocol);
+
+		/* For non-VLAN SOCK_RAW frames on VLAN subinterfaces with
+		 * software tag insertion, hard_header_len includes space
+		 * for the VLAN tag while min_header_len is the on-wire
+		 * Ethernet header length. The user frame carries a
+		 * standard Ethernet header, so its L3 sits at
+		 * min_header_len, not hard_header_len. Move
+		 * network_header to the actual L2/L3 boundary so the
+		 * transport header probe below and subsequent GSO see
+		 * the right L3.
+		 */
+		if (sock->type == SOCK_RAW && !is_vlan_packet &&
+		    is_vlan_dev(skb->dev))
+			skb_set_network_header(skb, skb->dev->min_header_len);
+	}
+
 	skb_probe_transport_header(skb);
 
 	/* Move network header to the right position for VLAN tagged packets */
-	if (likely(skb->dev->type == ARPHRD_ETHER) &&
-	    eth_type_vlan(skb->protocol) &&
+	if (is_vlan_packet &&
 	    vlan_get_protocol_and_depth(skb, skb->protocol, &depth) != 0)
 		skb_set_network_header(skb, depth);
 }
-- 
2.43.0

Re: [PATCH net v3] net/packet: fix network header offset for non-VLAN raw packets on VLAN subinterfaces

[Willem de Bruijn]

Junnan Zhang wrote:
> AF_PACKET SOCK_RAW sets skb network_header to dev->hard_header_len in
> packet_snd(). On VLAN subinterfaces with software tag insertion,
> hard_header_len includes space for the VLAN tag (ETH_HLEN + VLAN_HLEN =
> 18) while min_header_len is the on-wire Ethernet header length
> (ETH_HLEN = 14). A non-VLAN SOCK_RAW frame carries a standard 14-byte
> Ethernet header, so its L3 header sits at min_header_len, not
> hard_header_len.
> 
> packet_parse_headers() only corrects network_header for VLAN-tagged
> frames. For non-VLAN frames it leaves network_header at hard_header_len,
> so the IP header is found VLAN_HLEN bytes too late and
> inet_gso_segment() fails with -EINVAL.
> 
> Observed on a virtio_net NIC (KVM guest) that advertises
> NETIF_F_HW_VLAN_CTAG_FILTER but not NETIF_F_HW_VLAN_CTAG_TX, so VLAN
> subinterfaces use software tag insertion (hard_header_len = 18). An
> AF_PACKET SOCK_RAW socket bound to the VLAN subinterface with
> PACKET_VNET_HDR enabled sends a large IPv4/TCP frame exceeding the path
> MTU, with gso_type set in the virtio-net header. The user frame is a
> plain [ethhdr][IP...] layout without a VLAN tag;
> vlan_dev_hard_start_xmit() only attaches the tag metadata via
> __vlan_hwaccel_put_tag(), and validate_xmit_vlan() on the physical
> device later inserts the actual tag bytes via
> __vlan_hwaccel_push_inside(). With network_header stuck at 18 while the
> real IP header is at ETH_HLEN (14), inet_gso_segment() reads a
> misaligned ip_hdr(skb) and returns -EINVAL.
> 
> For non-VLAN SOCK_RAW frames on VLAN subinterfaces, set network_header
> to min_header_len so that the L3/L4 header positions match the actual
> on-the-wire frame.
> 
> This fix is placed before skb_probe_transport_header() so that both the
> transport header probe (which uses skb_network_offset() as nhoff) and
> subsequent GSO see the right L3/L4 offsets. It complements
> commit 01fdecc0480d ("net: packet: fix wrong transport_header when sending VLAN-tagged frame")
> which only covers VLAN-tagged frames.
> 
> Fixes: dfed913e8b55 ("net/af_packet: add VLAN support for AF_PACKET SOCK_RAW GSO")
> Signed-off-by: Junnan Zhang <zhangjn11@chinatelecom.cn>
> Signed-off-by: Shouxin Sun <sunshx@chinatelecom.cn>
> Signed-off-by: Junnan Zhang <zhangjn_dev@163.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [net,v3] net/packet: fix network header offset for non-VLAN raw packets on VLAN subinterfaces

[netdev-bot+sashiko]

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

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

- [High] The new branch added to packet_parse_headers()
  (net/packet/af_packet.c:1950-1966) is a no-op in exactly the…
- [Medium] The only configurations in which the new statement in
  packet_parse_headers() (net/packet/af_packet.c:1963-1965) changes any…

--

net/packet: fix network header offset for non-VLAN raw packets on VLAN subinterfaces

This adds a branch to packet_parse_headers() that, for untagged SOCK_RAW
frames sent on a VLAN subinterface, moves skb->network_header to
dev->min_header_len before skb_probe_transport_header() runs.  The stated
goal is to make GSO see the L3 header at ETH_HLEN instead of at
hard_header_len.  The existing VLAN-tagged correction below is reworked to
reuse the newly computed is_vlan_packet boolean.

> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 76bde7906d494..8dfb823e819d6 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1938,6 +1938,7 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
>  static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
>  {
>  	int depth;
> +	bool is_vlan_packet = false;
>  
>  	/* On TX skb->data is the L2 header; anchor it for all socket types. */
>  	skb_reset_mac_header(skb);
> @@ -1946,11 +1947,28 @@ static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
>  	    sock->type == SOCK_RAW)
>  		skb->protocol = dev_parse_header_protocol(skb);
>  
> +	if (likely(skb->dev->type == ARPHRD_ETHER)) {
> +		is_vlan_packet = eth_type_vlan(skb->protocol);
> +
> +		/* For non-VLAN SOCK_RAW frames on VLAN subinterfaces with
> +		 * software tag insertion, hard_header_len includes space
> +		 * for the VLAN tag while min_header_len is the on-wire
> +		 * Ethernet header length. The user frame carries a
> +		 * standard Ethernet header, so its L3 sits at
> +		 * min_header_len, not hard_header_len. Move
> +		 * network_header to the actual L2/L3 boundary so the
> +		 * transport header probe below and subsequent GSO see
> +		 * the right L3.
> +		 */
> +		if (sock->type == SOCK_RAW && !is_vlan_packet &&
> +		    is_vlan_dev(skb->dev))
> +			skb_set_network_header(skb, skb->dev->min_header_len);
> +	}

[Severity: High]
Is this new assignment a no-op in exactly the configuration the changelog
describes?

The commit message says:

  "On VLAN subinterfaces with software tag insertion, hard_header_len
   includes space for the VLAN tag (ETH_HLEN + VLAN_HLEN = 18) while
   min_header_len is the on-wire Ethernet header length (ETH_HLEN = 14)."

That does not appear to hold in this tree any more.  vlan_dev_init() copies
the real device's value verbatim and accounts the tag space in
needed_headroom instead:

net/8021q/vlan_dev.c:vlan_dev_init() {
	dev->needed_headroom = real_dev->needed_headroom + VLAN_HLEN;
	dev->needed_tailroom = real_dev->needed_tailroom;
	dev->header_ops      = &vlan_header_ops;
	dev->hard_header_len = real_dev->hard_header_len;
}

vlan_transfer_features() does the same, so toggling
NETIF_F_HW_VLAN_CTAG_TX no longer changes hard_header_len at runtime
either.  This came in with commit 447cbe95ebb953 ("vlan: fix
skb_under_panic and races when toggling HW VLAN offload"), which is already
an ancestor of the baseline, and whose changelog states "AF_PACKET SOCK_RAW
network header offsets remain correctly aligned at
real_dev->hard_header_len".

min_header_len is never touched by net/8021q; vlan_setup() goes through
ether_setup():

net/ethernet/eth.c:ether_setup() {
	dev->hard_header_len 	= ETH_HLEN;
	dev->min_header_len	= ETH_HLEN;
}

So for a VLAN device over an ordinary Ethernet NIC, including the reported
virtio_net guest, hard_header_len == min_header_len == 14, and packet_snd()
has already placed network_header at mac_header + 14:

net/packet/af_packet.c:packet_snd() {
	skb_reset_network_header(skb);
	...
	} else if (reserve) {
		skb_reserve(skb, -reserve);
		if (len < reserve + sizeof(struct ipv6hdr) &&
		    dev->min_header_len != hard_header_len)
			skb_reset_network_header(skb);
	}
}

tpacket_fill_skb() reaches the same offset via skb_reset_network_header()
followed by skb_push(skb, hard_header_len).

If hard_header_len is 14 there, does skb_set_network_header(skb,
min_header_len) write back the identical offset, leaving the reported
inet_gso_segment() -EINVAL unchanged?  If the failure is real, would it be
worth re-deriving it against 447cbe95ebb953 and keying the fix off the
frame's actual content rather than off device header-length metadata?

The in-code comment and the Fixes: tag also describe device geometry that
447cbe95ebb953 removed, and would propagate that description into stable
trees.

[Severity: Medium]
The only stacks where this statement changes a value are VLAN devices whose
real device has hard_header_len != ETH_HLEN.  Does it then conflict with the
deliberate reset in packet_snd()?

The short-frame safeguard there is gated on precisely the same inequality
that makes the new code effective:

net/packet/af_packet.c:packet_snd() {
		if (len < reserve + sizeof(struct ipv6hdr) &&
		    dev->min_header_len != hard_header_len)
			skb_reset_network_header(skb);
}

That reset came from commits 993675a3100b16 and 88a8121dc1d3d0.  For a VLAN
created over, say, drivers/net/usb/dm9601.c (dm9601_bind() does
dev->net->hard_header_len += DM_TX_OVERHEAD), any SOCK_RAW frame with
14 <= len < hard_header_len + sizeof(struct ipv6hdr) gets network_header
reset to the MAC header by packet_snd(), and then moved back to
mac_header + 14 by the new statement a few instructions later.  Which of the
two sites is meant to win?

There is also the reverse case.  cx82310_eth is ARPHRD_ETHER, does not set
NETIF_F_VLAN_CHALLENGED, and zeroes hard_header_len:

drivers/net/usb/cx82310_eth.c:cx82310_bind() {
	dev->net->hard_header_len = 0;
}

vlan_check_real_dev() only rejects NETIF_F_VLAN_CHALLENGED and non
ARPHRD_ETHER devices, so a VLAN device over it inherits hard_header_len 0
while min_header_len stays ETH_HLEN.  dev_validate_header() then accepts a
SOCK_RAW frame shorter than 14 bytes.  For such a frame, does
skb_set_network_header(skb, min_header_len) place network_header past
skb->tail, so that a later skb_reset_mac_len() (net/core/gso.c and
net/core/dev.c) computes skb->mac_len = 14 with skb->len below that?  That
looks like the state 993675a3100b16 was written to avoid.

Would gating on the frame contents, or on the actual position of the L2/L3
boundary in the skb, avoid touching these unrelated device stacks?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260901074214.57717-1-zhangjn_dev%40163.com