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

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

[Junnan Zhang]

AF_PACKET SOCK_RAW sets skb network_header to dev->hard_header_len in
packet_snd(). For Ethernet devices whose hard_header_len exceeds the
on-wire L2 header length (min_header_len = ETH_HLEN) -- e.g.
software-offload VLAN subinterfaces, where hard_header_len = ETH_HLEN +
VLAN_HLEN = 18 -- a non-VLAN SOCK_RAW frame still 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 (hard_header_len - min_header_len) 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; the VLAN subdevice
inserts the 802.1Q tag in vlan_dev_hard_start_xmit(). 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.

Set network_header to min_header_len for non-VLAN SOCK_RAW frames on
Ethernet devices whose hard_header_len exceeds min_header_len, so 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>
---
v2:
- Drop "on VLAN subinterfaces" from the subject and reword the scope to
  Ethernet devices whose hard_header_len exceeds the on-wire L2 header
  length (min_header_len). The min_header_len < hard_header_len test is
  not VLAN-specific; it also covers Ethernet drivers that reserve extra
  hard_header_len space for driver-internal wrapping.
- Restructure packet_parse_headers() to test dev->type once; replace
  has_vlan, which mixed the device and packet tests, with the
  packet-only is_vlan.
- Describe the header offset error generically as (hard_header_len -
  min_header_len) bytes instead of hard-coding the 4-byte VLAN case.
