[PATCH v2 net 0/2] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[PATCH v2 net 0/2] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

This series fixes a bypass of untrusted GSO flow dissection in
__virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:

- Patch 1 initializes skb->dev and skb->network_header before calling
  virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
  virtnet_receive_done(), and raw_verify_header(), removes the
  '&& skb->network_header' condition and the unvalidated
  'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
  virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
  so it validates the dissected L3 protocol (keys.basic.n_proto) rather
  than the outer L2 protocol.

- Patch 2 adds a kselftest in tools/testing/selftests/net/tun.c verifying
  that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
  flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
  TAP device, and that mismatched GSO types and truncated headers without
  NEEDS_CSUM are rejected with -EINVAL.

v2:
 - Patch 1: drop the pre-dissection virtio_net_hdr_match_proto() check
   inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
   NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
 - Patch 1: clarify the changelog regarding why skb->network_header was 0
   in those callers and why skb_reset_mac_header() is dropped in
   tun_get_user() for IFF_TUN (Michael S. Tsirkin).
 - Patch 2: add selftest in tools/testing/selftests/net/tun.c based on
   Michael's reproducer.
 - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/

Eric Dumazet (2):
  net: always dissect GSO packets in __virtio_net_hdr_to_skb()
  selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM

 arch/um/drivers/vector_transports.c |   1 +
 drivers/net/tun.c                   |  23 +++---
 drivers/net/virtio_net.c            |   2 +
 include/linux/virtio_net.h          |  60 +++++++---------
 tools/testing/selftests/net/tun.c   | 107 ++++++++++++++++++++++++++++
 5 files changed, 149 insertions(+), 44 deletions(-)

-- 
2.56.0.rc1.315.gc6ed9934b7-goog

[PATCH v2 net 1/2] 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.

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, checking virtio_net_hdr_match_proto() only inside
'if (!skb->protocol)' before flow dissection both skipped validation when
skb->protocol was pre-set by the caller and rejected VLAN-tagged frames
whose outer L2 protocol is ETH_P_8021Q or ETH_P_8021AD.

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. Moving the virtio_net_hdr_match_proto() check to after
   skb_flow_dissect_flow_keys_basic(), validating keys.basic.n_proto
   against hdr_gso_type.

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@kernel.org>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
 arch/um/drivers/vector_transports.c |  1 +
 drivers/net/tun.c                   | 23 ++++++-----
 drivers/net/virtio_net.c            |  2 +
 include/linux/virtio_net.h          | 60 ++++++++++++-----------------
 4 files changed, 42 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..d6466f96cdd00cdf059d4fa8842ba1672780e556 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -111,48 +111,38 @@ 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;
-			}
+		struct flow_keys_basic keys;
+
+		if (!skb->protocol) {
+			skb->protocol = dev_parse_header_protocol(skb);
+			if (!skb->protocol)
+				virtio_net_hdr_set_proto(skb, hdr);
+		}
 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;
+		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)
-				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);
-		} else if (gso_type) {
-			p_off = nh_min_len + thlen;
-			if (!pskb_may_pull(skb, p_off))
-				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

[PATCH v2 net 2/2] selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM

[Eric Dumazet]

Add a selftest in tun.c verifying that a VLAN-tagged (802.1Q) TCPv4 GSO
packet without VIRTIO_NET_HDR_F_NEEDS_CSUM (both flags = 0 and
flags = VIRTIO_NET_HDR_F_DATA_VALID) is accepted when written to a TAP
device (/dev/net/tun with IFF_TAP | IFF_NO_PI | IFF_VNET_HDR).

Also verify that mismatched GSO types (e.g. VIRTIO_NET_HDR_GSO_TCPV6 on
a VLAN-tagged IPv4 packet) and truncated headers without NEEDS_CSUM are
rejected with -EINVAL.

Based on a reproducer by Michael S. Tsirkin <mst@redhat.com>.

Assisted-by: LLM
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 tools/testing/selftests/net/tun.c | 107 ++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
index abe488bac50bb3b05df5c446836c1c3dfa9ea604..c6afafb7b957360e2873640e60a7d580c6564a66 100644
--- a/tools/testing/selftests/net/tun.c
+++ b/tools/testing/selftests/net/tun.c
@@ -9,6 +9,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <linux/if_tun.h>
+#include <netinet/tcp.h>
 #include <sys/ioctl.h>
 #include <sys/socket.h>
 
@@ -542,6 +543,112 @@ TEST_F(tun, reattach_close_delete)
 	EXPECT_EQ(tun_delete(self->ifname), 0);
 }
 
