[PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
validation") added a '&& skb->network_header' check before flow-dissecting
GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
__virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
virtio_net_hdr_*_to_skb() before initializing skb->network_header and
skb->dev.

However, skb->network_header is an offset from skb->head, not a boolean
flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
LL_RESERVED_SPACE_EX(dev, 0) == 0), skb_reset_network_header(skb)
legitimately sets skb->network_header to 0.

Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
for TCPv4) without dissecting the packet, without validating ip_proto or
n_proto, and without setting skb->transport_header. If an IPv4 packet
carries IP options (ihl > 5, up to 60 bytes) or an IPv6 packet carries
extension headers, pulling only nh_min_len + thlen leaves the TCP header
outside skb->head, causing tcp_hdrlen(skb) in skb_gso_transport_seglen()
to read out-of-bounds:

  BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
  Read of size 1 at addr ffff888103c7ec4d by task repro/5718
  Call Trace:
   <TASK>
   skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
   skb_gso_network_seglen include/linux/skbuff.h:4718 [inline]
   skb_gso_validate_network_len+0x92/0x1a0 net/core/skbuff.c:5566
   ip_finish_output_gso net/ipv4/ip_output.c:285 [inline]
   __ip_finish_output+0x28b/0x4a0 net/ipv4/ip_output.c:316

In addition, when skb->protocol is pre-set by a caller before
__virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
virtio_net_hdr_match_proto() was not checked.

Fix this by:
1. Initializing skb->dev and skb->network_header (plus skb->protocol for
   IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
   tun_xdp_one(), virtnet_receive_done(), and raw_verify_header().
2. Removing '&& skb->network_header' and the unvalidated
   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
   packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
   their transport header pulled into linear data, and have
   skb->transport_header set.
3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
   after skb_flow_dissect_flow_keys_basic().

Fixes: 9e8db5913264 ("net: avoid false positives in untrusted gso validation")
Fixes: d5be7f632bad ("net: validate untrusted gso packets without csum offload")
Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/
Assisted-by: LLM
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
---
 arch/um/drivers/vector_transports.c |  1 +
 drivers/net/tun.c                   | 23 ++++++----
 drivers/net/virtio_net.c            |  2 +
 include/linux/virtio_net.h          | 66 ++++++++++++++---------------
 4 files changed, 48 insertions(+), 44 deletions(-)

diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
index ddd127ee96785daa4c485b2a06f078686efd2046..e1fb2a76fdff59ef3032d091be06a06fa58046cc 100644
--- a/arch/um/drivers/vector_transports.c
+++ b/arch/um/drivers/vector_transports.c
@@ -209,6 +209,7 @@ static int raw_verify_header(
 	if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
 		return 1;
 
+	skb_set_network_header(skb, ETH_HLEN);
 	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
 	return 0;
 }
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5a302709a68aa3308b0c850b4a2957df3260b352..242899f7fd0711c5c59ff85accbf5bd1be9c6f39 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1897,12 +1897,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		}
 	}
 
-	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
-		atomic_long_inc(&tun->rx_frame_errors);
-		err = -EINVAL;
-		goto free_skb;
-	}
-
+	skb->dev = tun->dev;
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case IFF_TUN:
 		if (tun->flags & IFF_NO_PI) {
@@ -1927,9 +1922,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			}
 		}
 
-		skb_reset_mac_header(skb);
+		skb_reset_network_header(skb);
 		skb->protocol = pi.proto;
-		skb->dev = tun->dev;
 		break;
 	case IFF_TAP:
 		if (!pskb_may_pull(skb, ETH_HLEN)) {
@@ -1937,10 +1931,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			drop_reason = SKB_DROP_REASON_HDR_TRUNC;
 			goto drop;
 		}
-		skb->protocol = eth_type_trans(skb, tun->dev);
+		skb_set_network_header(skb, ETH_HLEN);
 		break;
 	}
 
+	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
+		atomic_long_inc(&tun->rx_frame_errors);
+		err = -EINVAL;
+		goto free_skb;
+	}
+
+	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
+		skb->protocol = eth_type_trans(skb, tun->dev);
+
 	/* copy skb_ubuf_info for callback when skb has no error */
 	if (zerocopy) {
 		skb_zcopy_init(skb, msg_control);
@@ -2600,6 +2603,8 @@ static int tun_xdp_one(struct tun_struct *tun,
 
 	features = tun_vnet_hdr_guest_features(READ_ONCE(tun->vnet_hdr_sz));
 	tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
+	skb->dev = tun->dev;
+	skb_set_network_header(skb, ETH_HLEN);
 	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
 		atomic_long_inc(&tun->rx_frame_errors);
 		kfree_skb(skb);
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index bf82ef9874abb4094931496fb6064a12ee75b789..daab43ac92ce4276b0f8f88684991e3993022ce6 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2515,6 +2515,8 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
 		goto frame_err;
 	}
 
+	skb->dev = dev;
+	skb_set_network_header(skb, ETH_HLEN);
 	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
 				      vi->rx_tnl_csum,
 				      virtio_is_little_endian(vi->vdev))) {
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index c381b916c1b54afacba5473888af589a24fe087c..8902d5a5c418c46af0a6e7b26ae89948c20b0b54 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -111,48 +111,44 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
 		p_off = nh_min_len + thlen;
 		if (!pskb_may_pull(skb, p_off))
 			return -EINVAL;
-	} else {
+	} else if (gso_type) {
 		/* gso packets without NEEDS_CSUM do not set transport_offset.
 		 * probe and drop if does not match one of the above types.
 		 */
-		if (gso_type && skb->network_header) {
-			struct flow_keys_basic keys;
-
-			if (!skb->protocol) {
-				__be16 protocol = dev_parse_header_protocol(skb);
-
-				if (!protocol)
-					virtio_net_hdr_set_proto(skb, hdr);
-				else if (!virtio_net_hdr_match_proto(protocol,
-								 hdr_gso_type))
-					return -EINVAL;
-				else
-					skb->protocol = protocol;
-			}
-retry:
-			if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
-							      NULL, 0, 0, 0,
-							      0)) {
-				/* UFO does not specify ipv4 or 6: try both */
-				if (gso_type & SKB_GSO_UDP &&
-				    skb->protocol == htons(ETH_P_IP)) {
-					skb->protocol = htons(ETH_P_IPV6);
-					goto retry;
-				}
-				return -EINVAL;
-			}
+		struct flow_keys_basic keys;
 
-			p_off = keys.control.thoff + thlen;
-			if (!pskb_may_pull(skb, p_off) ||
-			    keys.basic.ip_proto != ip_proto)
-				return -EINVAL;
+		if (!skb->protocol) {
+			__be16 protocol = dev_parse_header_protocol(skb);
 
-			skb_set_transport_header(skb, keys.control.thoff);
-		} else if (gso_type) {
-			p_off = nh_min_len + thlen;
-			if (!pskb_may_pull(skb, p_off))
+			if (!protocol)
+				virtio_net_hdr_set_proto(skb, hdr);
+			else if (!virtio_net_hdr_match_proto(protocol,
+							     hdr_gso_type))
 				return -EINVAL;
+			else
+				skb->protocol = protocol;
 		}
+retry:
+		if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
+						      NULL, 0, 0, 0,
+						      0)) {
+			/* UFO does not specify ipv4 or 6: try both */
+			if (gso_type & SKB_GSO_UDP &&
+			    skb->protocol == htons(ETH_P_IP)) {
+				skb->protocol = htons(ETH_P_IPV6);
+				goto retry;
+			}
+			return -EINVAL;
+		}
+
+		p_off = keys.control.thoff + thlen;
+		if (!pskb_may_pull(skb, p_off) ||
+		    keys.basic.ip_proto != ip_proto ||
+		    !virtio_net_hdr_match_proto(keys.basic.n_proto,
+						hdr_gso_type))
+			return -EINVAL;
+
+		skb_set_transport_header(skb, keys.control.thoff);
 	}
 
 	if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Michael S. Tsirkin]

On Sun, Sep 27, 2026 at 07:55:36PM +0000, Eric Dumazet wrote:
> Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> validation") added a '&& skb->network_header' check before flow-dissecting
> GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> skb->dev.
> 
> However, skb->network_header is an offset from skb->head, not a boolean
> flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> LL_RESERVED_SPACE_EX(dev, 0) == 0), skb_reset_network_header(skb)
> legitimately sets skb->network_header to 0.
> 
> Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
> the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
> for TCPv4) without dissecting the packet, without validating ip_proto or
> n_proto, and without setting skb->transport_header. If an IPv4 packet
> carries IP options (ihl > 5, up to 60 bytes) or an IPv6 packet carries
> extension headers, pulling only nh_min_len + thlen leaves the TCP header
> outside skb->head, causing tcp_hdrlen(skb) in skb_gso_transport_seglen()
> to read out-of-bounds:
> 
>   BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
>   Read of size 1 at addr ffff888103c7ec4d by task repro/5718
>   Call Trace:
>    <TASK>
>    skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
>    skb_gso_network_seglen include/linux/skbuff.h:4718 [inline]
>    skb_gso_validate_network_len+0x92/0x1a0 net/core/skbuff.c:5566
>    ip_finish_output_gso net/ipv4/ip_output.c:285 [inline]
>    __ip_finish_output+0x28b/0x4a0 net/ipv4/ip_output.c:316
> 
> In addition, when skb->protocol is pre-set by a caller before
> __virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
> virtio_net_hdr_match_proto() was not checked.
> 
> Fix this by:
> 1. Initializing skb->dev and skb->network_header (plus skb->protocol for
>    IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
>    tun_xdp_one(), virtnet_receive_done(), and raw_verify_header().
> 2. Removing '&& skb->network_header' and the unvalidated
>    'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
>    packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
>    their transport header pulled into linear data, and have
>    skb->transport_header set.
> 3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
>    after skb_flow_dissect_flow_keys_basic().
> 
> Fixes: 9e8db5913264 ("net: avoid false positives in untrusted gso validation")
> Fixes: d5be7f632bad ("net: validate untrusted gso packets without csum offload")
> Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/
> Assisted-by: LLM
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> ---
>  arch/um/drivers/vector_transports.c |  1 +
>  drivers/net/tun.c                   | 23 ++++++----
>  drivers/net/virtio_net.c            |  2 +
>  include/linux/virtio_net.h          | 66 ++++++++++++++---------------
>  4 files changed, 48 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> index ddd127ee96785daa4c485b2a06f078686efd2046..e1fb2a76fdff59ef3032d091be06a06fa58046cc 100644
> --- a/arch/um/drivers/vector_transports.c
> +++ b/arch/um/drivers/vector_transports.c
> @@ -209,6 +209,7 @@ static int raw_verify_header(
>  	if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
>  		return 1;
>  
> +	skb_set_network_header(skb, ETH_HLEN);
>  	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
>  	return 0;
>  }
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 5a302709a68aa3308b0c850b4a2957df3260b352..242899f7fd0711c5c59ff85accbf5bd1be9c6f39 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1897,12 +1897,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  		}
>  	}
>  
> -	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> -		atomic_long_inc(&tun->rx_frame_errors);
> -		err = -EINVAL;
> -		goto free_skb;
> -	}
> -
> +	skb->dev = tun->dev;
>  	switch (tun->flags & TUN_TYPE_MASK) {
>  	case IFF_TUN:
>  		if (tun->flags & IFF_NO_PI) {
> @@ -1927,9 +1922,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  			}
>  		}
>  
> -		skb_reset_mac_header(skb);