- Document the reproducer: virtio_net advertising
  NETIF_F_HW_VLAN_CTAG_FILTER but not NETIF_F_HW_VLAN_CTAG_TX, with
  PACKET_VNET_HDR and GSO triggering the -EINVAL from
  inet_gso_segment().

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..be5bf9db7ea1 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 = 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 = eth_type_vlan(skb->protocol);
+
+		/* For non-VLAN SOCK_RAW frames on Ethernet devices whose
+		 * hard_header_len exceeds the on-wire L2 header length
+		 * (min_header_len) -- e.g. software-offload VLAN subinterfaces,
+		 * or Ethernet drivers that reserve extra space in
+		 * hard_header_len for driver-internal wrapping -- the SOCK_RAW
+		 * send paths leave network_header at hard_header_len, while the
+		 * user frame's L3 sits at min_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 (!is_vlan && sock->type == SOCK_RAW &&
+		    skb->dev->min_header_len < skb->dev->hard_header_len)
+			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 &&
 	    vlan_get_protocol_and_depth(skb, skb->protocol, &depth) != 0)
 		skb_set_network_header(skb, depth);
 }
-- 
2.43.0

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

[Willem de Bruijn]

Junnan Zhang wrote:
> AF_PACKET SOCK_RAW sets skb network_header to dev->hard_header_len in
> packet_snd(). For Ethernet devices whose hard_header_len exceeds the
> on-wire L2 header length (min_header_len = ETH_HLEN) -- e.g.
> software-offload VLAN subinterfaces, where hard_header_len = ETH_HLEN +
> VLAN_HLEN = 18 -- a non-VLAN SOCK_RAW frame still 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 (hard_header_len - min_header_len) 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; the VLAN subdevice
> inserts the 802.1Q tag in vlan_dev_hard_start_xmit(). 

Where does it do this? validate_xmit_vlan does this on the physical
device I think.

That adds a bigger problem that the extra needed headroom of
(hard_header_len - min_header_len) is not reserved at the start of the
frame.

In packet_snd, reserve == hard_header_len, so the packet socket
starts writing at the start of the buffer. When
__vlan_insert_inner_tag inserts the tag, it tries to move the mac
header back, leaving the network header in place. Which works fine if
the network header was set correctly from the start, and the original
mac started at network_header - min_header_len.

This is all messy because before needed_headroom existed, legacy
tunnels had to resort to increasing hard_header_len to reserve extra
room for their outer headers.

Long aside, and I don't expect to address/disentangle that in this bug
fix targeting net. Will take a look whether we can safely do that in
net-next.

But please Link to this version of the patch (and thus the thread)
when sending the next version.

> 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.
> 
> Set network_header to min_header_len for non-VLAN SOCK_RAW frames on
> Ethernet devices whose hard_header_len exceeds min_header_len, so 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>
> ---
> v2:
> - Drop "on VLAN subinterfaces" from the subject and reword the scope to
>   Ethernet devices whose hard_header_len exceeds the on-wire L2 header
>   length (min_header_len). The min_header_len < hard_header_len test is
>   not VLAN-specific; it also covers Ethernet drivers that reserve extra
>   hard_header_len space for driver-internal wrapping.
> - Restructure packet_parse_headers() to test dev->type once; replace
>   has_vlan, which mixed the device and packet tests, with the
>   packet-only is_vlan.
> - Describe the header offset error generically as (hard_header_len -
>   min_header_len) bytes instead of hard-coding the 4-byte VLAN case.
> - Document the reproducer: virtio_net advertising
>   NETIF_F_HW_VLAN_CTAG_FILTER but not NETIF_F_HW_VLAN_CTAG_TX, with
>   PACKET_VNET_HDR and GSO triggering the -EINVAL from
>   inet_gso_segment().
> 
> 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..be5bf9db7ea1 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 = 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 = eth_type_vlan(skb->protocol);
> +
> +		/* For non-VLAN SOCK_RAW frames on Ethernet devices whose
> +		 * hard_header_len exceeds the on-wire L2 header length
> +		 * (min_header_len) -- e.g. software-offload VLAN subinterfaces,
> +		 * or Ethernet drivers that reserve extra space in
> +		 * hard_header_len for driver-internal wrapping -- the SOCK_RAW
> +		 * send paths leave network_header at hard_header_len, while the
> +		 * user frame's L3 sits at min_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 (!is_vlan && sock->type == SOCK_RAW &&
> +		    skb->dev->min_header_len < skb->dev->hard_header_len)

Instead of using min_header_len != hard_header_len to detect vlan devices,
consider is_vlan_dev(). That does not have false positives for other
protocols, and makes the code more self-descriptive.

Unless we are certain that the same also exhibits for other devices that
play hard_header_len games, such as GRE tunnels. But then the
ARPHRD_ETHER check excludes those anyway. So for now I would focus on the
VLAN issue only.

  if (sock->type == SOCK_RAW && !is_vlan_packet && is_vlan_dev(skb->dev))
    skb_set_network_header(skb, skb->dev->min_header_len);

It sucks that we have to add another branch in the hot path for an edge
case. Would be preferable if we can fix this in the vlan driver. But
that will come too late for skb_probe_transport_header.

> +			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 &&
>  	    vlan_get_protocol_and_depth(skb, skb->protocol, &depth) != 0)
>  		skb_set_network_header(skb, depth);
>  }
> -- 
> 2.43.0
> 

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

[Junnan Zhang]

Hi Willem,

Thanks for the detailed review.

> Where does it do this? validate_xmit_vlan does this on the physical
> device I think.

You're right. vlan_dev_hard_start_xmit() only attaches the tag metadata
via __vlan_hwaccel_put_tag(); the actual tag bytes are inserted later by
validate_xmit_vlan() on the physical device via
__vlan_hwaccel_push_inside(). I've corrected this in the v3 commit
message.

> That adds a bigger problem that the extra needed headroom of
> (hard_header_len - min_header_len) is not reserved at the start of
> the frame.

Understood. In this case it happens to work because
LL_RESERVED_SPACE_EX() rounds the reservation up, so enough headroom
remains in front of the MAC header for __vlan_insert_inner_tag() to
push the tag inside, and with the fix the MAC header correctly sits at
network_header - min_header_len. Agreed that disentangling the legacy
hard_header_len usage is beyond this bug fix targeting net; happy to
help test if you look at it for net-next.

> Instead of using min_header_len != hard_header_len to detect vlan
> devices, consider is_vlan_dev(). [...] So for now I would focus on
> the VLAN issue only.

Done in v3: the condition is now

	if (sock->type == SOCK_RAW && !is_vlan_packet && is_vlan_dev(skb->dev))

and I renamed is_vlan to is_vlan_packet to avoid confusion with
is_vlan_dev(). You're also right that the ARPHRD_ETHER check excluded
the other hard_header_len cases anyway, so I scoped the subject and
commit message back to VLAN subinterfaces.

> It sucks that we have to add another branch in the hot path for an
> edge case. Would be preferable if we can fix this in the vlan driver.
> But that will come too late for skb_probe_transport_header.

Agreed - the correction has to happen before skb_probe_transport_header()
in packet_snd(), so the VLAN driver xmit path is too late.

v3 sent as a new thread with a Link to v2.

Thanks,
Junnan

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

[netdev-bot+sashiko]

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

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

- [High] The new guard in packet_parse_headers()
  (`skb->dev->min_header_len < skb->dev->hard_header_len`) can never be…
- [Medium] packet_parse_headers() decides the L2/L3 boundary from
  `is_vlan = eth_type_vlan(skb->protocol)`, i.e. from the socket-…
- [Medium] packet_parse_headers() re-reads the live
  `skb->dev->hard_header_len` and `skb->dev->min_header_len` to decide…

--

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

This changes packet_parse_headers() so that non-VLAN AF_PACKET SOCK_RAW
frames on Ethernet devices whose hard_header_len is larger than
min_header_len get network_header moved to min_header_len before
skb_probe_transport_header() runs.  The existing VLAN correction is
reorganized to reuse a new is_vlan boolean.

> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index b22cda322136..9a4c70b659d4 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 = 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 = eth_type_vlan(skb->protocol);

[Severity: Medium]
Is is_vlan a reliable statement about the frame layout here?  For SOCK_RAW
the protocol is only re-derived from the frame when it is zero or
ETH_P_ALL, just above:

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

Otherwise skb->protocol comes from the bind protocol or the user supplied
sll_protocol, assigned in packet_snd() as skb->protocol = proto shortly
before this call (and similarly in tpacket_fill_skb()).

So a socket opened as socket(AF_PACKET, SOCK_RAW, htons(ETH_P_IP)) that
sends a frame which does carry an inline 802.1Q tag keeps
skb->protocol == ETH_P_IP, is_vlan stays false, and the new branch below
forces network_header to min_header_len, i.e. onto the VLAN TPID rather
than the IP header.  The frame-inspecting correction at the end is gated on
the same is_vlan, so it does not repair that.

For the devices where the new branch actually fires with
hard_header_len - min_header_len == VLAN_HLEN (for example the usbnet
gl620a and ps3_gelic_net, both 18), the pre-patch offset of 18 was the
correct L3 offset for such a single-tagged frame.  Does this turn a correct
offset into an incorrect one for that case, and would parsing the frame be
preferable to trusting the socket-declared protocol?

> +
> +		/* For non-VLAN SOCK_RAW frames on Ethernet devices whose
> +		 * hard_header_len exceeds the on-wire L2 header length
> +		 * (min_header_len) -- e.g. software-offload VLAN subinterfaces,
> +		 * or Ethernet drivers that reserve extra space in
> +		 * hard_header_len for driver-internal wrapping -- the SOCK_RAW
> +		 * send paths leave network_header at hard_header_len, while the
> +		 * user frame's L3 sits at min_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 (!is_vlan && sock->type == SOCK_RAW &&
> +		    skb->dev->min_header_len < skb->dev->hard_header_len)
> +			skb_set_network_header(skb, skb->dev->min_header_len);
> +	}

[Severity: High]
Can this condition ever be true for the reproducer described in the commit
message?  The message says:

    "e.g. software-offload VLAN subinterfaces, where hard_header_len =
    ETH_HLEN + VLAN_HLEN = 18"

and

    "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)."

In this tree vlan_dev_init() no longer inflates hard_header_len; the tag
space went to needed_headroom:

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() repeats the same unconditional assignment, so
toggling NETIF_F_HW_VLAN_CTAG_TX does not change hard_header_len either.
This came in with commit 447cbe95ebb9 ("vlan: fix skb_under_panic and
races when toggling HW VLAN offload").

min_header_len for ARPHRD_ETHER is only ever set by ether_setup():

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

and VLAN devices reach it through vlan_setup() -> ether_setup().
virtio_net does not touch hard_header_len at all.

That leaves min_header_len == hard_header_len == 14 for both the VLAN
subinterface and the real device in the described setup, so the branch is
skipped and the -EINVAL from inet_gso_segment() would still happen.  Is
the fix a no-op for the configuration it was written for?

The devices that can satisfy the test are unrelated ARPHRD_ETHER drivers
that enlarge hard_header_len after ether_setup(), such as the usbnet
gl620a, dm9601, rndis_host, smsc95xx and cdc_eem drivers, thunderbolt,
cxgb2, ps3_gelic_net and br2684.  Should the changelog and the comment be
reworded around that device class instead of software-tagging VLAN
subinterfaces, and could the reproducer be re-checked against the current
vlan_dev_init()?

[Severity: Medium]
Should these two reads use the caller's snapshot rather than the live
fields?  packet_snd() takes one:

net/packet/af_packet.c:packet_snd() {
	...
	hard_header_len = READ_ONCE(dev->hard_header_len);
	if (sock->type == SOCK_RAW)
		reserve = hard_header_len;
	...
	} 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);
	}
	...
	packet_parse_headers(skb, sock);
	...
}

That snapshot arrived in commit 03390aa32e669 ("packet: use consistent
hard_header_len in non-ring send paths") because dev->hard_header_len can
change under a sender, for example from bond_setup_by_slave():

drivers/net/bonding/bond_main.c:bond_setup_by_slave() {
	...
	bond_dev->hard_header_len   = slave_dev->hard_header_len;
	...
}

tpacket_fill_skb() is even passed hard_header_len as a parameter for the
same reason.  If hard_header_len shrinks to min_header_len between skb
construction (which reserved the larger value) and this point, the branch
does not fire and network_header stays where the commit message says it is
wrong; if it grows after the skb was built with reserve == min_header_len,
the branch fires on a length that was never used to lay out the skb.

Would passing the already snapshotted length into packet_parse_headers(),
or at least a single READ_ONCE() here, keep the offset decision consistent
with the layout that was used, and avoid the plain concurrent read?

>  
>  	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 &&
>  	    vlan_get_protocol_and_depth(skb, skb->protocol, &depth) != 0)
>  		skb_set_network_header(skb, depth);
>  }

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