+FIXTURE(tun_vnet_gso)
+{
+	char ifname[IFNAMSIZ];
+	int fd;
+};
+
+FIXTURE_SETUP(tun_vnet_gso)
+{
+	int flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
+
+	memset(self->ifname, 0, sizeof(self->ifname));
+	self->fd = tun_open(self->ifname, flags, 0, 0, NULL);
+	ASSERT_GE(self->fd, 0);
+}
+
+FIXTURE_TEARDOWN(tun_vnet_gso)
+{
+	if (self->fd >= 0)
+		close(self->fd);
+}
+
+static int build_vlan_tcpv4_gso_packet(uint8_t *buf, int payload_len)
+{
+	uint16_t vlan_tag[2] = { htons(100), htons(ETH_P_IP) };
+	uint8_t *cur = buf + sizeof(struct virtio_net_hdr);
+	struct virtio_net_hdr vh = { 0 };
+	struct tcphdr tcph = { 0 };
+	uint32_t sum;
+
+	cur += build_eth(cur, ETH_P_8021Q, param_hwaddr_outer_src,
+			 param_hwaddr_outer_dst);
+
+	/* 802.1Q tag: VID=100, inner protocol=ETH_P_IP */
+	memcpy(cur, vlan_tag, sizeof(vlan_tag));
+	cur += sizeof(vlan_tag);
+
+	cur += build_ipv4_header(cur, IPPROTO_TCP,
+				 sizeof(tcph) + payload_len,
+				 &param_ipaddr4_outer_src,
+				 &param_ipaddr4_outer_dst);
+
+	tcph.source = htons(12345);
+	tcph.dest = htons(80);
+	tcph.seq = htonl(1);
+	tcph.doff = sizeof(tcph) / 4;
+	tcph.ack = 1;
+	tcph.window = htons(65535);
+	memcpy(cur, &tcph, sizeof(tcph));
+	memset(cur + sizeof(tcph), PKT_DATA, payload_len);
+
+	sum = add_csum((const uint8_t *)&param_ipaddr4_outer_src,
+		       sizeof(param_ipaddr4_outer_src));
+	sum += add_csum((const uint8_t *)&param_ipaddr4_outer_dst,
+			sizeof(param_ipaddr4_outer_dst));
+	sum += htons(IPPROTO_TCP) + htons(sizeof(tcph) + payload_len);
+	sum += add_csum(cur, sizeof(tcph) + payload_len);
+	tcph.check = finish_ip_csum(sum);
+	memcpy(cur, &tcph, sizeof(tcph));
+	cur += sizeof(tcph) + payload_len;
+
+	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
+	vh.gso_size = 1400;
+	vh.hdr_len = (cur - buf) - sizeof(vh) - payload_len;
+	memcpy(buf, &vh, sizeof(vh));
+
+	return cur - buf;
+}
+
+TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum)
+{
+	struct virtio_net_hdr vh;
+	uint8_t pkt[4096] = { 0 };
+	int len, ret;
+
+	len = build_vlan_tcpv4_gso_packet(pkt, 2800);
+	memcpy(&vh, pkt, sizeof(vh));
+
+	/* Valid VLAN-tagged TCPv4 GSO with flags = 0 (no NEEDS_CSUM) */
+	vh.flags = 0;
+	memcpy(pkt, &vh, sizeof(vh));
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, len);
+
+	/* Valid VLAN-tagged TCPv4 GSO with flags = DATA_VALID */
+	vh.flags = VIRTIO_NET_HDR_F_DATA_VALID;
+	memcpy(pkt, &vh, sizeof(vh));
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, len);
+
+	/* Mismatched GSO type (TCPV6 on VLAN-tagged IPv4 packet) */
+	vh.flags = 0;
+	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+	memcpy(pkt, &vh, sizeof(vh));
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	/* Truncated TCP header without NEEDS_CSUM */
+	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
+	vh.hdr_len = ETH_HLEN + 4 + sizeof(struct iphdr);
+	memcpy(pkt, &vh, sizeof(vh));
+	ret = write(self->fd, pkt, sizeof(vh) + vh.hdr_len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
 FIXTURE(tun_vnet_udptnl)
 {
 	char ifname[IFNAMSIZ];
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH v2 net 2/2] selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM

[Willem de Bruijn]

Eric Dumazet wrote:
> Add a selftest in tun.c verifying that a VLAN-tagged (802.1Q) TCPv4 GSO
> packet without VIRTIO_NET_HDR_F_NEEDS_CSUM (both flags = 0 and
> flags = VIRTIO_NET_HDR_F_DATA_VALID) is accepted when written to a TAP
> device (/dev/net/tun with IFF_TAP | IFF_NO_PI | IFF_VNET_HDR).
> 
> Also verify that mismatched GSO types (e.g. VIRTIO_NET_HDR_GSO_TCPV6 on
> a VLAN-tagged IPv4 packet) and truncated headers without NEEDS_CSUM are
> rejected with -EINVAL.
> 
> Based on a reproducer by Michael S. Tsirkin <mst@redhat.com>.
> 
> Assisted-by: LLM
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
>  tools/testing/selftests/net/tun.c | 107 ++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> index abe488bac50bb3b05df5c446836c1c3dfa9ea604..c6afafb7b957360e2873640e60a7d580c6564a66 100644
> --- a/tools/testing/selftests/net/tun.c
> +++ b/tools/testing/selftests/net/tun.c
> @@ -9,6 +9,7 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <linux/if_tun.h>
> +#include <netinet/tcp.h>
>  #include <sys/ioctl.h>
>  #include <sys/socket.h>
>  
> @@ -542,6 +543,112 @@ TEST_F(tun, reattach_close_delete)
>  	EXPECT_EQ(tun_delete(self->ifname), 0);
>  }
>  
> +FIXTURE(tun_vnet_gso)
> +{
> +	char ifname[IFNAMSIZ];
> +	int fd;
> +};
> +
> +FIXTURE_SETUP(tun_vnet_gso)
> +{
> +	int flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
> +
> +	memset(self->ifname, 0, sizeof(self->ifname));
> +	self->fd = tun_open(self->ifname, flags, 0, 0, NULL);
> +	ASSERT_GE(self->fd, 0);
> +}
> +
> +FIXTURE_TEARDOWN(tun_vnet_gso)
> +{
> +	if (self->fd >= 0)
> +		close(self->fd);
> +}
> +
> +static int build_vlan_tcpv4_gso_packet(uint8_t *buf, int payload_len)
> +{
> +	uint16_t vlan_tag[2] = { htons(100), htons(ETH_P_IP) };
> +	uint8_t *cur = buf + sizeof(struct virtio_net_hdr);
> +	struct virtio_net_hdr vh = { 0 };
> +	struct tcphdr tcph = { 0 };
> +	uint32_t sum;
> +
> +	cur += build_eth(cur, ETH_P_8021Q, param_hwaddr_outer_src,
> +			 param_hwaddr_outer_dst);
> +
> +	/* 802.1Q tag: VID=100, inner protocol=ETH_P_IP */
> +	memcpy(cur, vlan_tag, sizeof(vlan_tag));
> +	cur += sizeof(vlan_tag);
> +
> +	cur += build_ipv4_header(cur, IPPROTO_TCP,
> +				 sizeof(tcph) + payload_len,
> +				 &param_ipaddr4_outer_src,
> +				 &param_ipaddr4_outer_dst);
> +
> +	tcph.source = htons(12345);
> +	tcph.dest = htons(80);
> +	tcph.seq = htonl(1);
> +	tcph.doff = sizeof(tcph) / 4;
> +	tcph.ack = 1;
> +	tcph.window = htons(65535);
> +	memcpy(cur, &tcph, sizeof(tcph));
> +	memset(cur + sizeof(tcph), PKT_DATA, payload_len);
> +
> +	sum = add_csum((const uint8_t *)&param_ipaddr4_outer_src,
> +		       sizeof(param_ipaddr4_outer_src));
> +	sum += add_csum((const uint8_t *)&param_ipaddr4_outer_dst,
> +			sizeof(param_ipaddr4_outer_dst));
> +	sum += htons(IPPROTO_TCP) + htons(sizeof(tcph) + payload_len);
> +	sum += add_csum(cur, sizeof(tcph) + payload_len);
> +	tcph.check = finish_ip_csum(sum);
> +	memcpy(cur, &tcph, sizeof(tcph));
> +	cur += sizeof(tcph) + payload_len;
> +
> +	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> +	vh.gso_size = 1400;
> +	vh.hdr_len = (cur - buf) - sizeof(vh) - payload_len;
> +	memcpy(buf, &vh, sizeof(vh));
> +
> +	return cur - buf;
> +}
> +
> +TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum)
> +{
> +	struct virtio_net_hdr vh;
> +	uint8_t pkt[4096] = { 0 };
> +	int len, ret;
> +
> +	len = build_vlan_tcpv4_gso_packet(pkt, 2800);
> +	memcpy(&vh, pkt, sizeof(vh));
> +
> +	/* Valid VLAN-tagged TCPv4 GSO with flags = 0 (no NEEDS_CSUM) */

This is not a GSO packet if no vh.gso_type

> +	vh.flags = 0;
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, len);
> +
> +	/* Valid VLAN-tagged TCPv4 GSO with flags = DATA_VALID */