Could you explain why is this being removed?
What sets the mac header then?
Thanks!


> +		skb_reset_network_header(skb);
>  		skb->protocol = pi.proto;
> -		skb->dev = tun->dev;
>  		break;
>  	case IFF_TAP:
>  		if (!pskb_may_pull(skb, ETH_HLEN)) {
> @@ -1937,10 +1931,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  			drop_reason = SKB_DROP_REASON_HDR_TRUNC;
>  			goto drop;
>  		}
> -		skb->protocol = eth_type_trans(skb, tun->dev);
> +		skb_set_network_header(skb, ETH_HLEN);
>  		break;
>  	}
>  
> +	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> +		atomic_long_inc(&tun->rx_frame_errors);
> +		err = -EINVAL;
> +		goto free_skb;
> +	}
> +
> +	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
> +		skb->protocol = eth_type_trans(skb, tun->dev);
> +
>  	/* copy skb_ubuf_info for callback when skb has no error */
>  	if (zerocopy) {
>  		skb_zcopy_init(skb, msg_control);
> @@ -2600,6 +2603,8 @@ static int tun_xdp_one(struct tun_struct *tun,
>  
>  	features = tun_vnet_hdr_guest_features(READ_ONCE(tun->vnet_hdr_sz));
>  	tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
> +	skb->dev = tun->dev;
> +	skb_set_network_header(skb, ETH_HLEN);
>  	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
>  		atomic_long_inc(&tun->rx_frame_errors);
>  		kfree_skb(skb);
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index bf82ef9874abb4094931496fb6064a12ee75b789..daab43ac92ce4276b0f8f88684991e3993022ce6 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2515,6 +2515,8 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
>  		goto frame_err;
>  	}
>  
> +	skb->dev = dev;
> +	skb_set_network_header(skb, ETH_HLEN);
>  	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
>  				      vi->rx_tnl_csum,
>  				      virtio_is_little_endian(vi->vdev))) {
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index c381b916c1b54afacba5473888af589a24fe087c..8902d5a5c418c46af0a6e7b26ae89948c20b0b54 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -111,48 +111,44 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
>  		p_off = nh_min_len + thlen;
>  		if (!pskb_may_pull(skb, p_off))
>  			return -EINVAL;
> -	} else {
> +	} else if (gso_type) {
>  		/* gso packets without NEEDS_CSUM do not set transport_offset.
>  		 * probe and drop if does not match one of the above types.
>  		 */
> -		if (gso_type && skb->network_header) {
> -			struct flow_keys_basic keys;
> -
> -			if (!skb->protocol) {
> -				__be16 protocol = dev_parse_header_protocol(skb);
> -
> -				if (!protocol)
> -					virtio_net_hdr_set_proto(skb, hdr);
> -				else if (!virtio_net_hdr_match_proto(protocol,
> -								 hdr_gso_type))
> -					return -EINVAL;
> -				else
> -					skb->protocol = protocol;
> -			}
> -retry:
> -			if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
> -							      NULL, 0, 0, 0,
> -							      0)) {
> -				/* UFO does not specify ipv4 or 6: try both */
> -				if (gso_type & SKB_GSO_UDP &&
> -				    skb->protocol == htons(ETH_P_IP)) {
> -					skb->protocol = htons(ETH_P_IPV6);
> -					goto retry;
> -				}
> -				return -EINVAL;
> -			}
> +		struct flow_keys_basic keys;
>  
> -			p_off = keys.control.thoff + thlen;
> -			if (!pskb_may_pull(skb, p_off) ||
> -			    keys.basic.ip_proto != ip_proto)
> -				return -EINVAL;
> +		if (!skb->protocol) {
> +			__be16 protocol = dev_parse_header_protocol(skb);
>  
> -			skb_set_transport_header(skb, keys.control.thoff);
> -		} else if (gso_type) {
> -			p_off = nh_min_len + thlen;
> -			if (!pskb_may_pull(skb, p_off))
> +			if (!protocol)
> +				virtio_net_hdr_set_proto(skb, hdr);
> +			else if (!virtio_net_hdr_match_proto(protocol,
> +							     hdr_gso_type))
>  				return -EINVAL;
> +			else
> +				skb->protocol = protocol;
>  		}
> +retry:
> +		if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
> +						      NULL, 0, 0, 0,
> +						      0)) {
> +			/* UFO does not specify ipv4 or 6: try both */
> +			if (gso_type & SKB_GSO_UDP &&
> +			    skb->protocol == htons(ETH_P_IP)) {
> +				skb->protocol = htons(ETH_P_IPV6);
> +				goto retry;
> +			}
> +			return -EINVAL;
> +		}
> +
> +		p_off = keys.control.thoff + thlen;
> +		if (!pskb_may_pull(skb, p_off) ||
> +		    keys.basic.ip_proto != ip_proto ||
> +		    !virtio_net_hdr_match_proto(keys.basic.n_proto,
> +						hdr_gso_type))




> +			return -EINVAL;
> +
> +		skb_set_transport_header(skb, keys.control.thoff);
>  	}
>  
>  	if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

On Sun, Sep 27, 2026 at 10:33 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Sep 27, 2026 at 07:55:36PM +0000, Eric Dumazet wrote:
> > Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> > validation") added a '&& skb->network_header' check before flow-dissecting
> > GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> > __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> > tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> > virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> > skb->dev.
> >
> > However, skb->network_header is an offset from skb->head, not a boolean
> > flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> > instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> > LL_RESERVED_SPACE_EX(dev, 0) == 0), skb_reset_network_header(skb)
> > legitimately sets skb->network_header to 0.
> >
> > Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
> > the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
> > for TCPv4) without dissecting the packet, without validating ip_proto or
> > n_proto, and without setting skb->transport_header. If an IPv4 packet
> > carries IP options (ihl > 5, up to 60 bytes) or an IPv6 packet carries
> > extension headers, pulling only nh_min_len + thlen leaves the TCP header
> > outside skb->head, causing tcp_hdrlen(skb) in skb_gso_transport_seglen()
> > to read out-of-bounds:
> >
> >   BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
> >   Read of size 1 at addr ffff888103c7ec4d by task repro/5718
> >   Call Trace:
> >    <TASK>
> >    skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
> >    skb_gso_network_seglen include/linux/skbuff.h:4718 [inline]
> >    skb_gso_validate_network_len+0x92/0x1a0 net/core/skbuff.c:5566
> >    ip_finish_output_gso net/ipv4/ip_output.c:285 [inline]
> >    __ip_finish_output+0x28b/0x4a0 net/ipv4/ip_output.c:316
> >
> > In addition, when skb->protocol is pre-set by a caller before
> > __virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
> > virtio_net_hdr_match_proto() was not checked.
> >
> > Fix this by:
> > 1. Initializing skb->dev and skb->network_header (plus skb->protocol for
> >    IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
> >    tun_xdp_one(), virtnet_receive_done(), and raw_verify_header().
> > 2. Removing '&& skb->network_header' and the unvalidated
> >    'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
> >    packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
> >    their transport header pulled into linear data, and have
> >    skb->transport_header set.
> > 3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
> >    after skb_flow_dissect_flow_keys_basic().
> >
> > Fixes: 9e8db5913264 ("net: avoid false positives in untrusted gso validation")
> > Fixes: d5be7f632bad ("net: validate untrusted gso packets without csum offload")
> > Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/
> > Assisted-by: LLM
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Willem de Bruijn <willemb@google.com>
> > Cc: Jason Wang <jasowang@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  arch/um/drivers/vector_transports.c |  1 +
> >  drivers/net/tun.c                   | 23 ++++++----
> >  drivers/net/virtio_net.c            |  2 +
> >  include/linux/virtio_net.h          | 66 ++++++++++++++---------------
> >  4 files changed, 48 insertions(+), 44 deletions(-)
> >
> > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> > index ddd127ee96785daa4c485b2a06f078686efd2046..e1fb2a76fdff59ef3032d091be06a06fa58046cc 100644
> > --- a/arch/um/drivers/vector_transports.c
> > +++ b/arch/um/drivers/vector_transports.c
> > @@ -209,6 +209,7 @@ static int raw_verify_header(
> >       if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
> >               return 1;
> >
> > +     skb_set_network_header(skb, ETH_HLEN);
> >       virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
> >       return 0;
> >  }
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 5a302709a68aa3308b0c850b4a2957df3260b352..242899f7fd0711c5c59ff85accbf5bd1be9c6f39 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -1897,12 +1897,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >               }
> >       }
> >
> > -     if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> > -             atomic_long_inc(&tun->rx_frame_errors);
> > -             err = -EINVAL;
> > -             goto free_skb;
> > -     }
> > -
> > +     skb->dev = tun->dev;
> >       switch (tun->flags & TUN_TYPE_MASK) {
> >       case IFF_TUN:
> >               if (tun->flags & IFF_NO_PI) {
> > @@ -1927,9 +1922,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >                       }
> >               }
> >
> > -             skb_reset_mac_header(skb);
>
>
> Could you explain why is this being removed?
> What sets the mac header then?
> Thanks!

Hi Michael

tun_get_user() uncondittionally call tun_vnet_hdr_tnl_to_skb() right
after the switch statement (even when IFF_VNET_HDR is not set, with a
zeroed hdr), which calls __virtio_net_hdr_to_skb().

