[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