Same

> +	vh.flags = VIRTIO_NET_HDR_F_DATA_VALID;
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, len);
> +
> +	/* Mismatched GSO type (TCPV6 on VLAN-tagged IPv4 packet) */
> +	vh.flags = 0;
> +	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);
> +
> +	/* Truncated TCP header without NEEDS_CSUM */
> +	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> +	vh.hdr_len = ETH_HLEN + 4 + sizeof(struct iphdr);
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, sizeof(vh) + vh.hdr_len);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);
> +}
> +
>  FIXTURE(tun_vnet_udptnl)
>  {
>  	char ifname[IFNAMSIZ];
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
> 

Re: [PATCH v2 net 2/2] selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM

[Eric Dumazet]

On Mon, Sep 28, 2026 at 9:17 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Eric Dumazet wrote:
> > Add a selftest in tun.c verifying that a VLAN-tagged (802.1Q) TCPv4 GSO
> > packet without VIRTIO_NET_HDR_F_NEEDS_CSUM (both flags = 0 and
> > flags = VIRTIO_NET_HDR_F_DATA_VALID) is accepted when written to a TAP
> > device (/dev/net/tun with IFF_TAP | IFF_NO_PI | IFF_VNET_HDR).
> >
> > Also verify that mismatched GSO types (e.g. VIRTIO_NET_HDR_GSO_TCPV6 on
> > a VLAN-tagged IPv4 packet) and truncated headers without NEEDS_CSUM are
> > rejected with -EINVAL.
> >
> > Based on a reproducer by Michael S. Tsirkin <mst@redhat.com>.
> >
> > Assisted-by: LLM
> > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> > ---
> >  tools/testing/selftests/net/tun.c | 107 ++++++++++++++++++++++++++++++
> >  1 file changed, 107 insertions(+)
> >
> > diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> > index abe488bac50bb3b05df5c446836c1c3dfa9ea604..c6afafb7b957360e2873640e60a7d580c6564a66 100644
> > --- a/tools/testing/selftests/net/tun.c
> > +++ b/tools/testing/selftests/net/tun.c
> > @@ -9,6 +9,7 @@
> >  #include <string.h>
> >  #include <unistd.h>
> >  #include <linux/if_tun.h>
> > +#include <netinet/tcp.h>
> >  #include <sys/ioctl.h>
> >  #include <sys/socket.h>
> >
> > @@ -542,6 +543,112 @@ TEST_F(tun, reattach_close_delete)
> >       EXPECT_EQ(tun_delete(self->ifname), 0);
> >  }
> >
> > +FIXTURE(tun_vnet_gso)
> > +{
> > +     char ifname[IFNAMSIZ];
> > +     int fd;
> > +};
> > +
> > +FIXTURE_SETUP(tun_vnet_gso)
> > +{
> > +     int flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
> > +
> > +     memset(self->ifname, 0, sizeof(self->ifname));
> > +     self->fd = tun_open(self->ifname, flags, 0, 0, NULL);
> > +     ASSERT_GE(self->fd, 0);
> > +}
> > +
> > +FIXTURE_TEARDOWN(tun_vnet_gso)
> > +{
> > +     if (self->fd >= 0)
> > +             close(self->fd);
> > +}
> > +
> > +static int build_vlan_tcpv4_gso_packet(uint8_t *buf, int payload_len)
> > +{
> > +     uint16_t vlan_tag[2] = { htons(100), htons(ETH_P_IP) };
> > +     uint8_t *cur = buf + sizeof(struct virtio_net_hdr);
> > +     struct virtio_net_hdr vh = { 0 };
> > +     struct tcphdr tcph = { 0 };
> > +     uint32_t sum;
> > +
> > +     cur += build_eth(cur, ETH_P_8021Q, param_hwaddr_outer_src,
> > +                      param_hwaddr_outer_dst);
> > +
> > +     /* 802.1Q tag: VID=100, inner protocol=ETH_P_IP */
> > +     memcpy(cur, vlan_tag, sizeof(vlan_tag));
> > +     cur += sizeof(vlan_tag);
> > +
> > +     cur += build_ipv4_header(cur, IPPROTO_TCP,
> > +                              sizeof(tcph) + payload_len,
> > +                              &param_ipaddr4_outer_src,
> > +                              &param_ipaddr4_outer_dst);
> > +
> > +     tcph.source = htons(12345);
> > +     tcph.dest = htons(80);
> > +     tcph.seq = htonl(1);
> > +     tcph.doff = sizeof(tcph) / 4;
> > +     tcph.ack = 1;
> > +     tcph.window = htons(65535);
> > +     memcpy(cur, &tcph, sizeof(tcph));
> > +     memset(cur + sizeof(tcph), PKT_DATA, payload_len);
> > +
> > +     sum = add_csum((const uint8_t *)&param_ipaddr4_outer_src,
> > +                    sizeof(param_ipaddr4_outer_src));
> > +     sum += add_csum((const uint8_t *)&param_ipaddr4_outer_dst,
> > +                     sizeof(param_ipaddr4_outer_dst));
> > +     sum += htons(IPPROTO_TCP) + htons(sizeof(tcph) + payload_len);
> > +     sum += add_csum(cur, sizeof(tcph) + payload_len);
> > +     tcph.check = finish_ip_csum(sum);
> > +     memcpy(cur, &tcph, sizeof(tcph));
> > +     cur += sizeof(tcph) + payload_len;
> > +
> > +     vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> > +     vh.gso_size = 1400;
> > +     vh.hdr_len = (cur - buf) - sizeof(vh) - payload_len;
> > +     memcpy(buf, &vh, sizeof(vh));
> > +
> > +     return cur - buf;
> > +}
> > +
> > +TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum)
> > +{
> > +     struct virtio_net_hdr vh;
> > +     uint8_t pkt[4096] = { 0 };
> > +     int len, ret;
> > +
> > +     len = build_vlan_tcpv4_gso_packet(pkt, 2800);
> > +     memcpy(&vh, pkt, sizeof(vh));
> > +
> > +     /* Valid VLAN-tagged TCPv4 GSO with flags = 0 (no NEEDS_CSUM) */
>
> This is not a GSO packet if no vh.gso_type
>
> > +     vh.flags = 0;
> > +     memcpy(pkt, &vh, sizeof(vh));
> > +     ret = write(self->fd, pkt, len);
> > +     ASSERT_EQ(ret, len);
> > +
> > +     /* Valid VLAN-tagged TCPv4 GSO with flags = DATA_VALID */
>
> Same

build_vlan_tcpv4_gso_packet() already sets:

vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
vh.gso_size = 1400;
vh.hdr_len = (cur - buf) - sizeof(vh) - payload_len;
memcpy(buf, &vh, sizeof(vh));

and vlan_tcpv4_gso_no_csum() copies it back into the local vh right
afterwards:

len = build_vlan_tcpv4_gso_packet(pkt, 2800);
memcpy(&vh, pkt, sizeof(vh));

so vh.gso_type is VIRTIO_NET_HDR_GSO_TCPV4 (and vh.gso_size is 1400) for
both the flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID cases.

Re: [PATCH v2 net 2/2] selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM

[Willem de Bruijn]

Eric Dumazet wrote:
> On Mon, Sep 28, 2026 at 9:17 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Eric Dumazet wrote:
> > > Add a selftest in tun.c verifying that a VLAN-tagged (802.1Q) TCPv4 GSO
> > > packet without VIRTIO_NET_HDR_F_NEEDS_CSUM (both flags = 0 and
> > > flags = VIRTIO_NET_HDR_F_DATA_VALID) is accepted when written to a TAP
> > > device (/dev/net/tun with IFF_TAP | IFF_NO_PI | IFF_VNET_HDR).
> > >
> > > Also verify that mismatched GSO types (e.g. VIRTIO_NET_HDR_GSO_TCPV6 on
> > > a VLAN-tagged IPv4 packet) and truncated headers without NEEDS_CSUM are
> > > rejected with -EINVAL.
> > >
> > > Based on a reproducer by Michael S. Tsirkin <mst@redhat.com>.
> > >
> > > Assisted-by: LLM
> > > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> > > ---
> > >  tools/testing/selftests/net/tun.c | 107 ++++++++++++++++++++++++++++++
> > >  1 file changed, 107 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> > > index abe488bac50bb3b05df5c446836c1c3dfa9ea604..c6afafb7b957360e2873640e60a7d580c6564a66 100644
> > > --- a/tools/testing/selftests/net/tun.c
> > > +++ b/tools/testing/selftests/net/tun.c
> > > @@ -9,6 +9,7 @@
> > >  #include <string.h>
> > >  #include <unistd.h>
> > >  #include <linux/if_tun.h>
> > > +#include <netinet/tcp.h>
> > >  #include <sys/ioctl.h>
> > >  #include <sys/socket.h>
> > >
> > > @@ -542,6 +543,112 @@ TEST_F(tun, reattach_close_delete)
> > >       EXPECT_EQ(tun_delete(self->ifname), 0);
> > >  }
> > >
> > > +FIXTURE(tun_vnet_gso)
> > > +{
> > > +     char ifname[IFNAMSIZ];
> > > +     int fd;
> > > +};
> > > +
> > > +FIXTURE_SETUP(tun_vnet_gso)
> > > +{
> > > +     int flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
> > > +
> > > +     memset(self->ifname, 0, sizeof(self->ifname));
> > > +     self->fd = tun_open(self->ifname, flags, 0, 0, NULL);
> > > +     ASSERT_GE(self->fd, 0);
> > > +}
> > > +
> > > +FIXTURE_TEARDOWN(tun_vnet_gso)
> > > +{
> > > +     if (self->fd >= 0)
> > > +             close(self->fd);
> > > +}
> > > +
> > > +static int build_vlan_tcpv4_gso_packet(uint8_t *buf, int payload_len)
> > > +{
> > > +     uint16_t vlan_tag[2] = { htons(100), htons(ETH_P_IP) };
> > > +     uint8_t *cur = buf + sizeof(struct virtio_net_hdr);
> > > +     struct virtio_net_hdr vh = { 0 };
> > > +     struct tcphdr tcph = { 0 };
> > > +     uint32_t sum;
> > > +
> > > +     cur += build_eth(cur, ETH_P_8021Q, param_hwaddr_outer_src,
> > > +                      param_hwaddr_outer_dst);
> > > +
> > > +     /* 802.1Q tag: VID=100, inner protocol=ETH_P_IP */
> > > +     memcpy(cur, vlan_tag, sizeof(vlan_tag));
> > > +     cur += sizeof(vlan_tag);
> > > +
> > > +     cur += build_ipv4_header(cur, IPPROTO_TCP,
> > > +                              sizeof(tcph) + payload_len,
> > > +                              &param_ipaddr4_outer_src,
> > > +                              &param_ipaddr4_outer_dst);
> > > +
> > > +     tcph.source = htons(12345);
> > > +     tcph.dest = htons(80);
> > > +     tcph.seq = htonl(1);
> > > +     tcph.doff = sizeof(tcph) / 4;
> > > +     tcph.ack = 1;
> > > +     tcph.window = htons(65535);
> > > +     memcpy(cur, &tcph, sizeof(tcph));
> > > +     memset(cur + sizeof(tcph), PKT_DATA, payload_len);
> > > +
> > > +     sum = add_csum((const uint8_t *)&param_ipaddr4_outer_src,
> > > +                    sizeof(param_ipaddr4_outer_src));
> > > +     sum += add_csum((const uint8_t *)&param_ipaddr4_outer_dst,
> > > +                     sizeof(param_ipaddr4_outer_dst));
> > > +     sum += htons(IPPROTO_TCP) + htons(sizeof(tcph) + payload_len);
> > > +     sum += add_csum(cur, sizeof(tcph) + payload_len);
> > > +     tcph.check = finish_ip_csum(sum);
> > > +     memcpy(cur, &tcph, sizeof(tcph));
> > > +     cur += sizeof(tcph) + payload_len;
> > > +
> > > +     vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> > > +     vh.gso_size = 1400;
> > > +     vh.hdr_len = (cur - buf) - sizeof(vh) - payload_len;
> > > +     memcpy(buf, &vh, sizeof(vh));
> > > +
> > > +     return cur - buf;
> > > +}
> > > +
> > > +TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum)
> > > +{
> > > +     struct virtio_net_hdr vh;
> > > +     uint8_t pkt[4096] = { 0 };
> > > +     int len, ret;
> > > +
> > > +     len = build_vlan_tcpv4_gso_packet(pkt, 2800);
> > > +     memcpy(&vh, pkt, sizeof(vh));
> > > +
> > > +     /* Valid VLAN-tagged TCPv4 GSO with flags = 0 (no NEEDS_CSUM) */
> >
> > This is not a GSO packet if no vh.gso_type
> >
> > > +     vh.flags = 0;
> > > +     memcpy(pkt, &vh, sizeof(vh));
> > > +     ret = write(self->fd, pkt, len);
> > > +     ASSERT_EQ(ret, len);
> > > +
> > > +     /* Valid VLAN-tagged TCPv4 GSO with flags = DATA_VALID */
> >
> > Same
> 
> build_vlan_tcpv4_gso_packet() already sets:
> 
> vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> vh.gso_size = 1400;
> vh.hdr_len = (cur - buf) - sizeof(vh) - payload_len;
> memcpy(buf, &vh, sizeof(vh));

Missed that, sorry. I was only looking at the top of the function.

> and vlan_tcpv4_gso_no_csum() copies it back into the local vh right
> afterwards:
> 
> len = build_vlan_tcpv4_gso_packet(pkt, 2800);
> memcpy(&vh, pkt, sizeof(vh));
> 
> so vh.gso_type is VIRTIO_NET_HDR_GSO_TCPV4 (and vh.gso_size is 1400) for
> both the flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID cases.

Re: [PATCH v2 net 1/2] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 (1) · 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).