__virtio_net_hdr_to_skb() unconditionally calls skb_reset_mac_header(skb)
(since commit 61431a5907fc ("net: ensure mac header is set in
virtio_net_hdr_to_skb()")).

Before this patch, tun_get_user() called tun_vnet_hdr_tnl_to_skb()
before the switch statement (which already reset mac_header, but left
network_header unset during __virtio_net_hdr_to_skb()), and then reset
mac_header a second time in the IFF_TUN case.

Thanks.

>
>
> > +             skb_reset_network_header(skb);
> >               skb->protocol = pi.proto;
> > -             skb->dev = tun->dev;
> >               break;
> >       case IFF_TAP:
> >               if (!pskb_may_pull(skb, ETH_HLEN)) {
> > @@ -1937,10 +1931,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >                       drop_reason = SKB_DROP_REASON_HDR_TRUNC;
> >                       goto drop;
> >               }
> > -             skb->protocol = eth_type_trans(skb, tun->dev);
> > +             skb_set_network_header(skb, ETH_HLEN);
> >               break;
> >       }
> >
> > +     if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> > +             atomic_long_inc(&tun->rx_frame_errors);
> > +             err = -EINVAL;
> > +             goto free_skb;
> > +     }
> > +
> > +     if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
> > +             skb->protocol = eth_type_trans(skb, tun->dev);
> > +
> >       /* copy skb_ubuf_info for callback when skb has no error */
> >       if (zerocopy) {
> >               skb_zcopy_init(skb, msg_control);
> > @@ -2600,6 +2603,8 @@ static int tun_xdp_one(struct tun_struct *tun,
> >
> >       features = tun_vnet_hdr_guest_features(READ_ONCE(tun->vnet_hdr_sz));
> >       tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
> > +     skb->dev = tun->dev;
> > +     skb_set_network_header(skb, ETH_HLEN);
> >       if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
> >               atomic_long_inc(&tun->rx_frame_errors);
> >               kfree_skb(skb);
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index bf82ef9874abb4094931496fb6064a12ee75b789..daab43ac92ce4276b0f8f88684991e3993022ce6 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2515,6 +2515,8 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
> >               goto frame_err;
> >       }
> >
> > +     skb->dev = dev;
> > +     skb_set_network_header(skb, ETH_HLEN);
> >       if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
> >                                     vi->rx_tnl_csum,
> >                                     virtio_is_little_endian(vi->vdev))) {
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index c381b916c1b54afacba5473888af589a24fe087c..8902d5a5c418c46af0a6e7b26ae89948c20b0b54 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -111,48 +111,44 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> >               p_off = nh_min_len + thlen;
> >               if (!pskb_may_pull(skb, p_off))
> >                       return -EINVAL;
> > -     } else {
> > +     } else if (gso_type) {
> >               /* gso packets without NEEDS_CSUM do not set transport_offset.
> >                * probe and drop if does not match one of the above types.
> >                */
> > -             if (gso_type && skb->network_header) {
> > -                     struct flow_keys_basic keys;
> > -
> > -                     if (!skb->protocol) {
> > -                             __be16 protocol = dev_parse_header_protocol(skb);
> > -
> > -                             if (!protocol)
> > -                                     virtio_net_hdr_set_proto(skb, hdr);
> > -                             else if (!virtio_net_hdr_match_proto(protocol,
> > -                                                              hdr_gso_type))
> > -                                     return -EINVAL;
> > -                             else
> > -                                     skb->protocol = protocol;
> > -                     }
> > -retry:
> > -                     if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
> > -                                                           NULL, 0, 0, 0,
> > -                                                           0)) {
> > -                             /* UFO does not specify ipv4 or 6: try both */
> > -                             if (gso_type & SKB_GSO_UDP &&
> > -                                 skb->protocol == htons(ETH_P_IP)) {
> > -                                     skb->protocol = htons(ETH_P_IPV6);
> > -                                     goto retry;
> > -                             }
> > -                             return -EINVAL;
> > -                     }
> > +             struct flow_keys_basic keys;
> >
> > -                     p_off = keys.control.thoff + thlen;
> > -                     if (!pskb_may_pull(skb, p_off) ||
> > -                         keys.basic.ip_proto != ip_proto)
> > -                             return -EINVAL;
> > +             if (!skb->protocol) {
> > +                     __be16 protocol = dev_parse_header_protocol(skb);
> >
> > -                     skb_set_transport_header(skb, keys.control.thoff);
> > -             } else if (gso_type) {
> > -                     p_off = nh_min_len + thlen;
> > -                     if (!pskb_may_pull(skb, p_off))
> > +                     if (!protocol)
> > +                             virtio_net_hdr_set_proto(skb, hdr);
> > +                     else if (!virtio_net_hdr_match_proto(protocol,
> > +                                                          hdr_gso_type))
> >                               return -EINVAL;
> > +                     else
> > +                             skb->protocol = protocol;
> >               }
> > +retry:
> > +             if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
> > +                                                   NULL, 0, 0, 0,
> > +                                                   0)) {
> > +                     /* UFO does not specify ipv4 or 6: try both */
> > +                     if (gso_type & SKB_GSO_UDP &&
> > +                         skb->protocol == htons(ETH_P_IP)) {
> > +                             skb->protocol = htons(ETH_P_IPV6);
> > +                             goto retry;
> > +                     }
> > +                     return -EINVAL;
> > +             }
> > +
> > +             p_off = keys.control.thoff + thlen;
> > +             if (!pskb_may_pull(skb, p_off) ||
> > +                 keys.basic.ip_proto != ip_proto ||
> > +                 !virtio_net_hdr_match_proto(keys.basic.n_proto,
> > +                                             hdr_gso_type))
>
>
>
>
> > +                     return -EINVAL;
> > +
> > +             skb_set_transport_header(skb, keys.control.thoff);
> >       }
> >
> >       if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> > --
> > 2.56.0.rc1.315.gc6ed9934b7-goog
>

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Willem de Bruijn]

Eric Dumazet wrote:
> Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> validation") added a '&& skb->network_header' check before flow-dissecting
> GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> skb->dev.
> 
> However, skb->network_header is an offset from skb->head, not a boolean
> flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> LL_RESERVED_SPACE_EX(dev, 0) == 0), skb_reset_network_header(skb)
> legitimately sets skb->network_header to 0.
> 
> Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
> the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
> for TCPv4) without dissecting the packet, without validating ip_proto or
> n_proto, and without setting skb->transport_header. If an IPv4 packet
> carries IP options (ihl > 5, up to 60 bytes) or an IPv6 packet carries
> extension headers, pulling only nh_min_len + thlen leaves the TCP header
> outside skb->head, causing tcp_hdrlen(skb) in skb_gso_transport_seglen()
> to read out-of-bounds:
> 
>   BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
>   Read of size 1 at addr ffff888103c7ec4d by task repro/5718
>   Call Trace:
>    <TASK>
>    skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
>    skb_gso_network_seglen include/linux/skbuff.h:4718 [inline]
>    skb_gso_validate_network_len+0x92/0x1a0 net/core/skbuff.c:5566
>    ip_finish_output_gso net/ipv4/ip_output.c:285 [inline]
>    __ip_finish_output+0x28b/0x4a0 net/ipv4/ip_output.c:316
> 
> In addition, when skb->protocol is pre-set by a caller before
> __virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
> virtio_net_hdr_match_proto() was not checked.
> 
> Fix this by:
> 1. Initializing skb->dev and skb->network_header (plus skb->protocol for
>    IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
>    tun_xdp_one(), virtnet_receive_done(), and raw_verify_header().
> 2. Removing '&& skb->network_header' and the unvalidated
>    'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
>    packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
>    their transport header pulled into linear data, and have
>    skb->transport_header set.
> 3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
>    after skb_flow_dissect_flow_keys_basic().
> 
> Fixes: 9e8db5913264 ("net: avoid false positives in untrusted gso validation")
> Fixes: d5be7f632bad ("net: validate untrusted gso packets without csum offload")
> Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/
> Assisted-by: LLM
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>

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

Thanks Eric.

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Michael S. Tsirkin]

On Sun, Sep 27, 2026 at 07:55:36PM +0000, Eric Dumazet wrote:
> Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> validation") added a '&& skb->network_header' check before flow-dissecting
> GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> skb->dev.
> 
> However, skb->network_header is an offset from skb->head, not a boolean
> flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> LL_RESERVED_SPACE_EX(dev, 0) == 0),

Hmm. I have:

LL_RESERVED_SPACE_EX(dev, 0) 
        ((((hlen) + READ_ONCE((dev)->needed_headroom)) \
          & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)

#define HH_DATA_MOD     16

So how can LL_RESERVED_SPACE_EX(dev, 0) == 0 ?


> skb_reset_network_header(skb)
> legitimately sets skb->network_header to 0.
> 
> Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
> the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
> for TCPv4) without dissecting the packet, without validating ip_proto or
> n_proto, and without setting skb->transport_header. If an IPv4 packet
> carries IP options (ihl > 5, up to 60 bytes) or an IPv6 packet carries
> extension headers, pulling only nh_min_len + thlen leaves the TCP header
> outside skb->head, causing tcp_hdrlen(skb) in skb_gso_transport_seglen()
> to read out-of-bounds:
> 
>   BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
>   Read of size 1 at addr ffff888103c7ec4d by task repro/5718
>   Call Trace:
>    <TASK>
>    skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
>    skb_gso_network_seglen include/linux/skbuff.h:4718 [inline]
>    skb_gso_validate_network_len+0x92/0x1a0 net/core/skbuff.c:5566
>    ip_finish_output_gso net/ipv4/ip_output.c:285 [inline]
>    __ip_finish_output+0x28b/0x4a0 net/ipv4/ip_output.c:316
> 
> In addition, when skb->protocol is pre-set by a caller before
> __virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
> virtio_net_hdr_match_proto() was not checked.
> 
> Fix this by:
> 1. Initializing skb->dev and skb->network_header (plus skb->protocol for
>    IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
>    tun_xdp_one(), virtnet_receive_done(), and raw_verify_header().
> 2. Removing '&& skb->network_header' and the unvalidated
>    'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
>    packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
>    their transport header pulled into linear data, and have
>    skb->transport_header set.
> 3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
>    after skb_flow_dissect_flow_keys_basic().
> 
> Fixes: 9e8db5913264 ("net: avoid false positives in untrusted gso validation")
> Fixes: d5be7f632bad ("net: validate untrusted gso packets without csum offload")
> Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/
> Assisted-by: LLM
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> ---
>  arch/um/drivers/vector_transports.c |  1 +
>  drivers/net/tun.c                   | 23 ++++++----
>  drivers/net/virtio_net.c            |  2 +
>  include/linux/virtio_net.h          | 66 ++++++++++++++---------------
>  4 files changed, 48 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> index ddd127ee96785daa4c485b2a06f078686efd2046..e1fb2a76fdff59ef3032d091be06a06fa58046cc 100644
> --- a/arch/um/drivers/vector_transports.c
> +++ b/arch/um/drivers/vector_transports.c
> @@ -209,6 +209,7 @@ static int raw_verify_header(
>  	if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
>  		return 1;
>  
> +	skb_set_network_header(skb, ETH_HLEN);
>  	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
>  	return 0;
>  }
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 5a302709a68aa3308b0c850b4a2957df3260b352..242899f7fd0711c5c59ff85accbf5bd1be9c6f39 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1897,12 +1897,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  		}
>  	}
>  
> -	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> -		atomic_long_inc(&tun->rx_frame_errors);
> -		err = -EINVAL;
> -		goto free_skb;
> -	}
> -
> +	skb->dev = tun->dev;
>  	switch (tun->flags & TUN_TYPE_MASK) {
>  	case IFF_TUN:
>  		if (tun->flags & IFF_NO_PI) {
> @@ -1927,9 +1922,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  			}
>  		}
>  
> -		skb_reset_mac_header(skb);
> +		skb_reset_network_header(skb);
>  		skb->protocol = pi.proto;
> -		skb->dev = tun->dev;
>  		break;
>  	case IFF_TAP:
>  		if (!pskb_may_pull(skb, ETH_HLEN)) {
> @@ -1937,10 +1931,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  			drop_reason = SKB_DROP_REASON_HDR_TRUNC;
>  			goto drop;
>  		}
> -		skb->protocol = eth_type_trans(skb, tun->dev);
> +		skb_set_network_header(skb, ETH_HLEN);
>  		break;
>  	}
>  
> +	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> +		atomic_long_inc(&tun->rx_frame_errors);
> +		err = -EINVAL;
> +		goto free_skb;
> +	}
> +
> +	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
> +		skb->protocol = eth_type_trans(skb, tun->dev);
> +
>  	/* copy skb_ubuf_info for callback when skb has no error */
>  	if (zerocopy) {
>  		skb_zcopy_init(skb, msg_control);
> @@ -2600,6 +2603,8 @@ static int tun_xdp_one(struct tun_struct *tun,
>  
>  	features = tun_vnet_hdr_guest_features(READ_ONCE(tun->vnet_hdr_sz));
>  	tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
> +	skb->dev = tun->dev;
> +	skb_set_network_header(skb, ETH_HLEN);
>  	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
>  		atomic_long_inc(&tun->rx_frame_errors);
>  		kfree_skb(skb);
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index bf82ef9874abb4094931496fb6064a12ee75b789..daab43ac92ce4276b0f8f88684991e3993022ce6 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2515,6 +2515,8 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
>  		goto frame_err;
>  	}
>  
> +	skb->dev = dev;
> +	skb_set_network_header(skb, ETH_HLEN);
>  	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
>  				      vi->rx_tnl_csum,
>  				      virtio_is_little_endian(vi->vdev))) {
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index c381b916c1b54afacba5473888af589a24fe087c..8902d5a5c418c46af0a6e7b26ae89948c20b0b54 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -111,48 +111,44 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
>  		p_off = nh_min_len + thlen;
>  		if (!pskb_may_pull(skb, p_off))
>  			return -EINVAL;
> -	} else {
> +	} else if (gso_type) {
>  		/* gso packets without NEEDS_CSUM do not set transport_offset.
>  		 * probe and drop if does not match one of the above types.
>  		 */
> -		if (gso_type && skb->network_header) {
> -			struct flow_keys_basic keys;
> -
> -			if (!skb->protocol) {
> -				__be16 protocol = dev_parse_header_protocol(skb);
> -
> -				if (!protocol)
> -					virtio_net_hdr_set_proto(skb, hdr);
> -				else if (!virtio_net_hdr_match_proto(protocol,
> -								 hdr_gso_type))
> -					return -EINVAL;
> -				else
> -					skb->protocol = protocol;
> -			}
> -retry:
> -			if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
> -							      NULL, 0, 0, 0,
> -							      0)) {
> -				/* UFO does not specify ipv4 or 6: try both */
> -				if (gso_type & SKB_GSO_UDP &&
> -				    skb->protocol == htons(ETH_P_IP)) {
> -					skb->protocol = htons(ETH_P_IPV6);
> -					goto retry;
> -				}
> -				return -EINVAL;
> -			}
> +		struct flow_keys_basic keys;
>  
> -			p_off = keys.control.thoff + thlen;
> -			if (!pskb_may_pull(skb, p_off) ||
> -			    keys.basic.ip_proto != ip_proto)
> -				return -EINVAL;
> +		if (!skb->protocol) {
> +			__be16 protocol = dev_parse_header_protocol(skb);
>  
> -			skb_set_transport_header(skb, keys.control.thoff);
> -		} else if (gso_type) {
> -			p_off = nh_min_len + thlen;
> -			if (!pskb_may_pull(skb, p_off))
> +			if (!protocol)
> +				virtio_net_hdr_set_proto(skb, hdr);
> +			else if (!virtio_net_hdr_match_proto(protocol,
> +							     hdr_gso_type))
>  				return -EINVAL;
> +			else
> +				skb->protocol = protocol;
>  		}
> +retry:
> +		if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
> +						      NULL, 0, 0, 0,
> +						      0)) {
> +			/* UFO does not specify ipv4 or 6: try both */
> +			if (gso_type & SKB_GSO_UDP &&
> +			    skb->protocol == htons(ETH_P_IP)) {
> +				skb->protocol = htons(ETH_P_IPV6);
> +				goto retry;
> +			}
> +			return -EINVAL;
> +		}
> +
> +		p_off = keys.control.thoff + thlen;
> +		if (!pskb_may_pull(skb, p_off) ||
> +		    keys.basic.ip_proto != ip_proto ||
> +		    !virtio_net_hdr_match_proto(keys.basic.n_proto,
> +						hdr_gso_type))
> +			return -EINVAL;
> +
> +		skb_set_transport_header(skb, keys.control.thoff);
>  	}
>  
>  	if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

On Mon, Sep 28, 2026 at 3:31 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Sep 27, 2026 at 07:55:36PM +0000, Eric Dumazet wrote:
> > Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> > validation") added a '&& skb->network_header' check before flow-dissecting
> > GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> > __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> > tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> > virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> > skb->dev.
> >
> > However, skb->network_header is an offset from skb->head, not a boolean
> > flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> > instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> > LL_RESERVED_SPACE_EX(dev, 0) == 0),
>
> Hmm. I have:
>
> LL_RESERVED_SPACE_EX(dev, 0)
>         ((((hlen) + READ_ONCE((dev)->needed_headroom)) \
>           & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)
>
> #define HH_DATA_MOD     16
>
> So how can LL_RESERVED_SPACE_EX(dev, 0) == 0 ?
>
>

In practice, skb->network_header was 0 on tun_get_user() (including the
reported reproducer), tun_xdp_one(), virtnet_receive_done(), and
raw_verify_header() because __alloc_skb() / __build_skb_around()
zero-initializes skb->network_header to 0 (unlike mac_header and
transport_header which are initialized to ~0U), and those callers invoked
virtio_net_hdr_*_to_skb() before setting skb->network_header.

More generally, because 0 is both the initial value from alloc_skb() and a
valid offset whenever skb_headroom(skb) == 0, skb->network_header cannot
be used as a boolean to test whether the network header was initialized.

I can send a v2 with the corrected commit message if preferred, the
patch stays the same.


> > skb_reset_network_header(skb)
> > legitimately sets skb->network_header to 0.
> >
> > Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
> > the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
> > for TCPv4) without dissecting the packet, without validating ip_proto or
> > n_proto, and without setting skb->transport_header. If an IPv4 packet
> > carries IP options (ihl > 5, up to 60 bytes) or an IPv6 packet carries
> > extension headers, pulling only nh_min_len + thlen leaves the TCP header
> > outside skb->head, causing tcp_hdrlen(skb) in skb_gso_transport_seglen()
> > to read out-of-bounds:
> >
> >   BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
> >   Read of size 1 at addr ffff888103c7ec4d by task repro/5718
> >   Call Trace:
> >    <TASK>
> >    skb_gso_transport_seglen+0x173/0x200 net/core/skbuff.c:5503
> >    skb_gso_network_seglen include/linux/skbuff.h:4718 [inline]
> >    skb_gso_validate_network_len+0x92/0x1a0 net/core/skbuff.c:5566
> >    ip_finish_output_gso net/ipv4/ip_output.c:285 [inline]
> >    __ip_finish_output+0x28b/0x4a0 net/ipv4/ip_output.c:316
> >
> > In addition, when skb->protocol is pre-set by a caller before
> > __virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
> > virtio_net_hdr_match_proto() was not checked.
> >
> > Fix this by:
> > 1. Initializing skb->dev and skb->network_header (plus skb->protocol for
> >    IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
> >    tun_xdp_one(), virtnet_receive_done(), and raw_verify_header().
> > 2. Removing '&& skb->network_header' and the unvalidated
> >    'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
> >    packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
> >    their transport header pulled into linear data, and have
> >    skb->transport_header set.
> > 3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
> >    after skb_flow_dissect_flow_keys_basic().
> >
> > Fixes: 9e8db5913264 ("net: avoid false positives in untrusted gso validation")
> > Fixes: d5be7f632bad ("net: validate untrusted gso packets without csum offload")
> > Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/
> > Assisted-by: LLM
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Willem de Bruijn <willemb@google.com>
> > Cc: Jason Wang <jasowang@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  arch/um/drivers/vector_transports.c |  1 +
> >  drivers/net/tun.c                   | 23 ++++++----
> >  drivers/net/virtio_net.c            |  2 +
> >  include/linux/virtio_net.h          | 66 ++++++++++++++---------------
> >  4 files changed, 48 insertions(+), 44 deletions(-)
> >
> > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> > index ddd127ee96785daa4c485b2a06f078686efd2046..e1fb2a76fdff59ef3032d091be06a06fa58046cc 100644
> > --- a/arch/um/drivers/vector_transports.c
> > +++ b/arch/um/drivers/vector_transports.c
> > @@ -209,6 +209,7 @@ static int raw_verify_header(
> >       if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
> >               return 1;
> >
> > +     skb_set_network_header(skb, ETH_HLEN);
> >       virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
> >       return 0;
> >  }
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 5a302709a68aa3308b0c850b4a2957df3260b352..242899f7fd0711c5c59ff85accbf5bd1be9c6f39 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -1897,12 +1897,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >               }
> >       }
> >
> > -     if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> > -             atomic_long_inc(&tun->rx_frame_errors);
> > -             err = -EINVAL;
> > -             goto free_skb;
> > -     }
> > -
> > +     skb->dev = tun->dev;
> >       switch (tun->flags & TUN_TYPE_MASK) {
> >       case IFF_TUN:
> >               if (tun->flags & IFF_NO_PI) {
> > @@ -1927,9 +1922,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >                       }
> >               }
> >
> > -             skb_reset_mac_header(skb);
> > +             skb_reset_network_header(skb);
> >               skb->protocol = pi.proto;
> > -             skb->dev = tun->dev;
> >               break;
> >       case IFF_TAP:
> >               if (!pskb_may_pull(skb, ETH_HLEN)) {
> > @@ -1937,10 +1931,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >                       drop_reason = SKB_DROP_REASON_HDR_TRUNC;
> >                       goto drop;
> >               }
> > -             skb->protocol = eth_type_trans(skb, tun->dev);
> > +             skb_set_network_header(skb, ETH_HLEN);
> >               break;
> >       }
> >
> > +     if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> > +             atomic_long_inc(&tun->rx_frame_errors);
> > +             err = -EINVAL;
> > +             goto free_skb;
> > +     }
> > +
> > +     if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
> > +             skb->protocol = eth_type_trans(skb, tun->dev);
> > +
> >       /* copy skb_ubuf_info for callback when skb has no error */
> >       if (zerocopy) {
> >               skb_zcopy_init(skb, msg_control);
> > @@ -2600,6 +2603,8 @@ static int tun_xdp_one(struct tun_struct *tun,
> >
> >       features = tun_vnet_hdr_guest_features(READ_ONCE(tun->vnet_hdr_sz));
> >       tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
> > +     skb->dev = tun->dev;
> > +     skb_set_network_header(skb, ETH_HLEN);
> >       if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
> >               atomic_long_inc(&tun->rx_frame_errors);
> >               kfree_skb(skb);
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index bf82ef9874abb4094931496fb6064a12ee75b789..daab43ac92ce4276b0f8f88684991e3993022ce6 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2515,6 +2515,8 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
> >               goto frame_err;
> >       }
> >
> > +     skb->dev = dev;
> > +     skb_set_network_header(skb, ETH_HLEN);
> >       if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
> >                                     vi->rx_tnl_csum,
> >                                     virtio_is_little_endian(vi->vdev))) {
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index c381b916c1b54afacba5473888af589a24fe087c..8902d5a5c418c46af0a6e7b26ae89948c20b0b54 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -111,48 +111,44 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> >               p_off = nh_min_len + thlen;
> >               if (!pskb_may_pull(skb, p_off))
> >                       return -EINVAL;
> > -     } else {
> > +     } else if (gso_type) {
> >               /* gso packets without NEEDS_CSUM do not set transport_offset.
> >                * probe and drop if does not match one of the above types.
> >                */
> > -             if (gso_type && skb->network_header) {
> > -                     struct flow_keys_basic keys;
> > -
> > -                     if (!skb->protocol) {
> > -                             __be16 protocol = dev_parse_header_protocol(skb);
> > -
> > -                             if (!protocol)
> > -                                     virtio_net_hdr_set_proto(skb, hdr);
> > -                             else if (!virtio_net_hdr_match_proto(protocol,
> > -                                                              hdr_gso_type))
> > -                                     return -EINVAL;
> > -                             else
> > -                                     skb->protocol = protocol;
> > -                     }
> > -retry:
> > -                     if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
> > -                                                           NULL, 0, 0, 0,
> > -                                                           0)) {
> > -                             /* UFO does not specify ipv4 or 6: try both */
> > -                             if (gso_type & SKB_GSO_UDP &&
> > -                                 skb->protocol == htons(ETH_P_IP)) {
> > -                                     skb->protocol = htons(ETH_P_IPV6);
> > -                                     goto retry;
> > -                             }
> > -                             return -EINVAL;
> > -                     }
> > +             struct flow_keys_basic keys;
> >
> > -                     p_off = keys.control.thoff + thlen;
> > -                     if (!pskb_may_pull(skb, p_off) ||
> > -                         keys.basic.ip_proto != ip_proto)
> > -                             return -EINVAL;
> > +             if (!skb->protocol) {
> > +                     __be16 protocol = dev_parse_header_protocol(skb);
> >
> > -                     skb_set_transport_header(skb, keys.control.thoff);
> > -             } else if (gso_type) {
> > -                     p_off = nh_min_len + thlen;
> > -                     if (!pskb_may_pull(skb, p_off))
> > +                     if (!protocol)
> > +                             virtio_net_hdr_set_proto(skb, hdr);
> > +                     else if (!virtio_net_hdr_match_proto(protocol,
> > +                                                          hdr_gso_type))
> >                               return -EINVAL;
> > +                     else
> > +                             skb->protocol = protocol;
> >               }
> > +retry:
> > +             if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
> > +                                                   NULL, 0, 0, 0,
> > +                                                   0)) {
> > +                     /* UFO does not specify ipv4 or 6: try both */
> > +                     if (gso_type & SKB_GSO_UDP &&
> > +                         skb->protocol == htons(ETH_P_IP)) {
> > +                             skb->protocol = htons(ETH_P_IPV6);
> > +                             goto retry;
> > +                     }
> > +                     return -EINVAL;
> > +             }
> > +
> > +             p_off = keys.control.thoff + thlen;
> > +             if (!pskb_may_pull(skb, p_off) ||
> > +                 keys.basic.ip_proto != ip_proto ||
> > +                 !virtio_net_hdr_match_proto(keys.basic.n_proto,
> > +                                             hdr_gso_type))
> > +                     return -EINVAL;
> > +
> > +             skb_set_transport_header(skb, keys.control.thoff);
> >       }
> >
> >       if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> > --
> > 2.56.0.rc1.315.gc6ed9934b7-goog
>

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

On Mon, Sep 28, 2026 at 8:22 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> On Mon, Sep 28, 2026 at 3:31 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Sun, Sep 27, 2026 at 07:55:36PM +0000, Eric Dumazet wrote:
> > > Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> > > validation") added a '&& skb->network_header' check before flow-dissecting
> > > GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> > > __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> > > tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> > > virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> > > skb->dev.
> > >
> > > However, skb->network_header is an offset from skb->head, not a boolean
> > > flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> > > instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> > > LL_RESERVED_SPACE_EX(dev, 0) == 0),
> >
> > Hmm. I have:
> >
> > LL_RESERVED_SPACE_EX(dev, 0)
> >         ((((hlen) + READ_ONCE((dev)->needed_headroom)) \
> >           & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)
> >
> > #define HH_DATA_MOD     16
> >
> > So how can LL_RESERVED_SPACE_EX(dev, 0) == 0 ?
> >
> >
>
> In practice, skb->network_header was 0 on tun_get_user() (including the
> reported reproducer), tun_xdp_one(), virtnet_receive_done(), and
> raw_verify_header() because __alloc_skb() / __build_skb_around()
> zero-initializes skb->network_header to 0 (unlike mac_header and
> transport_header which are initialized to ~0U), and those callers invoked
> virtio_net_hdr_*_to_skb() before setting skb->network_header.
>
> More generally, because 0 is both the initial value from alloc_skb() and a
> valid offset whenever skb_headroom(skb) == 0, skb->network_header cannot
> be used as a boolean to test whether the network header was initialized.
>
> I can send a v2 with the corrected commit message if preferred, the
> patch stays the same.

Revised changelog would look like this, let me know if it looks ok this time.

net: always dissect GSO packets in __virtio_net_hdr_to_skb()

Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
validation") added a '&& skb->network_header' check before flow-dissecting
GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
__virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
virtio_net_hdr_*_to_skb() before initializing skb->network_header and
skb->dev.

Because __alloc_skb() and __build_skb_around() zero-initialize
skb->network_header to 0 (unlike mac_header and transport_header which
are initialized to ~0U), those four callers always had
skb->network_header == 0 and bypassed flow dissection in
__virtio_net_hdr_to_skb(). More generally, skb->network_header is an
offset from skb->head (where 0 is also a valid offset whenever
skb_headroom(skb) is 0), not a boolean flag.

Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
for TCPv4) without dissecting the packet, without validating ip_proto or
n_proto, and without setting skb->transport_header.

If the packet has a malformed network header, it is not rejected and a
subsequent skb_probe_transport_header() also fails, leaving
skb->transport_header at ~0U (0xffff). Similarly, if an IPv4 packet
carries IP options (ihl > 5) or an IPv6 packet carries extension headers,
pulling only nh_min_len + thlen can leave the TCP header outside
skb->head. In both cases, tcp_hdrlen(skb) in skb_gso_transport_seglen()
reads out-of-bounds:

  BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen
  Read of size 2 by task poc/133
  skb_gso_transport_seglen (net/core/gso.c:155)
  skb_gso_validate_mac_len (net/core/gso.c:270)
  tbf_enqueue (net/sched/sch_tbf.c:260)
  dev_qdisc_enqueue (net/core/dev.c:4227)
  __dev_queue_xmit (net/core/dev.c:4884)

In addition, when skb->protocol is pre-set by a caller before
__virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
virtio_net_hdr_match_proto() was not checked.

Fix this by:
1. Initializing skb->dev and skb->network_header (plus skb->protocol for
   IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
   tun_xdp_one(), virtnet_receive_done(), and raw_verify_header(). In
   tun_get_user(), drop the redundant skb_reset_mac_header(skb) in the
   IFF_TUN case since __virtio_net_hdr_to_skb() unconditionally resets
   mac_header.
2. Removing '&& skb->network_header' and the unvalidated
   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
   packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
   their transport header pulled into linear data, and have
   skb->transport_header set.
3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
   after skb_flow_dissect_flow_keys_basic().

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Michael S. Tsirkin]