- [Medium] `__skb_flow_dissect()` sets `key_control->thoff = min_t(u16,
  nhoff, skb ?

Pre-existing issues:
- [Medium] In af_packet, `packet_parse_headers()` runs before
  `virtio_net_hdr_to_skb()` (`packet_snd()` lines 3130/3133;…
- [Low] In the UML vector driver, `prep_skb()` sets `skb->len` to the
  full receive-buffer capacity (`skb_put(result, vp->max_packet)`, plus…

--

net: always dissect GSO packets in __virtio_net_hdr_to_skb()

This patch drops the skb->network_header condition and the unvalidated
fallback in __virtio_net_hdr_to_skb(). Every GSO packet without
VIRTIO_NET_HDR_F_NEEDS_CSUM is now flow-dissected and gets a transport
header. The tun, virtio_net and UML vector callers now set skb->dev and
skb->network_header before the conversion, and the
virtio_net_hdr_match_proto() check now runs after dissection.

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

> diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
> index ddd127ee9678..e1fb2a76fdff 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());

[Severity: Low]
This isn't a bug introduced by this patch, but raw_verify_header() now
takes the dissection path. At this point, isn't skb->len still the full
receive buffer capacity rather than the length actually received?

prep_skb() sizes the skb up front:

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

Both vector_legacy_rx() and vector_mmsg_rx() call verify_header() before
pskb_trim() cuts the skb down to the received length:

vector_legacy_rx()
    header_check = vp->verify_header(vp->header_rxbuffer, skb, vp);
    ...
    pskb_trim(skb, pkt_len - vp->rx_header_size);

So the dissection, pskb_may_pull() of thoff + thlen and
skb_set_transport_header() all work on stale buffer bytes past the
received length.

After the trim, can a short frame with a GSO vnet header end up with
transport_header and GSO metadata beyond skb->tail?

The reads stay inside the allocated linear head. Triggering this needs
the host peer to send a GSO vnet header for a frame shorter than its
headers. The verify-before-trim ordering predates this patch, and the
old fallback also set gso_size/gso_type from the untrimmed length.

>  	return 0;
>  }

[ ... ]

> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index c381b916c1b5..d6466f96cdd0 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -111,48 +111,38 @@ 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.
>  		 */

[ ... ]

> +		struct flow_keys_basic keys;
> +
> +		if (!skb->protocol) {
> +			skb->protocol = dev_parse_header_protocol(skb);
> +			if (!skb->protocol)
> +				virtio_net_hdr_set_proto(skb, hdr);
> +		}
>  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;
> +		}

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. The
commit message mentions VLAN-tagged frames, though. For af_packet VLAN
frames, does this dissector call get a protocol and a network offset
that describe different headers?

packet_snd() calls packet_parse_headers() before
virtio_net_hdr_to_skb(). tpacket_snd() does the same through
tpacket_fill_skb():

net/packet/af_packet.c:packet_parse_headers() {
    ...
	    vlan_get_protocol_and_depth(skb, skb->protocol, &depth) != 0)
		skb_set_network_header(skb, depth);
}

For VLAN-tagged frames this moves network_header past the tags, but
skb->protocol stays ETH_P_8021Q or ETH_P_8021AD.

The call here passes data == NULL, so the dissector uses
proto = skb->protocol and nhoff = skb_network_offset(skb). It then
reads the inner L3 header as a vlan_hdr:

net/core/flow_dissector.c:__skb_flow_dissect() {
    ...
		proto = vlan->h_vlan_encapsulated_proto;
    ...
}

That value would come from the IPv4 tot_len field or the IPv6 payload
length bytes. Would VLAN GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM
sent through AF_PACKET with PACKET_VNET_HDR then fail with -EINVAL? And
could they be misparsed if those bytes happen to form a valid ethertype?

tap_get_user() keeps network_header at ETH_HLEN while it calls
tun_vnet_hdr_to_skb(), and only moves it for VLAN afterwards. That is
the layout this helper expects.

The af_packet network header adjustment comes from commit dfed913e8b55a
("net/af_packet: add VLAN support for AF_PACKET SOCK_RAW GSO"). These
skbs already hit the same dissector call before this patch.

>  
> -			p_off = keys.control.thoff + thlen;
> -			if (!pskb_may_pull(skb, p_off) ||
> -			    keys.basic.ip_proto != ip_proto)
> -				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);
> -		} else if (gso_type) {
> -			p_off = nh_min_len + thlen;
> -			if (!pskb_may_pull(skb, p_off))
> -				return -EINVAL;
> -		}
> +		skb_set_transport_header(skb, keys.control.thoff);

[Severity: Medium]
Can keys.control.thoff be truncated here for large skbs?
__skb_flow_dissect() ends with:

net/core/flow_dissector.c:__skb_flow_dissect() {
    ...
out:
	key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
    ...
}

min_t(u16, ...) casts skb->len to 16 bits. When skb->len is in
[65536, 65536 + nhoff), the result wraps to a small value.

With this patch, tun_get_user() calls tun_vnet_hdr_tnl_to_skb() before
eth_type_trans():

	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
	...
	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
		skb->protocol = eth_type_trans(skb, tun->dev);

So skb->len still includes the 14-byte Ethernet header during
dissection. Take an IFF_TAP frame with skb->len = 65540 and a plain
IPv4/TCP header (real nhoff of 34). That gives
thoff = (u16)65540 = 4.

ip_proto and n_proto are still correct, so every check above passes.
pskb_may_pull() pulls only 24 bytes, and skb_set_transport_header(skb, 4)
points into the destination MAC. The later skb_probe_transport_header()
in tun_get_user() does nothing, because the transport header is already
set.

Before this patch, tun got its transport header from
skb_probe_transport_header() after eth_type_trans() had pulled 14
bytes. Frames of 65536..65549 bytes were not affected.

This does not look like an out-of-bounds read, since
transport_header + 12 falls inside the pulled thoff + thlen bytes. But
would tcp_hdrlen() in skb_gso_transport_seglen() and
qdisc_pkt_len_init() then read the wrong bytes? That would give wrong
qdisc pkt_len and gso_segs accounting, and a wrong tbf MAC length check.

It also seems to contradict this part of the commit message:

    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.

In this case the real TCP header at offset 34..54 can stay in page
frags.

This is reachable through IFF_TAP with IFF_VNET_HDR, from a vhost-net
guest or an unprivileged tun owner in a user namespace. tun_alloc_skb()
allows skb->len well above 64K. tun_xdp_one() does not look affected,
since datasize is limited to one page.

>  	}
>  
>  	if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928144254.3361044-1-edumazet%40kernel.org

Re: [PATCH v2 net 2/2] selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The 'Truncated TCP header without NEEDS_CSUM' sub-case in
  TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum)…

--

selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM

This adds a selftest in tun.c that writes a VLAN-tagged TCPv4 GSO packet
without VIRTIO_NET_HDR_F_NEEDS_CSUM to a TAP device with IFF_VNET_HDR. It
checks that the packet is accepted with flags = 0 and with
flags = VIRTIO_NET_HDR_F_DATA_VALID. It also checks that a mismatched GSO
type and a truncated header are rejected with -EINVAL.

> diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> index abe488bac50b..c6afafb7b957 100644
> --- a/tools/testing/selftests/net/tun.c
> +++ b/tools/testing/selftests/net/tun.c

[ ... ]

> +TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum)
> +{

[ ... ]

> +	/* Truncated TCP header without NEEDS_CSUM */
> +	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> +	vh.hdr_len = ETH_HLEN + 4 + sizeof(struct iphdr);
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, sizeof(vh) + vh.hdr_len);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);

[Severity: Low]
Would this sub-case still pass on a kernel without commit 67fbe1f8a962
("net: always dissect GSO packets in __virtio_net_hdr_to_skb()")?

The frame written here is ETH_HLEN + 4 + sizeof(struct iphdr) = 38 bytes
after the vnet header.

Before that fix, tun_get_user() reached the unvalidated gso_type fallback
with skb->network_header still at 0. That fallback did
pskb_may_pull(skb, nh_min_len + thlen), which is 40 bytes, so a 38-byte
frame already failed with -EINVAL.

After the fix, __virtio_net_hdr_to_skb() dissects the frame. thoff is
14 + 4 + 20 = 38:

include/linux/virtio_net.h:__virtio_net_hdr_to_skb() {
    ...
		p_off = keys.control.thoff + thlen;
		if (!pskb_may_pull(skb, p_off) ||
    ...
			return -EINVAL;
    ...
}

With p_off = 58 this also fails with -EINVAL. Both kernels therefore give
the same result, and the ASSERTs never exercise the thoff-based pull.

Only frame lengths from 40 to 57 bytes separate the two kernels for this
VLAN packet. Take hdr_len = ETH_HLEN + 4 + sizeof(struct iphdr) + 10 = 48.
The old kernel pulls 40 bytes and succeeds. Since skb->len - nh_off (8)
is not greater than gso_size, no GSO is set and write() returns the full
length. The fixed kernel needs 58 bytes and returns -EINVAL.

Could the truncated length be picked from that range, so this sub-case
catches a regression of the thoff-based pull? The TCPV6-on-IPv4 sub-case
above already separates the two kernels through
virtio_net_hdr_match_proto().

> +}
> +
>  FIXTURE(tun_vnet_udptnl)
>  {
>  	char ifname[IFNAMSIZ];

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928144254.3361044-1-edumazet%40kernel.org

Re: [PATCH v2 net 1/2] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

On Thu, Oct 1, 2026 at 7:52 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 (1) · 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).
>
> - [Medium] `__skb_flow_dissect()` sets `key_control->thoff = min_t(u16,
>   nhoff, skb ?

Allright. I will fix __skb_flow_dissect() in a separate patch.

pw-bot: cr