On Mon, Sep 28, 2026 at 08:31:53AM +0200, Eric Dumazet wrote:
> On Mon, Sep 28, 2026 at 8:22 AM Eric Dumazet <edumazet@kernel.org> wrote:
> >
> > On Mon, Sep 28, 2026 at 3:31 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Sun, Sep 27, 2026 at 07:55:36PM +0000, Eric Dumazet wrote:
> > > > Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> > > > validation") added a '&& skb->network_header' check before flow-dissecting
> > > > GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> > > > __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> > > > tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> > > > virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> > > > skb->dev.
> > > >
> > > > However, skb->network_header is an offset from skb->head, not a boolean
> > > > flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> > > > instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> > > > LL_RESERVED_SPACE_EX(dev, 0) == 0),
> > >
> > > Hmm. I have:
> > >
> > > LL_RESERVED_SPACE_EX(dev, 0)
> > >         ((((hlen) + READ_ONCE((dev)->needed_headroom)) \
> > >           & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)
> > >
> > > #define HH_DATA_MOD     16
> > >
> > > So how can LL_RESERVED_SPACE_EX(dev, 0) == 0 ?
> > >
> > >
> >
> > In practice, skb->network_header was 0 on tun_get_user() (including the
> > reported reproducer), tun_xdp_one(), virtnet_receive_done(), and
> > raw_verify_header() because __alloc_skb() / __build_skb_around()
> > zero-initializes skb->network_header to 0 (unlike mac_header and
> > transport_header which are initialized to ~0U), and those callers invoked
> > virtio_net_hdr_*_to_skb() before setting skb->network_header.
> >
> > More generally, because 0 is both the initial value from alloc_skb() and a
> > valid offset whenever skb_headroom(skb) == 0, skb->network_header cannot
> > be used as a boolean to test whether the network header was initialized.
> >
> > I can send a v2 with the corrected commit message if preferred, the
> > patch stays the same.
> 
> Revised changelog would look like this, let me know if it looks ok this time.
> 
> net: always dissect GSO packets in __virtio_net_hdr_to_skb()
> 
> Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> validation") added a '&& skb->network_header' check before flow-dissecting
> GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> skb->dev.
> 
> Because __alloc_skb() and __build_skb_around() zero-initialize
> skb->network_header to 0 (unlike mac_header and transport_header which
> are initialized to ~0U), those four callers always had
> skb->network_header == 0 and bypassed flow dissection in
> __virtio_net_hdr_to_skb(). More generally, skb->network_header is an
> offset from skb->head (where 0 is also a valid offset whenever
> skb_headroom(skb) is 0), not a boolean flag.
> 
> Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
> the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
> for TCPv4) without dissecting the packet, without validating ip_proto or
> n_proto, and without setting skb->transport_header.
> 
> If the packet has a malformed network header, it is not rejected and a
> subsequent skb_probe_transport_header() also fails, leaving
> skb->transport_header at ~0U (0xffff). Similarly, if an IPv4 packet
> carries IP options (ihl > 5) or an IPv6 packet carries extension headers,
> pulling only nh_min_len + thlen can leave the TCP header outside
> skb->head. In both cases, tcp_hdrlen(skb) in skb_gso_transport_seglen()
> reads out-of-bounds:
> 
>   BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen
>   Read of size 2 by task poc/133
>   skb_gso_transport_seglen (net/core/gso.c:155)
>   skb_gso_validate_mac_len (net/core/gso.c:270)
>   tbf_enqueue (net/sched/sch_tbf.c:260)
>   dev_qdisc_enqueue (net/core/dev.c:4227)
>   __dev_queue_xmit (net/core/dev.c:4884)
> 
> In addition, when skb->protocol is pre-set by a caller before
> __virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
> virtio_net_hdr_match_proto() was not checked.
> 
> Fix this by:
> 1. Initializing skb->dev and skb->network_header (plus skb->protocol for
>    IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
>    tun_xdp_one(), virtnet_receive_done(), and raw_verify_header(). In
>    tun_get_user(), drop the redundant skb_reset_mac_header(skb) in the
>    IFF_TUN case since __virtio_net_hdr_to_skb() unconditionally resets
>    mac_header.
> 2. Removing '&& skb->network_header' and the unvalidated
>    'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
>    packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
>    their transport header pulled into linear data, and have
>    skb->transport_header set.
> 3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
>    after skb_flow_dissect_flow_keys_basic().

I'm travelling for a week, if possible I'd like a bit of time to review this.

For now, I also asked Claude to find cases where this patch causes any
UAPI change (because if it does, there's a risk it will break some
userspace, right?).  It wrote the below test, which accepts a packet
without your patch and fails with, and claims this packet is valid and
was previously accepted. I tested it quickly and it seems to be true but I can't
analyze it now as I'm sleep deprived due to travel)

--->


/*
 * test_vlan_gso.c - Regression reproducer: VLAN-tagged GSO without NEEDS_CSUM
 *
 * Writes a VLAN-tagged TCPv4 GSO packet (no NEEDS_CSUM) to a TAP device.
 * Accepted without patch, rejected with patch = regression.
 *
 * Validates packet correctness with hex dump, memcmp, and pcap output.
 *
 * Packet layout (virtio_net_hdr + ethernet frame):
 *
 *   virtio_net_hdr (10 bytes):
 *     00/02        flags (0 = none, or 02 = DATA_VALID)
 *     01        gso_type = VIRTIO_NET_HDR_GSO_TCPV4
 *     3a 00     hdr_len = 58 (14 eth + 4 vlan + 20 ip + 20 tcp)
 *     78 05     gso_size = 1400
 *     00 00     csum_start (unused, no NEEDS_CSUM)
 *     00 00     csum_offset (unused)
 *
 *   Ethernet + VLAN (18 bytes):
 *     02:02:02:02:02:02  dst MAC
 *     04:04:04:04:04:04  src MAC
 *     81 00              ethertype = 802.1Q
 *     00 64              VLAN TCI: VID=100
 *     08 00              inner ethertype = IPv4
 *
 *   IPv4 (20 bytes):
 *     45 00 0b 18        v4, IHL=5, tot_len=2840
 *     00 00 00 00        id=0, flags=0, frag_off=0
 *     40 06              TTL=64, proto=TCP
 *     5b de              IP checksum (correct)
 *     0a 00 00 01        src = 10.0.0.1
 *     0a 00 00 02        dst = 10.0.0.2
 *
 *   TCP (20 bytes):
 *     30 39 00 50        sport=12345, dport=80
 *     00 00 00 01        seq=1
 *     00 00 00 00        ack=0
 *     50 10 ff ff        doff=5, flags=ACK, win=65535
 *     83 7b 00 00        checksum (correct), urgent=0
 *
 *   Payload: 2800 bytes of 'A' (0x41)
 */
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/mount.h>
#include <sys/reboot.h>
#include <net/if.h>
#include <linux/if_tun.h>
#include <linux/virtio_net.h>
#include <linux/if_ether.h>
#include <linux/ip.h>
#include <linux/tcp.h>
#include <linux/reboot.h>
#include <arpa/inet.h>
#include <stdint.h>

static unsigned int csum_add(const void *data, int len, unsigned int initial)
{
	const unsigned short *p = data;
	unsigned int sum = initial;

	while (len > 1) {
		sum += *p++;
		len -= 2;
	}
	if (len)
		sum += *(const unsigned char *)p;
	return sum;
}

static unsigned short csum_fold(unsigned int sum)
{
	sum = (sum >> 16) + (sum & 0xffff);
	sum += sum >> 16;
	return ~sum;
}

static unsigned short ip_csum(const void *data, int len)
{
	return csum_fold(csum_add(data, len, 0));
}

static unsigned short tcp_csum(struct iphdr *iph, struct tcphdr *tcph,
			       const void *payload, int payload_len)
{
	struct {
		uint32_t saddr;
		uint32_t daddr;
		uint8_t zero;
		uint8_t protocol;
		uint16_t tcp_len;
	} __attribute__((packed)) pseudo;
	unsigned int sum;
	int tcp_len = sizeof(struct tcphdr) + payload_len;

	pseudo.saddr = iph->saddr;
	pseudo.daddr = iph->daddr;
	pseudo.zero = 0;
	pseudo.protocol = IPPROTO_TCP;
	pseudo.tcp_len = htons(tcp_len);

	sum = csum_add(&pseudo, sizeof(pseudo), 0);
	sum = csum_add(tcph, sizeof(struct tcphdr), sum);
	sum = csum_add(payload, payload_len, sum);
	return csum_fold(sum);
}

static void hexdump(const char *label, const unsigned char *data, int len)
{
	int i;

	printf("%s (%d bytes):\n", label, len);
	for (i = 0; i < len; i++) {
		if (i % 16 == 0)
			printf("  %04x: ", i);
		printf("%02x ", data[i]);
		if (i % 16 == 15 || i == len - 1)
			printf("\n");
	}
}

static void write_pcap(const char *path, const unsigned char *pkt, int len)
{
	FILE *f;
	uint32_t val32;
	uint16_t val16;
	uint32_t pkt_hdr[4];

	f = fopen(path, "wb");
	if (!f) {
		printf("cannot write pcap: %s\n", strerror(errno));
		return;
	}

	/* pcap global header */
	val32 = 0xa1b2c3d4; fwrite(&val32, 4, 1, f); /* magic */
	val16 = 2;           fwrite(&val16, 2, 1, f); /* version major */
	val16 = 4;           fwrite(&val16, 2, 1, f); /* version minor */
	val32 = 0;           fwrite(&val32, 4, 1, f); /* thiszone */
	val32 = 0;           fwrite(&val32, 4, 1, f); /* sigfigs */
	val32 = 65535;       fwrite(&val32, 4, 1, f); /* snaplen */
	val32 = 1;           fwrite(&val32, 4, 1, f); /* network (LINKTYPE_ETHERNET) */

	/* packet header */
	pkt_hdr[0] = 0;     /* ts_sec */
	pkt_hdr[1] = 0;     /* ts_usec */
	pkt_hdr[2] = len;   /* incl_len */
	pkt_hdr[3] = len;   /* orig_len */
	fwrite(pkt_hdr, sizeof(pkt_hdr), 1, f);

	/* packet data */
	fwrite(pkt, 1, len, f);
	fclose(f);
	printf("wrote pcap: %s\n", path);
}

static int validate_packet(const unsigned char *frame, int frame_len,
			    int ip_off, int tcp_off, int pay_off)
{
	struct ethhdr *eth = (struct ethhdr *)frame;
	struct iphdr *iph = (struct iphdr *)(frame + ip_off);
	struct tcphdr *tcph = (struct tcphdr *)(frame + tcp_off);
	int payload_len = frame_len - pay_off;
	unsigned short got, expect;
	int ok = 1;

	/* ETH */
	if (ntohs(eth->h_proto) != ETH_P_8021Q) {
		printf("  MISMATCH: ethertype %04x != 8021Q\n",
		       ntohs(eth->h_proto));
		ok = 0;
	}

	/* VLAN: inner ethertype */
	if (frame[sizeof(struct ethhdr) + 2] != 0x08 ||
	    frame[sizeof(struct ethhdr) + 3] != 0x00) {
		printf("  MISMATCH: inner ethertype %02x%02x != 0800\n",
		       frame[sizeof(struct ethhdr) + 2],
		       frame[sizeof(struct ethhdr) + 3]);
		ok = 0;
	}

	/* IP checksum */
	got = iph->check;
	iph->check = 0;
	expect = ip_csum(iph, iph->ihl * 4);
	iph->check = got;
	if (got != expect) {
		printf("  MISMATCH: IP csum %04x != %04x\n",
		       ntohs(got), ntohs(expect));
		ok = 0;
	}

	/* TCP checksum */
	got = tcph->check;
	tcph->check = 0;
	expect = tcp_csum(iph, tcph, frame + pay_off, payload_len);
	tcph->check = got;
	if (got != expect) {
		printf("  MISMATCH: TCP csum %04x != %04x\n",
		       ntohs(got), ntohs(expect));
		ok = 0;
	}

	/* IP header fields */
	if (iph->version != 4 || iph->ihl != 5) {
		printf("  MISMATCH: IP ver/ihl %d/%d\n",
		       iph->version, iph->ihl);
		ok = 0;
	}
	if (iph->protocol != IPPROTO_TCP) {
		printf("  MISMATCH: IP proto %d != TCP\n", iph->protocol);
		ok = 0;
	}
	if (ntohs(iph->tot_len) != frame_len - ip_off) {
		printf("  MISMATCH: IP tot_len %d != %d\n",
		       ntohs(iph->tot_len), frame_len - ip_off);
		ok = 0;
	}

	/* TCP header */
	if (tcph->doff != 5) {
		printf("  MISMATCH: TCP doff %d != 5\n", tcph->doff);
		ok = 0;
	}

	if (ok)
		printf("  packet validation: OK\n");
	return ok;
}

int main(void)
{
	unsigned char buf[4096];
	struct virtio_net_hdr *vhdr;
	struct ethhdr *eth;
	struct iphdr *iph;
	struct tcphdr *tcph;
	struct ifreq ifr;
	int fd, sock, ret;
	int payload_len = 2800;

	int vhdr_off = 0;
	int eth_off = sizeof(struct virtio_net_hdr);
	int vlan_off = eth_off + sizeof(struct ethhdr);
	int ip_off = vlan_off + 4;
	int tcp_off = ip_off + sizeof(struct iphdr);
	int pay_off = tcp_off + sizeof(struct tcphdr);
	int total = pay_off + payload_len;

	/* frame offsets (without virtio_net_hdr) */
	int f_ip_off = ip_off - eth_off;
	int f_tcp_off = tcp_off - eth_off;
	int f_pay_off = pay_off - eth_off;
	int frame_len = total - eth_off;

	mount("proc", "/proc", "proc", 0, NULL);
	mount("sysfs", "/sys", "sysfs", 0, NULL);
	mount("devtmpfs", "/dev", "devtmpfs", 0, NULL);

	memset(buf, 0, total);

	/* virtio_net_hdr - filled per test below */
	vhdr = (struct virtio_net_hdr *)(buf + vhdr_off);

	/* Ethernet header */
	eth = (struct ethhdr *)(buf + eth_off);
	memset(eth->h_dest, 0x02, ETH_ALEN);
	memset(eth->h_source, 0x04, ETH_ALEN);
	eth->h_proto = htons(ETH_P_8021Q);

	/* 802.1Q: VID=100, inner ethertype=IPv4 */
	buf[vlan_off + 0] = 0x00;
	buf[vlan_off + 1] = 0x64;
	buf[vlan_off + 2] = 0x08;
	buf[vlan_off + 3] = 0x00;

	/* IP header */
	iph = (struct iphdr *)(buf + ip_off);
	iph->ihl = 5;
	iph->version = 4;
	iph->tot_len = htons(sizeof(struct iphdr) + sizeof(struct tcphdr) +
			     payload_len);
	iph->ttl = 64;
	iph->protocol = IPPROTO_TCP;
	iph->saddr = htonl(0x0a000001);
	iph->daddr = htonl(0x0a000002);
	iph->check = ip_csum(iph, sizeof(struct iphdr));

	/* TCP header */
	tcph = (struct tcphdr *)(buf + tcp_off);
	tcph->source = htons(12345);
	tcph->dest = htons(80);
	tcph->seq = htonl(1);
	tcph->doff = sizeof(struct tcphdr) / 4;
	tcph->ack = 1;
	tcph->window = htons(65535);

	/* Payload */
	memset(buf + pay_off, 'A', payload_len);

	/* TCP checksum over pseudo-header + TCP header + payload */
	tcph->check = tcp_csum(iph, tcph, buf + pay_off, payload_len);

	/* Set virtio_net_hdr GSO fields for dump (flags adjusted per test) */
	vhdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
	vhdr->gso_size = 1400;
	vhdr->hdr_len = tcp_off - eth_off + sizeof(struct tcphdr);

	/* Validate and dump */
	printf("\n=== Packet validation ===\n");
	validate_packet(buf + eth_off, frame_len, f_ip_off, f_tcp_off, f_pay_off);
	hexdump("virtio_net_hdr (flags=0)", buf, sizeof(struct virtio_net_hdr));
	hexdump("Ethernet frame (first 80 bytes)",
		buf + eth_off, frame_len < 80 ? frame_len : 80);
	write_pcap("/tmp/vlan_gso.pcap", buf + eth_off, frame_len);

	/* Open TAP */
	fd = open("/dev/net/tun", O_RDWR);
	if (fd < 0) {
		perror("open tun");
		goto fail;
	}
	memset(&ifr, 0, sizeof(ifr));
	ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
	strncpy(ifr.ifr_name, "tap0", IFNAMSIZ - 1);
	if (ioctl(fd, TUNSETIFF, &ifr) < 0) {
		perror("TUNSETIFF");
		goto fail;
	}
	sock = socket(AF_INET, SOCK_DGRAM, 0);
	memset(&ifr, 0, sizeof(ifr));
	strncpy(ifr.ifr_name, "tap0", IFNAMSIZ - 1);
	ifr.ifr_flags = IFF_UP;
	ioctl(sock, SIOCSIFFLAGS, &ifr);
	close(sock);

	/* Test 1: flags=0 (no NEEDS_CSUM, no DATA_VALID) */
	vhdr->flags = 0;

	printf("\n=== TAP write tests ===\n");
	ret = write(fd, buf, total);
	if (ret == total)
		printf("PASS: VLAN GSO flags=0 accepted (%d bytes)\n", ret);
	else
		printf("FAIL: VLAN GSO flags=0 rejected: %s\n", strerror(errno));

	/* Test 2: flags=DATA_VALID (realistic: LRO -> bridge -> TAP) */
	vhdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;

	ret = write(fd, buf, total);
	if (ret == total)
		printf("PASS: VLAN GSO DATA_VALID accepted (%d bytes)\n", ret);
	else
		printf("FAIL: VLAN GSO DATA_VALID rejected: %s\n", strerror(errno));

	close(fd);
	sync();
	reboot(LINUX_REBOOT_CMD_POWER_OFF);
	return 0;

fail:
	sync();
	reboot(LINUX_REBOOT_CMD_POWER_OFF);
	return 1;
}

-- 
MST

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

On Mon, Sep 28, 2026 at 10:10 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Sep 28, 2026 at 08:31:53AM +0200, Eric Dumazet wrote:
> > On Mon, Sep 28, 2026 at 8:22 AM Eric Dumazet <edumazet@kernel.org> wrote:
> > >
> > > On Mon, Sep 28, 2026 at 3:31 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Sun, Sep 27, 2026 at 07:55:36PM +0000, Eric Dumazet wrote:
> > > > > Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> > > > > validation") added a '&& skb->network_header' check before flow-dissecting
> > > > > GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> > > > > __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> > > > > tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> > > > > virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> > > > > skb->dev.
> > > > >
> > > > > However, skb->network_header is an offset from skb->head, not a boolean
> > > > > flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> > > > > instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> > > > > LL_RESERVED_SPACE_EX(dev, 0) == 0),
> > > >
> > > > Hmm. I have:
> > > >
> > > > LL_RESERVED_SPACE_EX(dev, 0)
> > > >         ((((hlen) + READ_ONCE((dev)->needed_headroom)) \
> > > >           & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)
> > > >
> > > > #define HH_DATA_MOD     16
> > > >
> > > > So how can LL_RESERVED_SPACE_EX(dev, 0) == 0 ?
> > > >
> > > >
> > >
> > > In practice, skb->network_header was 0 on tun_get_user() (including the
> > > reported reproducer), tun_xdp_one(), virtnet_receive_done(), and
> > > raw_verify_header() because __alloc_skb() / __build_skb_around()
> > > zero-initializes skb->network_header to 0 (unlike mac_header and
> > > transport_header which are initialized to ~0U), and those callers invoked
> > > virtio_net_hdr_*_to_skb() before setting skb->network_header.
> > >
> > > More generally, because 0 is both the initial value from alloc_skb() and a
> > > valid offset whenever skb_headroom(skb) == 0, skb->network_header cannot
> > > be used as a boolean to test whether the network header was initialized.
> > >
> > > I can send a v2 with the corrected commit message if preferred, the
> > > patch stays the same.
> >
> > Revised changelog would look like this, let me know if it looks ok this time.
> >
> > net: always dissect GSO packets in __virtio_net_hdr_to_skb()
> >
> > Commit 9e8db5913264 ("net: avoid false positives in untrusted gso
> > validation") added a '&& skb->network_header' check before flow-dissecting
> > GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in
> > __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(),
> > tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called
> > virtio_net_hdr_*_to_skb() before initializing skb->network_header and
> > skb->dev.
> >
> > Because __alloc_skb() and __build_skb_around() zero-initialize
> > skb->network_header to 0 (unlike mac_header and transport_header which
> > are initialized to ~0U), those four callers always had
> > skb->network_header == 0 and bypassed flow dissection in
> > __virtio_net_hdr_to_skb(). More generally, skb->network_header is an
> > offset from skb->head (where 0 is also a valid offset whenever
> > skb_headroom(skb) is 0), not a boolean flag.
> >
> > Whenever the 'if (gso_type && skb->network_header)' branch was skipped,
> > the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes
> > for TCPv4) without dissecting the packet, without validating ip_proto or
> > n_proto, and without setting skb->transport_header.
> >
> > If the packet has a malformed network header, it is not rejected and a
> > subsequent skb_probe_transport_header() also fails, leaving
> > skb->transport_header at ~0U (0xffff). Similarly, if an IPv4 packet
> > carries IP options (ihl > 5) or an IPv6 packet carries extension headers,
> > pulling only nh_min_len + thlen can leave the TCP header outside
> > skb->head. In both cases, tcp_hdrlen(skb) in skb_gso_transport_seglen()
> > reads out-of-bounds:
> >
> >   BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen
> >   Read of size 2 by task poc/133
> >   skb_gso_transport_seglen (net/core/gso.c:155)
> >   skb_gso_validate_mac_len (net/core/gso.c:270)
> >   tbf_enqueue (net/sched/sch_tbf.c:260)
> >   dev_qdisc_enqueue (net/core/dev.c:4227)
> >   __dev_queue_xmit (net/core/dev.c:4884)
> >
> > In addition, when skb->protocol is pre-set by a caller before
> > __virtio_net_hdr_to_skb(), 'if (!skb->protocol)' is skipped and
> > virtio_net_hdr_match_proto() was not checked.
> >
> > Fix this by:
> > 1. Initializing skb->dev and skb->network_header (plus skb->protocol for
> >    IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(),
> >    tun_xdp_one(), virtnet_receive_done(), and raw_verify_header(). In
> >    tun_get_user(), drop the redundant skb_reset_mac_header(skb) in the
> >    IFF_TUN case since __virtio_net_hdr_to_skb() unconditionally resets
> >    mac_header.
> > 2. Removing '&& skb->network_header' and the unvalidated
> >    'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO
> >    packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have
> >    their transport header pulled into linear data, and have
> >    skb->transport_header set.
> > 3. Validating virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type)
> >    after skb_flow_dissect_flow_keys_basic().
>
> I'm travelling for a week, if possible I'd like a bit of time to review this.
>
> For now, I also asked Claude to find cases where this patch causes any
> UAPI change (because if it does, there's a risk it will break some
> userspace, right?).
> It wrote the below test, which accepts a packet
> without your patch and fails with, and claims this packet is valid and
> was previously accepted. I tested it quickly and it seems to be true but I can't
> analyze it now as I'm sleep deprived due to travel)

Thanks for the reproducer!

For a VLAN-tagged Ethernet frame, dev_parse_header_protocol(skb) returns
the outer VLAN EtherType (ETH_P_8021Q / ETH_P_8021AD), not the inner L3
protocol.

Because virtio_net_hdr_match_proto() only matches ETH_P_IP and
ETH_P_IPV6, checking virtio_net_hdr_match_proto(protocol, hdr_gso_type)
before skb_flow_dissect_flow_keys_basic() rejects VLAN-tagged GSO
packets without NEEDS_CSUM whenever skb->protocol is not pre-set.

Previously, tun_get_user() (IFF_TAP) only avoided this check because
skb->network_header was 0 and the whole flow-dissection block was
_skipped_, exposing our stack to a variety of malicious packets.

Since the patch already validates
virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type) after
skb_flow_dissect_flow_keys_basic() (where keys.basic.n_proto holds the
inner L3 protocol after dissecting any 802.1Q / 802.1AD headers), we
should drop the pre-dissection virtio_net_hdr_match_proto() check:

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 8902d5a5c418c46af0a6e7b26ae89948c20b0b54..d6466f96cdd00cdf059d4fa8842ba1672780e556
100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -118,15 +118,9 @@ static inline int __virtio_net_hdr_to_skb(struct
sk_buff *skb,
                struct flow_keys_basic keys;

                if (!skb->protocol) {
-                       __be16 protocol = dev_parse_header_protocol(skb);
-
-                       if (!protocol)
+                       skb->protocol = dev_parse_header_protocol(skb);
+                       if (!skb->protocol)
                                virtio_net_hdr_set_proto(skb, hdr);
-                       else if (!virtio_net_hdr_match_proto(protocol,
-                                                            hdr_gso_type))
-                               return -EINVAL;
-                       else
-                               skb->protocol = protocol;
                }
 retry:
                if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,

I will fold this into v2, and  add your repro in a new case in
tools/testing/selftests/net/tun.c

Re: [PATCH net] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] net: uninitialized mac_header usage in
  __virtio_net_hdr_to_skb() causes OOB read
- [Medium] The commit message names the wrong trigger.
- [Medium] VLAN-tagged GSO frames without VIRTIO_NET_HDR_F_NEEDS_CSUM are
  now always dropped on the rerouted receive paths: tun IFF_TAP…

Pre-existing issues:
- [Low] UML raw_verify_header() now goes through the dissection branch
  (skb_set_network_header(skb, ETH_HLEN) at vector_transports.c:212),…

--

net: always dissect GSO packets in __virtio_net_hdr_to_skb()

This commit removes the skb->network_header condition in
__virtio_net_hdr_to_skb(), so every GSO packet without
VIRTIO_NET_HDR_F_NEEDS_CSUM is flow-dissected. It also adds a
virtio_net_hdr_match_proto() check on keys.basic.n_proto after dissection.
To support this, tun_get_user(), tun_xdp_one(), virtnet_receive_done() and
the UML raw_verify_header() now set skb->dev and skb->network_header before
they parse the vnet header.

Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/

> However, skb->network_header is an offset from skb->head, not a boolean
> flag. When skb_headroom(skb) is 0 on a device without L2 headers (for
> instance packet_snd() or tpacket_snd() on a tunnel/pure-L3 device where
> LL_RESERVED_SPACE_EX(dev, 0) == 0), skb_reset_network_header(skb)
> legitimately sets skb->network_header to 0.

[Severity: Medium]
Is this trigger accurate? LL_RESERVED_SPACE_EX() always adds HH_DATA_MOD
(16), so it looks like it can never be 0:

include/linux/netdevice.h:
#define LL_RESERVED_SPACE_EX(dev, hlen) \
	((((hlen) + READ_ONCE((dev)->needed_headroom)) \
	  & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)

tpacket_fill_skb() reserves that amount before it resets the network
header:

	skb_reserve(skb, hlen);
	skb_reset_network_header(skb);

packet_snd() also reserves hlen = LL_RESERVED_SPACE_EX(dev, hard_header_len)
before skb_reset_network_header(). The later skb_reserve(skb, -reserve)
moves back by only hard_header_len, and that is always less than hlen.
So on both AF_PACKET paths network_header seems to stay non-zero, and
those paths already took the dissect branch before this patch.

The callers that actually reached the unvalidated else if (gso_type)
fallback appear to be the ones that never set network_header, so it
stayed 0 from allocation: tun_get_user(), tun_xdp_one() (vhost-net),
virtnet_receive_done() and raw_verify_header(). These are the callers
this patch changes.

Could the commit message name those paths as the trigger instead? It
could also note that tun users, vhost-net guests and virtio devices can
reach the bug, rather than only CAP_NET_RAW AF_PACKET users.

> diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> index ddd127ee96785..e1fb2a76fdff5 100644
> --- a/arch/um/drivers/vector_transports.c
> +++ b/arch/um/drivers/vector_transports.c
> @@ -209,6 +209,7 @@ static int raw_verify_header(
>  	if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
>  		return 1;
>  
> +	skb_set_network_header(skb, ETH_HLEN);
>  	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
>  	return 0;
>  }

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch. Now
that raw_verify_header() enters the dissection branch, it is worth
noting that it runs on an untrimmed skb and ignores the result.

prep_skb() sizes the skb to the whole preallocated buffer:

arch/um/drivers/vector_kern.c:prep_skb() {
    ...
	skb_put(result, vp->max_packet);
	result->data_len = len - vp->max_packet;
    ...
}

vector_legacy_rx() and vector_mmsg_rx() call vp->verify_header() before
pskb_trim(). As a result, the pskb_may_pull(skb, keys.control.thoff +
thlen) check and the gso_size threshold check compare against the buffer
size, not the received packet length.

raw_verify_header() also returns 0 whatever virtio_net_hdr_to_skb()
returns. Frames that fail validation are therefore still delivered, and
they can keep partial state such as CHECKSUM_PARTIAL from
skb_partial_csum_set() when a later transport offset check fails.

The out-of-bounds read this patch fixes does not look reachable here.
The UML linear area is max_packet in size and is never reallocated. Also,
every -EINVAL return happens before gso_size and gso_type are set, so a
rejected frame is delivered as a plain non-GSO skb.

Should the verify_header() callback run after pskb_trim(), and should a
failing virtio_net_hdr_to_skb() result cause the frame to be dropped?

[ ... ]

> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index c381b916c1b54..8902d5a5c418c 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -111,48 +111,44 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
>  		p_off = nh_min_len + thlen;
>  		if (!pskb_may_pull(skb, p_off))
>  			return -EINVAL;
> -	} else {
> +	} else if (gso_type) {
>  		/* gso packets without NEEDS_CSUM do not set transport_offset.
>  		 * probe and drop if does not match one of the above types.
>  		 */

[ ... ]

> +		if (!skb->protocol) {
> +			__be16 protocol = dev_parse_header_protocol(skb);
>  
> -			skb_set_transport_header(skb, keys.control.thoff);
> -		} else if (gso_type) {
> -			p_off = nh_min_len + thlen;
> -			if (!pskb_may_pull(skb, p_off))
> +			if (!protocol)
> +				virtio_net_hdr_set_proto(skb, hdr);
> +			else if (!virtio_net_hdr_match_proto(protocol,
> +							     hdr_gso_type))
>  				return -EINVAL;
> +			else
> +				skb->protocol = protocol;
>  		}

[Severity: Critical]
Does this read an uninitialized mac_header on the rerouted receive
paths?

After this change, tun_get_user() sets skb->dev and network_header
before it calls tun_vnet_hdr_tnl_to_skb(). However, eth_type_trans()
now runs afterwards, so skb_reset_mac_header() has not been called yet
and skb->protocol is still 0. For a GSO frame without
VIRTIO_NET_HDR_F_NEEDS_CSUM, the dissect branch is now always taken.
With skb->dev set, dev_parse_header_protocol() reaches the Ethernet
parse_protocol callback:

tun_get_user()
  skb->dev = tun->dev;
  skb_set_network_header(skb, ETH_HLEN);   /* mac_header still ~0 */
  tun_vnet_hdr_tnl_to_skb()
    virtio_net_hdr_tnl_to_skb()
      virtio_net_hdr_to_skb()
        __virtio_net_hdr_to_skb()
          dev_parse_header_protocol()
            eth_header_parse_protocol()
              eth_hdr(skb)->h_proto   /* skb->head + 0xffff */

The skb comes straight from tun_alloc_skb() or tun_build_skb(), so
mac_header still holds the ~0 value set at allocation. eth_hdr()
therefore returns skb->head + 0xffff, and h_proto is read far past the
end of the linear buffer. With CONFIG_DEBUG_NET this should also trip
the skb_mac_header_was_set() warning in skb_mac_header().

Before this patch these callers had skb->dev == NULL and
network_header == 0, so they never reached
dev_parse_header_protocol().

tun_xdp_one() and virtnet_receive_done() have the same ordering. The
skb is built with build_skb() or the virtio_net receive helpers, and
mac_header is never set. The patch then sets skb->dev and
network_header, and eth_type_trans() only runs after
virtio_net_hdr_tnl_to_skb().

This looks reachable by anyone holding a tap fd with IFF_VNET_HDR, by
a vhost-net guest, and by the virtio-net device. The ethertype that
virtio_net_hdr_match_proto() compares against is also just whatever
happens to be at that address.

Would it be enough to call skb_reset_mac_header() together with
skb_set_network_header(skb, ETH_HLEN) in these callers?
eth_type_trans() resets it again afterwards anyway. Alternatively,
setting skb->protocol from the Ethernet header first, as
tap_get_user() does, would skip dev_parse_header_protocol()
altogether.

Could raw_verify_header() also be checked? It now sets network_header
but not mac_header. Depending on whether the UML receive path has set
skb->dev at that point, dev_parse_header_protocol() will either
dereference a NULL skb->dev or hit the same unset mac_header.

[Severity: Medium]
Will VLAN-tagged GSO frames without VIRTIO_NET_HDR_F_NEEDS_CSUM now be
dropped on the rerouted receive paths?

In tun_get_user() (IFF_TAP), tun_xdp_one(), virtnet_receive_done() and
raw_verify_header(), network_header and skb->dev are now set, but
skb->protocol is still 0 at this point, because eth_type_trans() runs
afterwards. For an in-band 802.1Q or 802.1AD frame the call chain looks
like this:

tun_get_user()
  skb_set_network_header(skb, ETH_HLEN);   /* skb->protocol == 0 */
  tun_vnet_hdr_tnl_to_skb()
    virtio_net_hdr_tnl_to_skb()
      virtio_net_hdr_to_skb()
        __virtio_net_hdr_to_skb()
          dev_parse_header_protocol()
            eth_header_parse_protocol()
              return eth->h_proto;   /* ETH_P_8021Q or ETH_P_8021AD */
          virtio_net_hdr_match_proto(protocol, hdr_gso_type) -> false
          return -EINVAL;

This rejection happens before skb_flow_dissect_flow_keys_basic() runs.
The dissector handles ETH_P_8021Q and ETH_P_8021AD and reports the inner
protocol in keys.basic.n_proto, so the new post-dissection check would
accept these frames:

	    !virtio_net_hdr_match_proto(keys.basic.n_proto,
					hdr_gso_type))

Before this patch, these callers had network_header == 0 and took the
else if (gso_type) fallback, which accepted such frames.

The peer callers behave differently. tap_get_user() and
tap_get_user_xdp() set the protocol beforehand, which skips the L2
pre-check:

	skb->protocol = eth_hdr(skb)->h_proto;

af_packet sets it in packet_parse_headers(). Both paths accept the same
frames, so macvtap and AF_PACKET accept input that tun, vhost-net and
virtio_net now reject.

On the rejecting paths, virtio_net counts the drop in rx_frame_errors,
and tun increments rx_frame_errors and returns -EINVAL.

Would it make sense to drop the pre-dissection ethertype match now that
n_proto is checked after dissection? Alternatively, the callers could set
skb->protocol from the Ethernet header first, as tap.c does.

The commit message also doesn't mention this change in behaviour.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927195536.2489079-1-edumazet%40google.com