[PATCH net v3 0/8] net: virtio: validate outer protocol of UDP tunnel GSO packets

[PATCH net v3 0/8] net: virtio: validate outer protocol of UDP tunnel GSO packets

[Eric Dumazet]

virtio_net_hdr_tnl_to_skb() does not check that a UDP tunnel GSO
packet really has outer IPv4/IPv6 and UDP headers matching the
declared tunnel type. A guest, or any tap writer, can send a frame
with another outer ethertype (NSH in the report), and the GSO stack
then builds segments with skb->data before skb->head. The host
crashes in skb_under_panic() as soon as a header is pushed.

Patch 1 validates the outer headers, using the parsing helpers from
Michael's series "net: resolve uncommon partial checksums at ingress",
as Willem suggested. The rest of that series (checksum validation and
completion) is left to Michael.

Patch 2 lets the callers tell whether frames start with an Ethernet
header, instead of using the device type, which tun can change with
TUNSETLINK.

Patch 3 clears a stale skb->protocol left on recycled NAPI frags skbs,
which could make the checks of patch 1 reject valid frames.

Patch 4 fixes a selftest helper computing UDP checksums over a length
in network byte order.

Patches 5 to 7 add tun selftests, on tap and IFF_TUN devices: valid
outer IPv4/IPv6 frames are accepted; mismatched or non-UDP outer
headers and misplaced outer transport headers are rejected, also on
retyped devices.

Patch 8 makes the UDP tunnel selftests write the tunnel offsets of the
virtio header in little endian, so they work on big-endian hosts.

v3:
- Patch 1: fix a comment, ipv6_find_hdr() stops at AH (Sashiko).
- New patch 2, and its tests in patch 6: Sashiko found that changing
  the device type with TUNSETLINK bypasses the checks of patch 1.
- New patch 3: Sashiko found that napi_reuse_skb() leaves a stale
  skb->protocol on IFF_NAPI_FRAGS skbs.
- New patch 7: test cases Sashiko found missing in patch 5 (outer
  IPv6, VLAN with IPv6 or with a wrong outer_th_offset, non-UDP outer
  L4). Fixed the patch 5 changelog.
- New patch 8: big-endian hosts (Sashiko).
- Collected Reviewed-by and Acked-by tags.

v2: https://lore.kernel.org/netdev/20261104050131.629967-1-edumazet@kernel.org/
- Patch 1: use Michael's helpers instead of a new one: find the L3
  header from the device type instead of assuming Ethernet when
  skb->protocol is unset, do not trust skb->protocol when it disagrees
  with the frame, and require UDP exactly at outer_th_offset. Stop at
  IPv6 AH headers (Willem).
- Patch 2: clear udph->check before summing and send 0xffff instead
  of a zero checksum (Willem).
- Patch 3: clear the reused buffer before building each frame (Willem).
  Add IFF_TUN tests, where skb->protocol comes from the tun_pi header.

v1: https://lore.kernel.org/netdev/20261103203113.550604-1-edumazet@kernel.org/

Eric Dumazet (8):
  net: virtio: validate outer protocol of UDP tunnel GSO packets
  net: virtio: do not use dev->type to find the outer L3 header
  tun: clear stale skb->protocol of NAPI frags skbs
  selftests: net: tuntap: fix UDP checksum length in
    build_udp_packet_csum()
  selftests: net: tun: test outer protocol of UDP tunnel GSO packets
  selftests: net: tun: test UDP tunnel GSO on retyped devices
  selftests: net: tun: test more outer header mismatches of UDP tunnel
    GSO
  selftests: net: tuntap: write UDP tunnel offsets in little endian

 drivers/net/tun.c                            |  14 +-
 drivers/net/tun_vnet.h                       |   5 +-
 drivers/net/virtio_net.c                     |   2 +-
 include/linux/virtio_net.h                   | 169 +++++++
 tools/testing/selftests/net/tun.c            | 440 +++++++++++++++++++
 tools/testing/selftests/net/tuntap_helpers.h |  29 +-
 6 files changed, 649 insertions(+), 10 deletions(-)


base-commit: 37dfd0efbfffc20a57e41c2eb349ac49b1ca1ff8
-- 
2.53.0

[PATCH net v3 1/8] net: virtio: validate outer protocol of UDP tunnel GSO packets

[Eric Dumazet]

virtio_net_hdr_tnl_to_skb() trusts the virtio header to describe a
UDP tunnel over IPv4 or IPv6, but never checks that the frame actually
carries such outer headers. Because VIRTIO_NET_HDR_F_NEEDS_CSUM is
mandatory for tunnel GSO, __virtio_net_hdr_to_skb() skips the flow
dissector, so virtio_net_hdr_match_proto() is never applied either.

A guest (or a tap writer) can then inject a frame with an arbitrary
outer ethertype, for example ETH_P_NSH, with SKB_GSO_UDP_TUNNEL set.
The GSO stack then runs nsh_gso_segment() on top of the UDP tunnel
segmentation, which assumes a different header layout. The result is
segments with skb->data before skb->head, and a host crash in
skb_under_panic() as soon as a later layer pushes a header:

  skbuff: skb_under_panic: ... put:4 ... dev:gretap1
  kernel BUG at net/core/skbuff.c:215!
   skb_push
   __gre_xmit
   gre_tap_xmit
   ...
   tun_get_user

Validate the outer headers with the parsing helpers from Michael's
series "net: resolve uncommon partial checksums at ingress":

- locate the L3 header from the device type: Ethernet with optional
  VLAN tags, or the network header set by the caller. skb->protocol is
  not trusted when it disagrees with the frame.
- require the outer L3 protocol to match the declared tunnel type.
- require the outer UDP header exactly at outer_th_offset, after IPv4
  options or IPv6 extension headers, and reject fragments.

Unlike the original helpers, ipv6_find_hdr() stops at an AH header,
as Willem suggested in the review of that series.

Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.")
Reported-by: Hassan Sultan <hassansultan@google.com>
Suggested-by: Willem de Bruijn <willemb@google.com>
Link: https://lore.kernel.org/netdev/93d0e37c99711bebdf60c120af03010455c2f2e2.1793276116.git.mst@redhat.com/
Cc: Willem de Bruijn <willemb@google.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Co-developed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 include/linux/virtio_net.h | 170 +++++++++++++++++++++++++++++++++++++
 1 file changed, 170 insertions(+)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index d6466f96cdd0..357116c2a148 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -5,7 +5,10 @@
 #include <linux/if_vlan.h>
 #include <linux/ip.h>
 #include <linux/ipv6.h>
+#include <linux/netdevice.h>
 #include <linux/udp.h>
+#include <net/ip.h>
+#include <net/ipv6.h>
 #include <net/tcp.h>
 #include <uapi/linux/tcp.h>
 #include <uapi/linux/virtio_net.h>
@@ -48,6 +51,147 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
 	return 0;
 }
 
+/* TODO: nothing virtio specific here, move to ip.h */
+
+/*
+ * Pull and minimally validate the IPv4 header at @nhoff.
+ * Returns the header and stores the transport header offset in
+ * @thoff, or NULL if truncated or malformed.  The whole header,
+ * options included, is guaranteed linear on return.
+ */
+static inline struct iphdr *virtio_net_ip4_parse(struct sk_buff *skb,
+						  unsigned int nhoff,
+						  unsigned int *thoff)
+{
+	unsigned int ihl;
+	struct iphdr *iph;
+
+	if (!pskb_may_pull(skb, nhoff + sizeof(struct iphdr)))
+		return NULL;
+
+	iph = (struct iphdr *)(skb->data + nhoff);
+	if (iph->version != 4 ||
+	    iph->ihl < sizeof(struct iphdr) / sizeof(u32))
+		return NULL;
+
+	ihl = iph->ihl * sizeof(u32);
+	*thoff = nhoff + ihl;
+	/* Pull the options as well: the transport header offset
+	 * returned above must be within the linear data.  iph goes
+	 * stale across the pull.
+	 */
+	if (!pskb_may_pull(skb, nhoff + ihl))
+		return NULL;
+
+	return (struct iphdr *)(skb->data + nhoff);
+}
+
+/* Where does the L3 header start, and what protocol is it?
+ * Handles Ethernet and VLAN headers; other devices use the
+ * network header offset the caller set up.
+ */
+static inline bool
+virtio_net_l3_start(struct sk_buff *skb, int *nhoff, __be16 *proto)
+{
+	/* skb->dev should always be set here already,
+	 * but let's not crash when it is not.
+	 */
+	if (WARN_ON_ONCE(!skb->dev))
+		return false;
+
+	if (skb->dev->type == ARPHRD_ETHER ||
+	    skb->dev->type == ARPHRD_LOOPBACK) {
+		__be16 h_proto;
+
+		if (!pskb_may_pull(skb, ETH_HLEN))
+			return false;
+		h_proto = eth_hdr(skb)->h_proto;
+		/* skb->protocol is a sender claim, and parts of the
+		 * stack parse the packet per it: when it disagrees with
+		 * the frame we do not know which view the packet will
+		 * be parsed as, so do not validate.
+		 */
+		if (skb->protocol && skb->protocol != h_proto)
+			return false;
+		*nhoff = ETH_HLEN;
+		*proto = h_proto;
+		if (eth_type_vlan(*proto)) {
+			*proto = __vlan_get_protocol(skb, *proto, nhoff);
+			if (!*proto)
+				return false;
+		}
+	} else {
+		const struct iphdr *iph;
+
+		*nhoff = skb_network_offset(skb);
+		/* An unset network header shows up as an offset
+		 * outside of the packet.
+		 */
+		if (*nhoff < 0 || (unsigned int)*nhoff >= skb->len)
+			return false;
+		/* No L2 header to cross-check with: take the protocol
+		 * from the IP version at the network header.
+		 */
+		if (!pskb_may_pull(skb, *nhoff + 1))
+			return false;
+		iph = (const struct iphdr *)(skb->data + *nhoff);
+		if (iph->version == 4)
+			*proto = htons(ETH_P_IP);
+		else if (iph->version == 6)
+			*proto = htons(ETH_P_IPV6);
+		else
+			return false;
+		if (skb->protocol && skb->protocol != *proto)
+			return false;
+	}
+	return true;
+}
+
+/* Walk the L3 header at @nhoff: where does its transport header
+ * start, and which protocol is it?  Returns false if the header
+ * is truncated, malformed or fragmented.
+ */
+static inline bool
+virtio_net_l4_start(struct sk_buff *skb, unsigned int nhoff, __be16 proto,
+		    unsigned int *thoff, int *ip_proto)
+{
+	if (proto == htons(ETH_P_IP)) {
+		const struct iphdr *iph = virtio_net_ip4_parse(skb, nhoff, thoff);
+
+		if (!iph || ip_is_fragment(iph))
+			return false;
+		*ip_proto = iph->protocol;
+		return true;
+	}
+
+	if (proto == htons(ETH_P_IPV6)) {
+		const struct ipv6hdr *iph;
+		int flags = IP6_FH_F_AUTH;
+
+		if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
+			return false;
+		iph = (const void *)(skb->data + nhoff);
+		if (iph->version != 6)
+			return false;
+		*thoff = nhoff + sizeof(*iph);
+		*ip_proto = iph->nexthdr;
+		if (*ip_proto != IPPROTO_TCP && *ip_proto != IPPROTO_UDP) {
+			*thoff = nhoff;
+			/* Find the transport header behind any
+			 * extension headers. IP6_FH_F_AUTH stops
+			 * the walk at AH, so frames with AH are rejected.
+			 */
+			*ip_proto = ipv6_find_hdr(skb, thoff, -1, NULL,
+						  &flags);
+			if (*ip_proto < 0 || flags & IP6_FH_F_FRAG)
+				return false;
+		}
+		return true;
+	}
+
+	return false;
+}
+
 static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
 					  const struct virtio_net_hdr *hdr,
 					  bool little_endian, u8 hdr_gso_type)
@@ -282,6 +426,29 @@ static inline unsigned int virtio_l3min(bool is_ipv6)
 	return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr);
 }
 
+/* Do the tunnel offsets match the outer headers in the frame?
+ * The L3 header the tunnel claims must be the one the frame
+ * carries, with UDP following it at @outer_th.
+ */
+static inline bool
+virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
+			  unsigned int outer_th)
+{
+	int nhoff;
+	__be16 proto;
+	unsigned int thoff;
+	int ip_proto;
+
+	if (!virtio_net_l3_start(skb, &nhoff, &proto))
+		return false;
+	if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
+		return false;
+	if (!virtio_net_l4_start(skb, nhoff, proto, &thoff, &ip_proto))
+		return false;
+
+	return ip_proto == IPPROTO_UDP && outer_th == thoff;
+}
+
 static inline int
 virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 			  const struct virtio_net_hdr_v1_hash_tunnel *vhdr,
@@ -338,6 +505,9 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 	if (ret)
 		return ret;
 
+	if (!virtio_net_tnl_offsets_ok(skb, outer_isv6, outer_th))
+		return -EINVAL;
+
 	/* In case of USO, the inner protocol is still unknown and
 	 * `inner_isv6` is just a guess, additional parsing is needed.
 	 * The previous validation ensures that accessing an ipv4 inner
-- 
2.53.0

Re: [PATCH net v3 1/8] net: virtio: validate outer protocol of UDP tunnel GSO packets

[netdev-bot+sashiko]

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

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

- [High] This patch makes Linux drop VLAN-tagged UDP-tunnel GSO frames
  that Linux's own tap device sends.
- [High] virtio_net_hdr_tnl_from_skb() chooses the tunnel type with `if
  (skb->protocol == htons(ETH_P_IPV6)) ... UDP_TUNNEL_IPV6; else ...…
- [High] In this commit, virtio_net_l3_start()
  (include/linux/virtio_net.h:102-103) decides whether the frame has an…

Pre-existing issues:
- [Low] The concern is that virtio_net_hdr_tnl_to_skb() requires
  `outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6)`…

--

net: virtio: validate outer protocol of UDP tunnel GSO packets

This patch makes virtio_net_hdr_tnl_to_skb() check that the outer L3
header in the frame matches the declared UDP tunnel GSO type. It also
checks that the outer UDP header sits exactly at outer_th_offset. The L3
header is found from the device type, walking any VLAN tags on Ethernet
devices, which closes the skb_under_panic() crash caused by tunnel GSO
frames with a bogus outer ethertype such as ETH_P_NSH.

Link: https://lore.kernel.org/netdev/93d0e37c99711bebdf60c120af03010455c2f2e2.1793276116.git.mst@redhat.com/

> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index d6466f96cdd00..357116c2a1489 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h

[ ... ]

> @@ -48,6 +51,147 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,

[ ... ]

> +static inline bool
> +virtio_net_l3_start(struct sk_buff *skb, int *nhoff, __be16 *proto)
> +{
> +	/* skb->dev should always be set here already,
> +	 * but let's not crash when it is not.
> +	 */
> +	if (WARN_ON_ONCE(!skb->dev))
> +		return false;
> +
> +	if (skb->dev->type == ARPHRD_ETHER ||
> +	    skb->dev->type == ARPHRD_LOOPBACK) {

[Severity: High]
Can the Ethernet vs L3 choice here disagree with how tun actually frames
the packet?

tun picks the framing from tun->flags (IFF_TAP vs IFF_TUN). The fd owner
can change dev->type separately through TUNSETLINK:

drivers/net/tun.c:__tun_chr_ioctl() {
    ...
			tun->dev->type = (int) arg;
    ...
}

Take an IFF_TAP device retyped to ARPHRD_NONE. It goes into the else
branch below, which only reads the IP version byte at
skb_network_offset() (ETH_HLEN). The skb->protocol cross-check in that
branch does nothing, because skb->protocol is still 0 at this point.

Once validation passes, tun_get_user() sets the protocol from an h_proto
that was never checked:

drivers/net/tun.c:tun_get_user() {
    ...
	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
		skb->protocol = eth_type_trans(skb, tun->dev);
    ...
}

tun_xdp_one() does the same unconditionally.

With an h_proto of ETH_P_NSH, wouldn't this still reach nsh_gso_segment()
and the skb_under_panic() described in the commit message?

The reverse case, IFF_TUN retyped to ARPHRD_ETHER, appears to validate an
Ethernet header that the stack then parses as L3.

A later patch in the series, "net: virtio: do not use dev->type to find
the outer L3 header", seems to fix this. It passes an explicit has_eth
argument from tun_get_user(), tun_xdp_one() and virtnet_receive_done().
Since this commit has a Fixes: tag, could that change be folded in here?

> +		__be16 h_proto;
> +
> +		if (!pskb_may_pull(skb, ETH_HLEN))
> +			return false;
> +		h_proto = eth_hdr(skb)->h_proto;

[ ... ]

> +		if (skb->protocol && skb->protocol != h_proto)
> +			return false;
> +		*nhoff = ETH_HLEN;
> +		*proto = h_proto;
> +		if (eth_type_vlan(*proto)) {
> +			*proto = __vlan_get_protocol(skb, *proto, nhoff);
> +			if (!*proto)
> +				return false;
> +		}
> +	} else {
> +		const struct iphdr *iph;
> +
> +		*nhoff = skb_network_offset(skb);

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch. The
ETH_HLEN floor that virtio_net_hdr_tnl_to_skb() already applies seems to
assume an L2 header:

	outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6);

For IFF_TUN, nhoff here is 0. A correctly formed L3-only tunnel GSO frame
(outer_th of 20 for IPv4) was rejected before this patch and is still
rejected.

This branch can still be reached: L3-only frames with at least 16 bytes
of IPv4 options, or enough IPv6 extension headers, clear the floor. The
series' selftests (outer_proto_ok and outer_proto_bad) exercise it.

So what remains is the old limitation on L3-only tunnel GSO. Is that
limitation intended?

> +		/* An unset network header shows up as an offset
> +		 * outside of the packet.
> +		 */
> +		if (*nhoff < 0 || (unsigned int)*nhoff >= skb->len)
> +			return false;
> +		/* No L2 header to cross-check with: take the protocol
> +		 * from the IP version at the network header.
> +		 */
> +		if (!pskb_may_pull(skb, *nhoff + 1))
> +			return false;
> +		iph = (const struct iphdr *)(skb->data + *nhoff);
> +		if (iph->version == 4)
> +			*proto = htons(ETH_P_IP);
> +		else if (iph->version == 6)
> +			*proto = htons(ETH_P_IPV6);
> +		else
> +			return false;
> +		if (skb->protocol && skb->protocol != *proto)
> +			return false;
> +	}
> +	return true;
> +}

[ ... ]

> @@ -282,6 +426,29 @@ static inline unsigned int virtio_l3min(bool is_ipv6)
>  	return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr);
>  }
>  
> +/* Do the tunnel offsets match the outer headers in the frame?
> + * The L3 header the tunnel claims must be the one the frame
> + * carries, with UDP following it at @outer_th.
> + */
> +static inline bool
> +virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
> +			  unsigned int outer_th)
> +{
> +	int nhoff;
> +	__be16 proto;
> +	unsigned int thoff;
> +	int ip_proto;
> +
> +	if (!virtio_net_l3_start(skb, &nhoff, &proto))
> +		return false;
> +	if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
> +		return false;

[Severity: High]
Does this reject IPv6-underlay tunnel GSO frames that virtio_net itself
sends when the VLAN tag is in-band?

virtio_net_l3_start() now walks past the VLAN tag and finds ETH_P_IPV6.
The sender, though, picks the tunnel type from skb->protocol:

include/linux/virtio_net.h:virtio_net_hdr_tnl_from_skb() {
    ...
	if (skb->protocol == htons(ETH_P_IPV6))
		hdr->gso_type |= VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6;
	else
		hdr->gso_type |= VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4;
    ...
}

virtio_net does not advertise NETIF_F_HW_VLAN_CTAG_TX. So
validate_xmit_vlan() calls __vlan_hwaccel_push_inside(), which puts the
tag into the frame and sets skb->protocol to ETH_P_8021Q.

GSO_UDP_TUNNEL is kept, because virtio_net sets dev->vlan_features to
dev->features and virtnet_features_check() does not strip GSO.

Consider a guest with a VLAN subinterface on virtio_net and an IPv6 VXLAN
or Geneve device on top. xmit_skb() then declares
VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4. On the host,
tun_get_user()/tun_xdp_one() -> virtio_net_hdr_tnl_to_skb() finds IPv6
behind the 802.1Q tag and returns -EINVAL.

Before this patch, outer_isv6 only chose the minimum offsets, so these
frames were accepted.

The selftest outer_proto_bad_vlan_ipv6 later in the series expects this
layout to be rejected. However, virtio_net_hdr_tnl_from_skb() still picks
the tunnel type from skb->protocol at the end of the series.

Should the sender look past the VLAN tag as well? Should the commit
message also mention this behaviour change?

> +	if (!virtio_net_l4_start(skb, nhoff, proto, &thoff, &ip_proto))
> +		return false;
> +
> +	return ip_proto == IPPROTO_UDP && outer_th == thoff;
> +}

[Severity: High]
Can this exact match drop VLAN-tagged tunnel GSO frames that tun itself
writes to userspace?

thoff here is measured after the VLAN walk in virtio_net_l3_start(). On
the send side, virtio_net_hdr_tnl_from_skb() passes vlan_hlen only to
virtio_net_hdr_from_skb(), where it adjusts csum_start. The tunnel
offsets are computed without it:

	inner_nh = skb->inner_network_header - skb_headroom(skb);
	outer_th = skb->transport_header - skb_headroom(skb);

When the skb has a hardware-accelerated VLAN tag, tun_put_user() builds
the vnet header first. It then inserts the 4 byte tag after the MAC
addresses:

drivers/net/tun.c:tun_put_user() {
    ...
	if (vlan_hlen) {
		...
		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
    ...
}

With an IPv4 outer header, the vnet header then says outer_th_offset is
34, while UDP is at 38 in the bytes the guest sees.

A guest with this patch runs virtnet_receive_done() ->
virtio_net_hdr_tnl_to_skb(). It follows the tag to nhoff = 18, gets
thoff = 38, returns -EINVAL and counts rx_frame_errors. Before this patch
the same frame passed the existing range checks.

tap advertises NETIF_F_HW_VLAN_CTAG_TX and keeps GSO_UDP_TUNNEL in
vlan_features and hw_enc_features. A VLAN-filtering bridge or OVS with a
tagged port toward the VM's tap looks like enough to hit this.

The series adds outer_proto_bad_vlan_offset, which expects this layout to
be rejected. At the end of the series, virtio_net_hdr_tnl_from_skb()
still leaves vlan_hlen out of both outer_th_offset and inner_nh_offset.

Should the sender add vlan_hlen to both offsets as part of this change?

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

[PATCH net v3 2/8] net: virtio: do not use dev->type to find the outer L3 header

[Eric Dumazet]

virtio_net_l3_start() decides from skb->dev->type whether the frame
starts with an Ethernet header. tun decides it from the IFF_TAP and
IFF_TUN flags, and TUNSETLINK can set the type of a down tun device
to any value, without changing the flags.

On an IFF_TAP device set to ARPHRD_NONE, the outer protocol checks
look at the IP header at the network header (ETH_HLEN), and never read
the Ethernet header. eth_type_trans() then sets skb->protocol from an
unchecked h_proto, for instance ETH_P_NSH, and the packet can reach
the skb_under_panic() addressed by the previous patch.

On an IFF_TUN device set to ARPHRD_ETHER, the checks validate an
Ethernet header that tun then treats as the start of the L3 header.

Checking in tun that the device type matches the framing is not
enough: the type is read again later, and a concurrent TUNSETLINK can
change it in between, since tun_get_user() only drops packets of a
down device after these checks.

Add a has_eth argument to virtio_net_hdr_tnl_to_skb(), passed down to
virtio_net_l3_start(), so the device type is no longer used:

- virtio_net always receives Ethernet frames.
- tun_get_user() passes whether the device is IFF_TAP.
- tun_xdp_one() always calls eth_type_trans() on its frames.

Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/sashiko-outbox-198573@kernel.org/
Link: https://lore.kernel.org/netdev/179387332014.488856.9111956413610107951@kernel.org/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 drivers/net/tun.c          |  9 +++++++--
 drivers/net/tun_vnet.h     |  5 +++--
 drivers/net/virtio_net.c   |  2 +-
 include/linux/virtio_net.h | 27 +++++++++++++--------------
 4 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index dcec7311e601..3920b3dff1e8 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1944,7 +1944,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		break;
 	}
 
-	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
+	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr,
+				    (tun->flags & TUN_TYPE_MASK) == IFF_TAP)) {
 		atomic_long_inc(&tun->rx_frame_errors);
 		err = -EINVAL;
 		goto free_skb;
@@ -2621,7 +2622,11 @@ static int tun_xdp_one(struct tun_struct *tun,
 	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)) {
+	/* Frames always start with an Ethernet header here, as the
+	 * eth_type_trans() call below expects.
+	 */
+	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr,
+				    true)) {
 		atomic_long_inc(&tun->rx_frame_errors);
 		kfree_skb(skb);
 		ret = -EINVAL;
diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
index f4c652b1fa44..3364e08dd6e0 100644
--- a/drivers/net/tun_vnet.h
+++ b/drivers/net/tun_vnet.h
@@ -197,9 +197,10 @@ static inline netdev_features_t tun_vnet_hdr_guest_features(int vnet_hdr_sz)
 static inline int
 tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features,
 			struct sk_buff *skb,
-			const struct virtio_net_hdr_v1_hash_tunnel *hdr)
+			const struct virtio_net_hdr_v1_hash_tunnel *hdr,
+			bool has_eth)
 {
-	return virtio_net_hdr_tnl_to_skb(skb, hdr,
+	return virtio_net_hdr_tnl_to_skb(skb, hdr, has_eth,
 				features & NETIF_F_GSO_UDP_TUNNEL,
 				features & NETIF_F_GSO_UDP_TUNNEL_CSUM,
 				tun_vnet_is_little_endian(flags));
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index dd2c41ef6fbe..ed20e76fbc4c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2533,7 +2533,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
 
 	skb->dev = dev;
 	skb_set_network_header(skb, ETH_HLEN);
-	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
+	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, true, vi->rx_tnl,
 				      vi->rx_tnl_csum,
 				      virtio_is_little_endian(vi->vdev))) {
 		net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n",
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 357116c2a148..b7b6b542f476 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -87,20 +87,18 @@ static inline struct iphdr *virtio_net_ip4_parse(struct sk_buff *skb,
 }
 
 /* Where does the L3 header start, and what protocol is it?
- * Handles Ethernet and VLAN headers; other devices use the
- * network header offset the caller set up.
+ * With @has_eth, the frame starts with an Ethernet header, possibly
+ * followed by VLAN tags. Otherwise the L3 header starts at the network
+ * header offset the caller set up.
+ *
+ * The caller knows how the frame will be parsed later: the device
+ * type does not tell, as tun can change it with TUNSETLINK.
  */
 static inline bool
-virtio_net_l3_start(struct sk_buff *skb, int *nhoff, __be16 *proto)
+virtio_net_l3_start(struct sk_buff *skb, bool has_eth, int *nhoff,
+		    __be16 *proto)
 {
-	/* skb->dev should always be set here already,
-	 * but let's not crash when it is not.
-	 */
-	if (WARN_ON_ONCE(!skb->dev))
-		return false;
-
-	if (skb->dev->type == ARPHRD_ETHER ||
-	    skb->dev->type == ARPHRD_LOOPBACK) {
+	if (has_eth) {
 		__be16 h_proto;
 
 		if (!pskb_may_pull(skb, ETH_HLEN))
@@ -431,7 +429,7 @@ static inline unsigned int virtio_l3min(bool is_ipv6)
  * carries, with UDP following it at @outer_th.
  */
 static inline bool
-virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
+virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool has_eth, bool outer_isv6,
 			  unsigned int outer_th)
 {
 	int nhoff;
@@ -439,7 +437,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
 	unsigned int thoff;
 	int ip_proto;
 
-	if (!virtio_net_l3_start(skb, &nhoff, &proto))
+	if (!virtio_net_l3_start(skb, has_eth, &nhoff, &proto))
 		return false;
 	if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
 		return false;
@@ -452,6 +450,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
 static inline int
 virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 			  const struct virtio_net_hdr_v1_hash_tunnel *vhdr,
+			  bool has_eth,
 			  bool tnl_hdr_negotiated,
 			  bool tnl_csum_negotiated,
 			  bool little_endian)
@@ -505,7 +504,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 	if (ret)
 		return ret;
 
-	if (!virtio_net_tnl_offsets_ok(skb, outer_isv6, outer_th))
+	if (!virtio_net_tnl_offsets_ok(skb, has_eth, outer_isv6, outer_th))
 		return -EINVAL;
 
 	/* In case of USO, the inner protocol is still unknown and
-- 
2.53.0

Re: [PATCH net v3 2/8] net: virtio: do not use dev->type to find the outer L3 header

[Willem de Bruijn]

Eric Dumazet wrote:
> virtio_net_l3_start() decides from skb->dev->type whether the frame
> starts with an Ethernet header. tun decides it from the IFF_TAP and
> IFF_TUN flags, and TUNSETLINK can set the type of a down tun device
> to any value, without changing the flags.
> 
> On an IFF_TAP device set to ARPHRD_NONE, the outer protocol checks
> look at the IP header at the network header (ETH_HLEN), and never read
> the Ethernet header. eth_type_trans() then sets skb->protocol from an
> unchecked h_proto, for instance ETH_P_NSH, and the packet can reach
> the skb_under_panic() addressed by the previous patch.
> 
> On an IFF_TUN device set to ARPHRD_ETHER, the checks validate an
> Ethernet header that tun then treats as the start of the L3 header.
> 
> Checking in tun that the device type matches the framing is not
> enough: the type is read again later, and a concurrent TUNSETLINK can
> change it in between, since tun_get_user() only drops packets of a
> down device after these checks.
> 
> Add a has_eth argument to virtio_net_hdr_tnl_to_skb(), passed down to
> virtio_net_l3_start(), so the device type is no longer used:
> 
> - virtio_net always receives Ethernet frames.
> - tun_get_user() passes whether the device is IFF_TAP.
> - tun_xdp_one() always calls eth_type_trans() on its frames.
> 
> Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/sashiko-outbox-198573@kernel.org/
> Link: https://lore.kernel.org/netdev/179387332014.488856.9111956413610107951@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

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

Makes sense, and easier to review stand-alone.

But if having to respin, squash into 1/8?

Also, to avoid having to respin too often could (now that they're
public and archived) leave the self-tests for a later series.

Re: [PATCH net v3 2/8] net: virtio: do not use dev->type to find the outer L3 header

[Eric Dumazet]

Le ven. 6 nov. 2026 à 05:18, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> a écrit :
>
> Eric Dumazet wrote:
> > virtio_net_l3_start() decides from skb->dev->type whether the frame
> > starts with an Ethernet header. tun decides it from the IFF_TAP and
> > IFF_TUN flags, and TUNSETLINK can set the type of a down tun device
> > to any value, without changing the flags.
> >
> > On an IFF_TAP device set to ARPHRD_NONE, the outer protocol checks
> > look at the IP header at the network header (ETH_HLEN), and never read
> > the Ethernet header. eth_type_trans() then sets skb->protocol from an
> > unchecked h_proto, for instance ETH_P_NSH, and the packet can reach
> > the skb_under_panic() addressed by the previous patch.
> >
> > On an IFF_TUN device set to ARPHRD_ETHER, the checks validate an
> > Ethernet header that tun then treats as the start of the L3 header.
> >
> > Checking in tun that the device type matches the framing is not
> > enough: the type is read again later, and a concurrent TUNSETLINK can
> > change it in between, since tun_get_user() only drops packets of a
> > down device after these checks.
> >
> > Add a has_eth argument to virtio_net_hdr_tnl_to_skb(), passed down to
> > virtio_net_l3_start(), so the device type is no longer used:
> >
> > - virtio_net always receives Ethernet frames.
> > - tun_get_user() passes whether the device is IFF_TAP.
> > - tun_xdp_one() always calls eth_type_trans() on its frames.
> >
> > Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/netdev/sashiko-outbox-198573@kernel.org/
> > Link: https://lore.kernel.org/netdev/179387332014.488856.9111956413610107951@kernel.org/
> > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
>
> Makes sense, and easier to review stand-alone.
>
> But if having to respin, squash into 1/8?
>
> Also, to avoid having to respin too often could (now that they're
> public and archived) leave the self-tests for a later series.

Thanks Willem, v4 does both: this patch is squashed into the first
one, and the selftests are dropped from the series. I will send them
separately, with the fixes Sashiko found in patches 7 and 8.

pw-bot: cr

Re: [PATCH net v3 2/8] net: virtio: do not use dev->type to find the outer L3 header

[Michael S. Tsirkin]

On Thu, Nov 05, 2026 at 12:57:09PM +0100, Eric Dumazet wrote:
> virtio_net_l3_start() decides from skb->dev->type whether the frame
> starts with an Ethernet header. tun decides it from the IFF_TAP and
> IFF_TUN flags, and TUNSETLINK can set the type of a down tun device
> to any value, without changing the flags.
> 
> On an IFF_TAP device set to ARPHRD_NONE, the outer protocol checks
> look at the IP header at the network header (ETH_HLEN), and never read
> the Ethernet header. eth_type_trans() then sets skb->protocol from an
> unchecked h_proto, for instance ETH_P_NSH, and the packet can reach
> the skb_under_panic() addressed by the previous patch.
> 
> On an IFF_TUN device set to ARPHRD_ETHER, the checks validate an
> Ethernet header that tun then treats as the start of the L3 header.
> 
> Checking in tun that the device type matches the framing is not
> enough: the type is read again later, and a concurrent TUNSETLINK can
> change it in between, since tun_get_user() only drops packets of a
> down device after these checks.
> 
> Add a has_eth argument to virtio_net_hdr_tnl_to_skb(), passed down to
> virtio_net_l3_start(), so the device type is no longer used:
> 
> - virtio_net always receives Ethernet frames.
> - tun_get_user() passes whether the device is IFF_TAP.
> - tun_xdp_one() always calls eth_type_trans() on its frames.
> 
> Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/sashiko-outbox-198573@kernel.org/
> Link: https://lore.kernel.org/netdev/179387332014.488856.9111956413610107951@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/tun.c          |  9 +++++++--
>  drivers/net/tun_vnet.h     |  5 +++--
>  drivers/net/virtio_net.c   |  2 +-
>  include/linux/virtio_net.h | 27 +++++++++++++--------------
>  4 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index dcec7311e601..3920b3dff1e8 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1944,7 +1944,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  		break;
>  	}
>  
> -	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> +	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr,
> +				    (tun->flags & TUN_TYPE_MASK) == IFF_TAP)) {
>  		atomic_long_inc(&tun->rx_frame_errors);
>  		err = -EINVAL;
>  		goto free_skb;
> @@ -2621,7 +2622,11 @@ static int tun_xdp_one(struct tun_struct *tun,
>  	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)) {
> +	/* Frames always start with an Ethernet header here, as the
> +	 * eth_type_trans() call below expects.
> +	 */
> +	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr,
> +				    true)) {
>  		atomic_long_inc(&tun->rx_frame_errors);
>  		kfree_skb(skb);
>  		ret = -EINVAL;
> diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
> index f4c652b1fa44..3364e08dd6e0 100644
> --- a/drivers/net/tun_vnet.h
> +++ b/drivers/net/tun_vnet.h
> @@ -197,9 +197,10 @@ static inline netdev_features_t tun_vnet_hdr_guest_features(int vnet_hdr_sz)
>  static inline int
>  tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features,
>  			struct sk_buff *skb,
> -			const struct virtio_net_hdr_v1_hash_tunnel *hdr)
> +			const struct virtio_net_hdr_v1_hash_tunnel *hdr,
> +			bool has_eth)
>  {
> -	return virtio_net_hdr_tnl_to_skb(skb, hdr,
> +	return virtio_net_hdr_tnl_to_skb(skb, hdr, has_eth,
>  				features & NETIF_F_GSO_UDP_TUNNEL,
>  				features & NETIF_F_GSO_UDP_TUNNEL_CSUM,
>  				tun_vnet_is_little_endian(flags));
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index dd2c41ef6fbe..ed20e76fbc4c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2533,7 +2533,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
>  
>  	skb->dev = dev;
>  	skb_set_network_header(skb, ETH_HLEN);
> -	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
> +	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, true, vi->rx_tnl,
>  				      vi->rx_tnl_csum,
>  				      virtio_is_little_endian(vi->vdev))) {
>  		net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n",
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 357116c2a148..b7b6b542f476 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -87,20 +87,18 @@ static inline struct iphdr *virtio_net_ip4_parse(struct sk_buff *skb,
>  }
>  
>  /* Where does the L3 header start, and what protocol is it?
> - * Handles Ethernet and VLAN headers; other devices use the
> - * network header offset the caller set up.
> + * With @has_eth, the frame starts with an Ethernet header, possibly
> + * followed by VLAN tags. Otherwise the L3 header starts at the network
> + * header offset the caller set up.
> + *
> + * The caller knows how the frame will be parsed later: the device
> + * type does not tell, as tun can change it with TUNSETLINK.
>   */
>  static inline bool
> -virtio_net_l3_start(struct sk_buff *skb, int *nhoff, __be16 *proto)
> +virtio_net_l3_start(struct sk_buff *skb, bool has_eth, int *nhoff,
> +		    __be16 *proto)
>  {
> -	/* skb->dev should always be set here already,
> -	 * but let's not crash when it is not.
> -	 */
> -	if (WARN_ON_ONCE(!skb->dev))
> -		return false;
> -
> -	if (skb->dev->type == ARPHRD_ETHER ||
> -	    skb->dev->type == ARPHRD_LOOPBACK) {
> +	if (has_eth) {
>  		__be16 h_proto;
>  
>  		if (!pskb_may_pull(skb, ETH_HLEN))
> @@ -431,7 +429,7 @@ static inline unsigned int virtio_l3min(bool is_ipv6)
>   * carries, with UDP following it at @outer_th.
>   */
>  static inline bool
> -virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
> +virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool has_eth, bool outer_isv6,
>  			  unsigned int outer_th)
>  {
>  	int nhoff;
> @@ -439,7 +437,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
>  	unsigned int thoff;
>  	int ip_proto;
>  
> -	if (!virtio_net_l3_start(skb, &nhoff, &proto))
> +	if (!virtio_net_l3_start(skb, has_eth, &nhoff, &proto))
>  		return false;
>  	if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
>  		return false;
> @@ -452,6 +450,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
>  static inline int
>  virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>  			  const struct virtio_net_hdr_v1_hash_tunnel *vhdr,
> +			  bool has_eth,
>  			  bool tnl_hdr_negotiated,
>  			  bool tnl_csum_negotiated,
>  			  bool little_endian)
> @@ -505,7 +504,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>  	if (ret)
>  		return ret;
>  
> -	if (!virtio_net_tnl_offsets_ok(skb, outer_isv6, outer_th))
> +	if (!virtio_net_tnl_offsets_ok(skb, has_eth, outer_isv6, outer_th))
>  		return -EINVAL;
>  
>  	/* In case of USO, the inner protocol is still unknown and
> -- 
> 2.53.0

Re: [PATCH net v3 2/8] net: virtio: do not use dev->type to find the outer L3 header

[Eric Dumazet]

Le ven. 6 nov. 2026 à 10:28, Michael S. Tsirkin <mst@redhat.com> a écrit :
>
> On Thu, Nov 05, 2026 at 12:57:09PM +0100, Eric Dumazet wrote:
> > virtio_net_l3_start() decides from skb->dev->type whether the frame
> > starts with an Ethernet header. tun decides it from the IFF_TAP and
> > IFF_TUN flags, and TUNSETLINK can set the type of a down tun device
> > to any value, without changing the flags.
> >
> > On an IFF_TAP device set to ARPHRD_NONE, the outer protocol checks
> > look at the IP header at the network header (ETH_HLEN), and never read
> > the Ethernet header. eth_type_trans() then sets skb->protocol from an
> > unchecked h_proto, for instance ETH_P_NSH, and the packet can reach
> > the skb_under_panic() addressed by the previous patch.
> >
> > On an IFF_TUN device set to ARPHRD_ETHER, the checks validate an
> > Ethernet header that tun then treats as the start of the L3 header.
> >
> > Checking in tun that the device type matches the framing is not
> > enough: the type is read again later, and a concurrent TUNSETLINK can
> > change it in between, since tun_get_user() only drops packets of a
> > down device after these checks.
> >
> > Add a has_eth argument to virtio_net_hdr_tnl_to_skb(), passed down to
> > virtio_net_l3_start(), so the device type is no longer used:
> >
> > - virtio_net always receives Ethernet frames.
> > - tun_get_user() passes whether the device is IFF_TAP.
> > - tun_xdp_one() always calls eth_type_trans() on its frames.
> >
> > Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/netdev/sashiko-outbox-198573@kernel.org/
> > Link: https://lore.kernel.org/netdev/179387332014.488856.9111956413610107951@kernel.org/
> > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Sorry, I saw this reply too late, I sent v4 without it.

Re: [PATCH net v3 2/8] net: virtio: do not use dev->type to find the outer L3 header

[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

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] net: virtio: outer_l3min includes ETH_HLEN in
  virtio_net_hdr_tnl_to_skb()

Pre-existing issues:
- [Medium] This patch makes Ethernet framing an explicit argument
  (`has_eth`), but virtio_net_hdr_tnl_to_skb() in…

--

net: virtio: do not use dev->type to find the outer L3 header

virtio_net_l3_start() used skb->dev->type to decide whether a frame
starts with an Ethernet header. tun can change that type with
TUNSETLINK, so this patch adds a has_eth argument to
virtio_net_hdr_tnl_to_skb() instead. Callers now pass true for
virtio_net and tun_xdp_one(), and pass whether the device is IFF_TAP
in tun_get_user().

Link: https://lore.kernel.org/netdev/179387332014.488856.9111956413610107951@kernel.org/

> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index dcec7311e6011..3920b3dff1e84 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1944,7 +1944,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  		break;
>  	}
>  
> -	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> +	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr,
> +				    (tun->flags & TUN_TYPE_MASK) == IFF_TAP)) {
>  		atomic_long_inc(&tun->rx_frame_errors);
>  		err = -EINVAL;
>  		goto free_skb;

[ ... ]

> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 357116c2a1489..b7b6b542f4763 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h

[ ... ]

> @@ -452,6 +450,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
>  static inline int
>  virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>  			  const struct virtio_net_hdr_v1_hash_tunnel *vhdr,
> +			  bool has_eth,
>  			  bool tnl_hdr_negotiated,
>  			  bool tnl_csum_negotiated,
>  			  bool little_endian)
> @@ -505,7 +504,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>  	if (ret)
>  		return ret;
>  
> -	if (!virtio_net_tnl_offsets_ok(skb, outer_isv6, outer_th))
> +	if (!virtio_net_tnl_offsets_ok(skb, has_eth, outer_isv6, outer_th))
>  		return -EINVAL;

[Severity: Medium]
Now that the framing is passed in as has_eth, should the minimum outer
transport offset check earlier in virtio_net_hdr_tnl_to_skb() depend
on it as well? outer_l3min is still computed as ETH_HLEN +
virtio_l3min(outer_isv6), so outer_th must be at least 34 for IPv4 or
54 for IPv6, whatever has_eth says.

On an IFF_TUN device, tun_get_user() calls skb_reset_network_header()
and passes has_eth=false. virtio_net_l3_start() then returns nhoff = 0,
and virtio_net_l4_start() computes thoff = 20 for a plain IPv4 outer
header, or 40 for a plain IPv6 one. virtio_net_tnl_offsets_ok() then
requires outer_th == thoff, but the floor above has already rejected
both values.

As a result, the has_eth=false path only accepts frames whose outer L3
header is at least ETH_HLEN bytes longer than the minimum, such as IPv4
with options or IPv6 with extension headers. Would ordinary UDP tunnel
GSO frames written to an IFF_TUN fd get -EINVAL and be counted in
rx_frame_errors?

The transmit side of the same ABI produces exactly those offsets: in
TUN mode virtio_net_hdr_tnl_from_skb() fills outer_th_offset from
skb->transport_header - skb_headroom(skb), which is 20 for an L3-first
IPv4 frame. set_offload() also enables NETIF_F_GSO_UDP_TUNNEL without
looking at IFF_TAP or IFF_TUN, so IFF_TUN devices reach this path.

This isn't introduced by this patch, but since has_eth is now available
here, would something like this work?

	outer_l3min = (has_eth ? ETH_HLEN : 0) +
		      virtio_l3min(outer_isv6);

[Severity: Medium]
This isn't a bug introduced by this patch, but now that the framing is
passed in as has_eth, should the minimum outer transport offset in
virtio_net_hdr_tnl_to_skb() depend on it too? That check comes earlier
in the same function and still assumes an Ethernet header:

include/linux/virtio_net.h:virtio_net_hdr_tnl_to_skb() {
    ...
	outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6);
    ...
	if (outer_th < outer_l3min ||
	    inner_nh < outer_th + sizeof(struct udphdr) ||
	    inner_th < inner_nh + inner_l3min)
		return -EINVAL;
    ...
}

On an IFF_TUN device, tun_get_user() resets the network header and
passes has_eth=false. virtio_net_l3_start() then returns nhoff = 0, and
virtio_net_tnl_offsets_ok() requires outer_th == thoff. For a plain
outer header, thoff is 20 for IPv4 and 40 for IPv6. The floor above has
already rejected both values, since it is 34 or 54.

As a result, the has_eth=false path only accepts frames whose outer L3
header is at least ETH_HLEN bytes longer than the minimum, such as IPv4
with options or IPv6 with extension headers. Would ordinary UDP tunnel
GSO frames written to an IFF_TUN fd get -EINVAL and be counted in
rx_frame_errors?

The transmit side of the same ABI seems to produce exactly those
offsets. In TUN mode, tun_put_user() ends up in
virtio_net_hdr_tnl_from_skb(), which writes:

	outer_th = skb->transport_header - skb_headroom(skb);
	vhdr->inner_nh_offset = cpu_to_le16(inner_nh);
	vhdr->outer_th_offset = cpu_to_le16(outer_th);

For an L3-first frame, that gives 20 for IPv4. Also, set_offload() in
tun.c enables NETIF_F_GSO_UDP_TUNNEL without checking IFF_TAP or
IFF_TUN, so IFF_TUN devices can reach this receive path.

The selftest commit in this series also mentions the limitation: "The
outer IPv4 header carries options, as virtio_net_hdr_tnl_to_skb()
expects room for an Ethernet header before the outer transport header."

Would something like this work here?

	outer_l3min = (has_eth ? ETH_HLEN : 0) + virtio_l3min(outer_isv6);

>  
>  	/* In case of USO, the inner protocol is still unknown and

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

[PATCH net v3 3/8] tun: clear stale skb->protocol of NAPI frags skbs

[Eric Dumazet]

With IFF_NAPI_FRAGS, tun_napi_alloc_frags() gets its skb from
napi_get_frags(). If GRO merged the previous frame with
GRO_MERGED_FREE, that skb was recycled by napi_reuse_skb(), which does
not reset skb->protocol set by napi_frags_skb() for the previous frame.

On the IFF_TAP path, tun_get_user() calls tun_vnet_hdr_tnl_to_skb()
before eth_type_trans(), so the virtio header checks see the protocol
of the previous frame: the outer protocol check of UDP tunnel GSO
packets rejects a valid frame when the two ethertypes differ, and
GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are dissected with
the wrong protocol.

This needs GRO to hold packets across writes, for example with
napi_defer_hard_irqs and gro_flush_timeout set on the device.

Clear skb->protocol for IFF_TAP frames, as for skbs allocated by
tun_alloc_skb(). eth_type_trans() sets it right after.

Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/179386753589.488856.11455181721004759454@kernel.org/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 drivers/net/tun.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3920b3dff1e8..6b648f21018c 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1941,6 +1941,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			goto drop;
 		}
 		skb_set_network_header(skb, ETH_HLEN);
+		/* A NAPI frags skb recycled by napi_reuse_skb() keeps the
+		 * protocol of a previous frame. eth_type_trans() sets it
+		 * below, the virtio header checks must not see a stale one.
+		 */
+		skb->protocol = 0;
 		break;
 	}
 
-- 
2.53.0

Re: [PATCH net v3 3/8] tun: clear stale skb->protocol of NAPI frags skbs

[Willem de Bruijn]

Eric Dumazet wrote:
> With IFF_NAPI_FRAGS, tun_napi_alloc_frags() gets its skb from
> napi_get_frags(). If GRO merged the previous frame with
> GRO_MERGED_FREE, that skb was recycled by napi_reuse_skb(), which does
> not reset skb->protocol set by napi_frags_skb() for the previous frame.
> 
> On the IFF_TAP path, tun_get_user() calls tun_vnet_hdr_tnl_to_skb()
> before eth_type_trans(), so the virtio header checks see the protocol
> of the previous frame: the outer protocol check of UDP tunnel GSO
> packets rejects a valid frame when the two ethertypes differ, and
> GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are dissected with
> the wrong protocol.
> 
> This needs GRO to hold packets across writes, for example with
> napi_defer_hard_irqs and gro_flush_timeout set on the device.
> 
> Clear skb->protocol for IFF_TAP frames, as for skbs allocated by
> tun_alloc_skb(). eth_type_trans() sets it right after.
> 
> Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/179386753589.488856.11455181721004759454@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

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

> ---
>  drivers/net/tun.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 3920b3dff1e8..6b648f21018c 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1941,6 +1941,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  			goto drop;
>  		}
>  		skb_set_network_header(skb, ETH_HLEN);
> +		/* A NAPI frags skb recycled by napi_reuse_skb() keeps the
> +		 * protocol of a previous frame. eth_type_trans() sets it
> +		 * below, the virtio header checks must not see a stale one.
> +		 */
> +		skb->protocol = 0;

This resets it for all IFF_TAP.

Would it make sense to do this in tun_napi_alloc_frags if it is only
needed for IFF_NAPI_FRAGS?

>  		break;
>  	}
>  
> -- 
> 2.53.0
> 

Re: [PATCH net v3 3/8] tun: clear stale skb->protocol of NAPI frags skbs

[Eric Dumazet]

Le ven. 6 nov. 2026 à 05:21, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> a écrit :
>
> Eric Dumazet wrote:
> > With IFF_NAPI_FRAGS, tun_napi_alloc_frags() gets its skb from
> > napi_get_frags(). If GRO merged the previous frame with
> > GRO_MERGED_FREE, that skb was recycled by napi_reuse_skb(), which does
> > not reset skb->protocol set by napi_frags_skb() for the previous frame.
> >
> > On the IFF_TAP path, tun_get_user() calls tun_vnet_hdr_tnl_to_skb()
> > before eth_type_trans(), so the virtio header checks see the protocol
> > of the previous frame: the outer protocol check of UDP tunnel GSO
> > packets rejects a valid frame when the two ethertypes differ, and
> > GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are dissected with
> > the wrong protocol.
> >
> > This needs GRO to hold packets across writes, for example with
> > napi_defer_hard_irqs and gro_flush_timeout set on the device.
> >
> > Clear skb->protocol for IFF_TAP frames, as for skbs allocated by
> > tun_alloc_skb(). eth_type_trans() sets it right after.
> >
> > Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/netdev/179386753589.488856.11455181721004759454@kernel.org/
> > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
>
> > ---
> >  drivers/net/tun.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 3920b3dff1e8..6b648f21018c 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -1941,6 +1941,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >                       goto drop;
> >               }
> >               skb_set_network_header(skb, ETH_HLEN);
> > +             /* A NAPI frags skb recycled by napi_reuse_skb() keeps the
> > +              * protocol of a previous frame. eth_type_trans() sets it
> > +              * below, the virtio header checks must not see a stale one.
> > +              */
> > +             skb->protocol = 0;
>
> This resets it for all IFF_TAP.
>
> Would it make sense to do this in tun_napi_alloc_frags if it is only
> needed for IFF_NAPI_FRAGS?

Yes, other skbs come from tun_alloc_skb() or build_skb() with a zero
protocol. Done in v4, where I kept your Reviewed-by.

Re: [PATCH net v3 3/8] tun: clear stale skb->protocol of NAPI frags skbs

[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 commit message understates what a stale skb->protocol does in
  the non-tunnel path.
- [Low] The first symptom in the commit message ("the outer protocol
  check of UDP tunnel GSO packets rejects a valid frame when the two…

--

tun: clear stale skb->protocol of NAPI frags skbs

This patch clears skb->protocol for IFF_TAP frames in tun_get_user()
before tun_vnet_hdr_tnl_to_skb() runs. With IFF_NAPI_FRAGS, an skb
recycled by napi_reuse_skb() keeps the protocol that napi_frags_skb() set
for the previous frame. Without this patch, the virtio header checks see
that stale value.

Closes: https://lore.kernel.org/netdev/179386753589.488856.11455181721004759454@kernel.org/

[ ... ]

> On the IFF_TAP path, tun_get_user() calls tun_vnet_hdr_tnl_to_skb()
> before eth_type_trans(), so the virtio header checks see the protocol
> of the previous frame: the outer protocol check of UDP tunnel GSO
> packets rejects a valid frame when the two ethertypes differ, and

[Severity: Low]
Does this symptom really come from 90e33d459407? The outer protocol check
that compares against skb->protocol seems to have been added earlier in
this series, by "net: virtio: validate outer protocol of UDP tunnel GSO
packets":

include/linux/virtio_net.h:virtio_net_l3_start() {
    ...
		h_proto = eth_hdr(skb)->h_proto;
		...
		if (skb->protocol && skb->protocol != h_proto)
			return false;
    ...
}

At the baseline, the tunnel path in virtio_net_hdr_tnl_to_skb() never
read skb->protocol. A stale value could not cause a tunnel rejection
there.

With the current ordering, the two intermediate commits ("net: virtio:
validate outer protocol of UDP tunnel GSO packets" and "net: virtio: do
not use dev->type to find the outer L3 header") would drop a valid UDP
tunnel GSO frame with -EINVAL. This happens on an IFF_TAP |
IFF_NAPI_FRAGS device when GRO holds packets and the outer ethertype
differs from the one in the previous merged frame, for example an IPv4
outer after an IPv6 frame:

tun_get_user()
  tun_vnet_hdr_tnl_to_skb()
    virtio_net_hdr_tnl_to_skb()
      virtio_net_tnl_offsets_ok()
        virtio_net_l3_start()   <- stale skb->protocol != h_proto

Could this patch move ahead of "net: virtio: validate outer protocol of
UDP tunnel GSO packets" so the series stays bisectable? If not, could
the commit message at least mention the dependency on that commit?

> GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are dissected with
> the wrong protocol.

[Severity: Low]
Is "dissected with the wrong protocol" too mild for what happens here?
In __virtio_net_hdr_to_skb(), a stale non-zero skb->protocol means
dev_parse_header_protocol() never reads the real h_proto of the frame:

include/linux/virtio_net.h:__virtio_net_hdr_to_skb() {
    ...
	} else if (gso_type) {
		...
		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)) {
		...
		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;
    ...
}

When data is NULL, the dissector starts from skb->protocol at
skb_network_offset(), which tun_get_user() sets to ETH_HLEN. So
virtio_net_hdr_match_proto() checks the protocol of the previous frame,
not the ethertype of this one.

A writer could then send a VIRTIO_NET_HDR_GSO_TCPV4 frame whose h_proto
is something else, such as MPLS or NSH, with what looks like an
IPv4/TCP payload at offset 14. That frame would pass validation and get
SKB_GSO_TCPV4 | SKB_GSO_DODGY. eth_type_trans() would then set
skb->protocol to the h_proto that was never checked.

The code change closes this. Could the commit message describe it as a
bypass of the check that the ethertype matches the gso_type, rather than
just a misparse?

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

[PATCH net v3 4/8] selftests: net: tuntap: fix UDP checksum length in build_udp_packet_csum()

[Eric Dumazet]

build_udp_packet_csum() passes udph->len, which is in network byte
order, as the number of bytes to checksum. On little-endian hosts this
reads up to 64KB past the UDP header and computes a wrong checksum.

Existing callers did not crash only because they use packet buffers
of MAX_VNET_TUNNEL_PACKET_SZ bytes.

Use ntohs(udph->len), as tap.c does in its own build_udp_packet().

While at it, clear udph->check before summing instead of relying on
the caller, and send 0xffff instead of a computed zero checksum, as
the kernel does with CSUM_MANGLED_0.

Fixes: a942fcd72e97 ("selftest: tun: Introduce tuntap_helpers.h header for TUN/TAP testing")
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 tools/testing/selftests/net/tuntap_helpers.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/tuntap_helpers.h b/tools/testing/selftests/net/tuntap_helpers.h
index d6c0437136ec..ecad83fae158 100644
--- a/tools/testing/selftests/net/tuntap_helpers.h
+++ b/tools/testing/selftests/net/tuntap_helpers.h
@@ -330,14 +330,22 @@ static inline void build_udp_packet_csum(uint8_t *buf, int family,
 	size_t ipalen = ip_addr_len(family);
 	uint32_t sum;
 
+	udph->check = 0;
+
 	/* No extension IPv4 and IPv6 headers addresses are the last fields */
 	sum = add_csum(buf - 2 * ipalen, 2 * ipalen);
 	sum += htons(IPPROTO_UDP) + udph->len;
 
 	if (!csum_off)
-		sum += add_csum(buf, udph->len);
+		sum += add_csum(buf, ntohs(udph->len));
 
 	udph->check = finish_ip_csum(sum);
+
+	/* A zero UDP checksum means no checksum: send 0xffff instead,
+	 * as the kernel does with CSUM_MANGLED_0.
+	 */
+	if (!csum_off && !udph->check)
+		udph->check = 0xffff;
 }
 
 static inline int build_udp_packet(uint8_t *buf, uint16_t sport, uint16_t dport,
-- 
2.53.0

[PATCH net v3 5/8] selftests: net: tun: test outer protocol of UDP tunnel GSO packets

[Eric Dumazet]

Add tests writing single 4in4 Geneve GSO frames to tun devices with
UDP tunnel offloads enabled.

tun_vnet_udptnl_proto uses a tap device:

- outer_proto_ok: an outer IPv4 header, with or without an 802.1Q tag,
  is accepted.
- outer_proto_bad: an outer ethertype that does not match the declared
  tunnel type (IPv6 on an IPv4 tunnel, or NSH with or without a VLAN
  tag) is rejected with EINVAL.

tun_vnet_udptnl_proto_l3 uses an IFF_TUN device with a tun_pi header,
so skb->protocol comes from userspace and there is no Ethernet header.
The outer IPv4 header carries options, as virtio_net_hdr_tnl_to_skb()
expects room for an Ethernet header before the outer transport header.

- outer_proto_ok: tun_pi and the frame agree on IPv4, accepted.
- outer_proto_bad: tun_pi claims IPv6, or the IP version of the frame
  is not 4 while tun_pi claims IPv4: rejected with EINVAL.

Without the first patch of this series, both outer_proto_bad tests fail
because the kernel accepts the frames.

Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 tools/testing/selftests/net/tun.c | 269 ++++++++++++++++++++++++++++++
 1 file changed, 269 insertions(+)

diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
index 34de2580e32e..81410afbbcc8 100644
--- a/tools/testing/selftests/net/tun.c
+++ b/tools/testing/selftests/net/tun.c
@@ -683,6 +683,275 @@ TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum_64k)
 	ASSERT_EQ(ret, len);
 }
 
+#ifndef ETH_P_NSH
+#define ETH_P_NSH 0x894F
+#endif
+
+FIXTURE(tun_vnet_udptnl_proto)
+{
+	char ifname[IFNAMSIZ];
+	uint8_t *pkt;
+	int fd;
+};
+
+FIXTURE_SETUP(tun_vnet_udptnl_proto)
+{
+	int flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
+	int features = TUN_F_CSUM | TUN_F_UDP_TUNNEL_GSO |
+		       TUN_F_UDP_TUNNEL_GSO_CSUM | TUN_F_USO4 | TUN_F_USO6;
+
+	self->fd = -1;
+	if (unshare(CLONE_NEWNET))
+		SKIP(return, "Cannot create network namespace: %s",
+		     strerror(errno));
+
+	self->pkt = calloc(1, MAX_VNET_TUNNEL_PACKET_SZ);
+	ASSERT_NE(self->pkt, NULL);
+
+	memset(self->ifname, 0, sizeof(self->ifname));
+	self->fd = tun_open(self->ifname, flags, TUN_VNET_TNL_SIZE, features,
+			    param_hwaddr_outer_src);
+	ASSERT_GE(self->fd, 0);
+}
+
+FIXTURE_TEARDOWN(tun_vnet_udptnl_proto)
+{
+	if (self->fd >= 0)
+		close(self->fd);
+	free(self->pkt);
+}
+
+/* Build a 4in4 Geneve GSO frame whose outer ethertype is @l3proto,
+ * optionally behind an 802.1Q tag.
+ */
+static int build_udptnl_frame(uint8_t *buf, bool vlan, uint16_t l3proto,
+			      int payload_len)
+{
+	struct virtio_net_hdr_v1_hash_tunnel *vh = (void *)buf;
+	int vlan_len = vlan ? 4 : 0;
+	int hlen = ETH_HLEN + vlan_len + UDP_TUNNEL_GENEVE_4IN4_HDRLEN;
+	uint8_t *outer_udph, *cur = buf;
+	int pktlen;
+
+	/* The buffer is reused: do not leave bytes from a previous frame. */
+	memset(buf, 0, TUN_VNET_TNL_SIZE + hlen + payload_len);
+
+	cur += build_virtio_net_hdr_v1_hash_tunnel(cur, true, hlen, 1400,
+						   AF_INET, AF_INET);
+	vh->outer_th_offset += vlan_len;
+	vh->inner_nh_offset += vlan_len;
+	vh->hash_hdr.hdr.csum_start += vlan_len;
+
+	if (vlan) {
+		uint16_t vlan_tag[2] = { htons(100), htons(l3proto) };
+
+		cur += build_eth(cur, ETH_P_8021Q, param_hwaddr_outer_dst,
+				 param_hwaddr_outer_src);
+		memcpy(cur, vlan_tag, sizeof(vlan_tag));
+		cur += sizeof(vlan_tag);
+	} else {
+		cur += build_eth(cur, l3proto, param_hwaddr_outer_dst,
+				 param_hwaddr_outer_src);
+	}
+
+	pktlen = UDP_TUNNEL_GENEVE_4IN4_HDRLEN - sizeof(struct iphdr) +
+		 payload_len;
+	cur += build_ipv4_header(cur, IPPROTO_UDP, pktlen,
+				 &param_ipaddr4_outer_dst,
+				 &param_ipaddr4_outer_src);
+
+	outer_udph = cur;
+	cur += build_udp_header(cur, UDP_SRC_PORT, VN_PORT,
+				pktlen - sizeof(struct udphdr));
+	cur += build_geneve_header(cur, VN_ID);
+	cur += build_eth(cur, ETH_P_IP, param_hwaddr_inner_dst,
+			 param_hwaddr_inner_src);
+	cur += build_ipv4_header(cur, IPPROTO_UDP,
+				 sizeof(struct udphdr) + payload_len,
+				 &param_ipaddr4_inner_dst,
+				 &param_ipaddr4_inner_src);
+	cur += build_udp_packet(cur, UDP_DST_PORT, UDP_SRC_PORT, payload_len,
+				AF_INET, false);
+	build_udp_packet_csum(outer_udph, AF_INET, false);
+
+	return cur - buf;
+}
+
+TEST_F(tun_vnet_udptnl_proto, outer_proto_ok)
+{
+	uint8_t *pkt = self->pkt;
+	int len, ret;
+
+	len = build_udptnl_frame(pkt, false, ETH_P_IP, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, len);
+
+	/* The outer IPv4 header can follow a VLAN tag. */
+	len = build_udptnl_frame(pkt, true, ETH_P_IP, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, len);
+}
+
+TEST_F(tun_vnet_udptnl_proto, outer_proto_bad)
+{
+	uint8_t *pkt = self->pkt;
+	int len, ret;
+
+	/* Outer ethertype does not match the declared IPv4 tunnel. */
+	len = build_udptnl_frame(pkt, false, ETH_P_IPV6, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	/* Outer header is not IP at all. */
+	len = build_udptnl_frame(pkt, false, ETH_P_NSH, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	/* Same, behind a VLAN tag. */
+	len = build_udptnl_frame(pkt, true, ETH_P_NSH, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
+/* IFF_TUN device with a tun_pi header: skb->protocol comes from
+ * userspace, and there is no Ethernet header.
+ */
+FIXTURE(tun_vnet_udptnl_proto_l3)
+{
+	char ifname[IFNAMSIZ];
+	uint8_t *pkt;
+	int fd;
+};
+
+FIXTURE_SETUP(tun_vnet_udptnl_proto_l3)
+{
+	int flags = IFF_TUN | IFF_VNET_HDR;
+	int features = TUN_F_CSUM | TUN_F_UDP_TUNNEL_GSO |
+		       TUN_F_UDP_TUNNEL_GSO_CSUM | TUN_F_USO4 | TUN_F_USO6;
+
+	self->fd = -1;
+	if (unshare(CLONE_NEWNET))
+		SKIP(return, "Cannot create network namespace: %s",
+		     strerror(errno));
+
+	self->pkt = calloc(1, MAX_VNET_TUNNEL_PACKET_SZ);
+	ASSERT_NE(self->pkt, NULL);
+
+	memset(self->ifname, 0, sizeof(self->ifname));
+	self->fd = tun_open(self->ifname, flags, TUN_VNET_TNL_SIZE, features,
+			    NULL);
+	ASSERT_GE(self->fd, 0);
+}
+
+FIXTURE_TEARDOWN(tun_vnet_udptnl_proto_l3)
+{
+	if (self->fd >= 0)
+		close(self->fd);
+	free(self->pkt);
+}
+
+#define IPV4_OPT_NOP 1
+#define IPV4_OPT_LEN 16
+
+/* Build a tun_pi header carrying @pi_proto, followed by a 4in4 Geneve
+ * GSO frame whose outer IP header claims version @ip_version.
+ *
+ * virtio_net_hdr_tnl_to_skb() wants room for an Ethernet header before
+ * the outer transport header, even without one: pad the outer IPv4
+ * header with NOP options.
+ */
+static int build_udptnl_l3_frame(uint8_t *buf, uint16_t pi_proto,
+				 int ip_version, int payload_len)
+{
+	struct tun_pi *pi = (struct tun_pi *)buf;
+	uint8_t *vnet = buf + sizeof(*pi);
+	struct virtio_net_hdr_v1_hash_tunnel *vh = (void *)vnet;
+	int hlen = UDP_TUNNEL_GENEVE_4IN4_HDRLEN + IPV4_OPT_LEN;
+	uint8_t *cur, *outer_iph, *outer_udph;
+	struct iphdr *iph;
+	int pktlen;
+
+	/* The buffer is reused: do not leave bytes from a previous frame. */
+	memset(buf, 0, sizeof(*pi) + TUN_VNET_TNL_SIZE + hlen + payload_len);
+	pi->proto = htons(pi_proto);
+
+	cur = vnet + build_virtio_net_hdr_v1_hash_tunnel(vnet, false, hlen,
+							 1400, AF_INET,
+							 AF_INET);
+	vh->outer_th_offset += IPV4_OPT_LEN;
+	vh->inner_nh_offset += IPV4_OPT_LEN;
+	vh->hash_hdr.hdr.csum_start += IPV4_OPT_LEN;
+
+	/* Build the frame without options first: build_udp_packet_csum()
+	 * expects the IP addresses right before the UDP header.
+	 */
+	outer_iph = cur;
+	pktlen = UDP_TUNNEL_GENEVE_4IN4_HDRLEN - sizeof(struct iphdr) +
+		 payload_len;
+	cur += build_ipv4_header(cur, IPPROTO_UDP, pktlen,
+				 &param_ipaddr4_outer_dst,
+				 &param_ipaddr4_outer_src);
+
+	outer_udph = cur;
+	cur += build_udp_header(cur, UDP_SRC_PORT, VN_PORT,
+				pktlen - sizeof(struct udphdr));
+	cur += build_geneve_header(cur, VN_ID);
+	cur += build_eth(cur, ETH_P_IP, param_hwaddr_inner_dst,
+			 param_hwaddr_inner_src);
+	cur += build_ipv4_header(cur, IPPROTO_UDP,
+				 sizeof(struct udphdr) + payload_len,
+				 &param_ipaddr4_inner_dst,
+				 &param_ipaddr4_inner_src);
+	cur += build_udp_packet(cur, UDP_DST_PORT, UDP_SRC_PORT, payload_len,
+				AF_INET, false);
+	build_udp_packet_csum(outer_udph, AF_INET, false);
+
+	/* Then insert the options after the fixed outer IPv4 header. */
+	memmove(outer_udph + IPV4_OPT_LEN, outer_udph, cur - outer_udph);
+	memset(outer_udph, IPV4_OPT_NOP, IPV4_OPT_LEN);
+	cur += IPV4_OPT_LEN;
+
+	iph = (struct iphdr *)outer_iph;
+	iph->version = ip_version;
+	iph->ihl += IPV4_OPT_LEN / 4;
+	iph->tot_len = htons(ntohs(iph->tot_len) + IPV4_OPT_LEN);
+	iph->check = 0;
+	iph->check = build_ip_csum(outer_iph, iph->ihl << 2, 0);
+
+	return cur - buf;
+}
+
+TEST_F(tun_vnet_udptnl_proto_l3, outer_proto_ok)
+{
+	uint8_t *pkt = self->pkt;
+	int len, ret;
+
+	len = build_udptnl_l3_frame(pkt, ETH_P_IP, 4, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, len);
+}
+
+TEST_F(tun_vnet_udptnl_proto_l3, outer_proto_bad)
+{
+	uint8_t *pkt = self->pkt;
+	int len, ret;
+
+	/* tun_pi protocol does not match the declared IPv4 tunnel. */
+	len = build_udptnl_l3_frame(pkt, ETH_P_IPV6, 4, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	/* tun_pi claims IPv4, but the frame says otherwise. */
+	len = build_udptnl_l3_frame(pkt, ETH_P_IP, 6, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
 FIXTURE(tun_vnet_udptnl)
 {
 	char ifname[IFNAMSIZ];
-- 
2.53.0

[PATCH net v3 6/8] selftests: net: tun: test UDP tunnel GSO on retyped devices

[Eric Dumazet]

Use TUNSETLINK to make the device type disagree with the tun/tap
framing, and check that the outer headers of UDP tunnel GSO frames are
still validated according to the framing:

- tun_vnet_udptnl_proto.outer_proto_link_type: tap device set to
  ARPHRD_NONE. Frames with an NSH outer ethertype, with or without a
  VLAN tag, are rejected. A valid outer IPv4 frame is accepted.
- tun_vnet_udptnl_proto_l3.outer_proto_link_type: IFF_TUN device set
  to ARPHRD_ETHER. An Ethernet frame behind the tun_pi header is
  rejected.

Without the patch "net: virtio: do not use dev->type to find the outer
L3 header", the frames expected to be rejected are accepted.

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

diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
index 81410afbbcc8..cfeca53863ce 100644
--- a/tools/testing/selftests/net/tun.c
+++ b/tools/testing/selftests/net/tun.c
@@ -816,6 +816,59 @@ TEST_F(tun_vnet_udptnl_proto, outer_proto_bad)
 	ASSERT_EQ(errno, EINVAL);
 }
 
+/* Change the link type of a tun device, which has to be down for it. */
+static int tun_set_link_type(int fd, const char *dev, int type)
+{
+	struct ifreq ifr = { 0 };
+	int sk, ret = -1;
+
+	sk = socket(PF_INET, SOCK_DGRAM, 0);
+	if (sk < 0)
+		return -1;
+
+	strncpy(ifr.ifr_name, dev, IFNAMSIZ - 1);
+	if (ioctl(sk, SIOCGIFFLAGS, &ifr) < 0)
+		goto out;
+	ifr.ifr_flags &= ~IFF_UP;
+	if (ioctl(sk, SIOCSIFFLAGS, &ifr) < 0)
+		goto out;
+	if (ioctl(fd, TUNSETLINK, type) < 0)
+		goto out;
+	ifr.ifr_flags |= IFF_UP;
+	if (ioctl(sk, SIOCSIFFLAGS, &ifr) < 0)
+		goto out;
+	ret = 0;
+out:
+	close(sk);
+	return ret;
+}
+
+TEST_F(tun_vnet_udptnl_proto, outer_proto_link_type)
+{
+	uint8_t *pkt = self->pkt;
+	int len, ret;
+
+	/* The frames still start with an Ethernet header, but the device
+	 * type no longer says so: the Ethernet header must still be
+	 * checked.
+	 */
+	ASSERT_EQ(tun_set_link_type(self->fd, self->ifname, ARPHRD_NONE), 0);
+
+	len = build_udptnl_frame(pkt, false, ETH_P_NSH, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	len = build_udptnl_frame(pkt, true, ETH_P_NSH, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	len = build_udptnl_frame(pkt, false, ETH_P_IP, 2800);
+	ret = write(self->fd, pkt, len);
+	ASSERT_EQ(ret, len);
+}
+
 /* IFF_TUN device with a tun_pi header: skb->protocol comes from
  * userspace, and there is no Ethernet header.
  */
@@ -952,6 +1005,27 @@ TEST_F(tun_vnet_udptnl_proto_l3, outer_proto_bad)
 	ASSERT_EQ(errno, EINVAL);
 }
 
+TEST_F(tun_vnet_udptnl_proto_l3, outer_proto_link_type)
+{
+	struct tun_pi *pi = (struct tun_pi *)self->pkt;
+	int len, ret;
+
+	/* The device type now claims an Ethernet header, but IFF_TUN
+	 * frames still start with the L3 header: an Ethernet frame
+	 * behind tun_pi must not pass for a valid tunnel packet.
+	 */
+	ASSERT_EQ(tun_set_link_type(self->fd, self->ifname, ARPHRD_ETHER), 0);
+
+	len = build_udptnl_frame(self->pkt + sizeof(*pi), false, ETH_P_IP,
+				 2800);
+	pi->flags = 0;
+	pi->proto = htons(ETH_P_IP);
+	len += sizeof(*pi);
+	ret = write(self->fd, self->pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
 FIXTURE(tun_vnet_udptnl)
 {
 	char ifname[IFNAMSIZ];
-- 
2.53.0

Re: [PATCH net v3 6/8] selftests: net: tun: test UDP tunnel GSO on retyped devices

[Willem de Bruijn]

Eric Dumazet wrote:
> Use TUNSETLINK to make the device type disagree with the tun/tap
> framing, and check that the outer headers of UDP tunnel GSO frames are
> still validated according to the framing:
> 
> - tun_vnet_udptnl_proto.outer_proto_link_type: tap device set to
>   ARPHRD_NONE. Frames with an NSH outer ethertype, with or without a
>   VLAN tag, are rejected. A valid outer IPv4 frame is accepted.
> - tun_vnet_udptnl_proto_l3.outer_proto_link_type: IFF_TUN device set
>   to ARPHRD_ETHER. An Ethernet frame behind the tun_pi header is
>   rejected.
> 
> Without the patch "net: virtio: do not use dev->type to find the outer
> L3 header", the frames expected to be rejected are accepted.
> 
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

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

This TUNSETLINK ioctl is so odd. Changing dev->type but not really
changing the underlying link layer expectations, such as eth_type_trans.

Re: [PATCH net v3 6/8] selftests: net: tun: test UDP tunnel GSO on retyped devices

[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] In TEST_F(tun_vnet_udptnl_proto, outer_proto_link_type) in
  tools/testing/selftests/net/tun.c, the second write sends a VLAN-…

--

selftests: net: tun: test UDP tunnel GSO on retyped devices

This commit adds a tun_set_link_type() helper that uses TUNSETLINK to
retype tun/tap devices. It also adds two tests that check the outer
headers of UDP tunnel GSO frames are validated by the tun/tap framing
rather than by dev->type.

> diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> index 81410afbbcc8d..cfeca53863ce2 100644
> --- a/tools/testing/selftests/net/tun.c
> +++ b/tools/testing/selftests/net/tun.c
> @@ -816,6 +816,59 @@ TEST_F(tun_vnet_udptnl_proto, outer_proto_bad)

[ ... ]

> +TEST_F(tun_vnet_udptnl_proto, outer_proto_link_type)
> +{
> +	uint8_t *pkt = self->pkt;
> +	int len, ret;
> +
> +	/* The frames still start with an Ethernet header, but the device
> +	 * type no longer says so: the Ethernet header must still be
> +	 * checked.
> +	 */
> +	ASSERT_EQ(tun_set_link_type(self->fd, self->ifname, ARPHRD_NONE), 0);
> +
> +	len = build_udptnl_frame(pkt, false, ETH_P_NSH, 2800);
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);
> +
> +	len = build_udptnl_frame(pkt, true, ETH_P_NSH, 2800);
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);

[Severity: Low]

Does this VLAN-tagged NSH write fail on a kernel that is missing the fix?
The commit message says:

  Without the patch "net: virtio: do not use dev->type to find the outer
  L3 header", the frames expected to be rejected are accepted.

Without that fix, dev->type is ARPHRD_NONE here, so virtio_net_l3_start()
takes the non-Ethernet branch. For IFF_TAP, tun_get_user() has already
done:

    skb_set_network_header(skb, ETH_HLEN);

so the IP version is read at frame offset 14. In a tagged frame, that
offset holds the 802.1Q TCI that build_udptnl_frame() writes:

    uint16_t vlan_tag[2] = { htons(100), htons(l3proto) };

The first byte is 0x00, so the old code sees version 0:

    if (iph->version == 4)
        *proto = htons(ETH_P_IP);
    else if (iph->version == 6)
        *proto = htons(ETH_P_IPV6);
    else
        return false;

and the write returns -EINVAL. The old code rejects every VLAN-tagged
frame on a retyped tap, NSH or not. With the fix, this frame is still
rejected, because the protocol after the tag is NSH. So this assertion
passes with or without the fix.

The untagged ETH_P_IP accept case below also passes on both kernels,
because the old path finds a valid IPv4 header at offset 14. Only the
untagged NSH write tells the two kernels apart.

Could a VLAN-tagged ETH_P_IP frame that must be accepted be added here?
The unfixed kernel rejects it (version 0 at offset 14), and the fixed
kernel accepts it after walking the tag. The case is still missing at the
end of the series, after "selftests: net: tuntap: write UDP tunnel offsets
in little endian".

Should the commit message also be narrowed, since the tagged NSH frame is
rejected without the fix as well?

> +
> +	len = build_udptnl_frame(pkt, false, ETH_P_IP, 2800);
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, len);
> +}

[ ... ]

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

[PATCH net v3 7/8] selftests: net: tun: test more outer header mismatches of UDP tunnel GSO

[Eric Dumazet]

Add tap tests for the outer header checks of UDP tunnel GSO packets
not covered yet:

- outer_proto_ok_ipv6: a 4in6 frame with an outer IPv6 header is
  accepted.
- outer_proto_bad_family: an IPv4 ethertype on a declared IPv6 tunnel
  is rejected.
- outer_proto_bad_vlan_ipv6: an IPv6 ethertype behind a VLAN tag on a
  declared IPv4 tunnel is rejected.
- outer_proto_bad_vlan_offset: a VLAN tagged frame whose
  outer_th_offset ignores the tag is rejected.
- outer_proto_bad_l4: an outer IPv4 header carrying GRE instead of UDP
  is rejected.

Without the first patch of this series, the four outer_proto_bad
tests fail because the kernel accepts the frames.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/179386753718.488856.13433005125725728519@kernel.org/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 tools/testing/selftests/net/tun.c | 97 +++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
index cfeca53863ce..a135d3aa10d1 100644
--- a/tools/testing/selftests/net/tun.c
+++ b/tools/testing/selftests/net/tun.c
@@ -869,6 +869,103 @@ TEST_F(tun_vnet_udptnl_proto, outer_proto_link_type)
 	ASSERT_EQ(ret, len);
 }
 
+/* Build a 4in6 Geneve GSO frame whose outer ethertype is @l3proto. */
+static int build_udptnl6_frame(uint8_t *buf, uint16_t l3proto,
+			       int payload_len)
+{
+	int hlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN;
+	uint8_t *outer_udph, *cur = buf;
+	int pktlen;
+
+	memset(buf, 0, TUN_VNET_TNL_SIZE + hlen + payload_len);
+
+	cur += build_virtio_net_hdr_v1_hash_tunnel(cur, true, hlen, 1400,
+						   AF_INET6, AF_INET);
+	cur += build_eth(cur, l3proto, param_hwaddr_outer_dst,
+			 param_hwaddr_outer_src);
+
+	pktlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN - ETH_HLEN -
+		 sizeof(struct ipv6hdr) + payload_len;
+	cur += build_ipv6_header(cur, IPPROTO_UDP, 0, pktlen,
+				 &param_ipaddr6_outer_dst,
+				 &param_ipaddr6_outer_src);
+
+	outer_udph = cur;
+	cur += build_udp_header(cur, UDP_SRC_PORT, VN_PORT,
+				pktlen - sizeof(struct udphdr));
+	cur += build_geneve_header(cur, VN_ID);
+	cur += build_eth(cur, ETH_P_IP, param_hwaddr_inner_dst,
+			 param_hwaddr_inner_src);
+	cur += build_ipv4_header(cur, IPPROTO_UDP,
+				 sizeof(struct udphdr) + payload_len,
+				 &param_ipaddr4_inner_dst,
+				 &param_ipaddr4_inner_src);
+	cur += build_udp_packet(cur, UDP_DST_PORT, UDP_SRC_PORT, payload_len,
+				AF_INET, false);
+	build_udp_packet_csum(outer_udph, AF_INET6, false);
+
+	return cur - buf;
+}
+
+TEST_F(tun_vnet_udptnl_proto, outer_proto_ok_ipv6)
+{
+	int len, ret;
+
+	len = build_udptnl6_frame(self->pkt, ETH_P_IPV6, 2800);
+	ret = write(self->fd, self->pkt, len);
+	ASSERT_EQ(ret, len);
+}
+
+/* An IPv4 ethertype on a declared IPv6 tunnel. */
+TEST_F(tun_vnet_udptnl_proto, outer_proto_bad_family)
+{
+	int len, ret;
+
+	len = build_udptnl6_frame(self->pkt, ETH_P_IP, 2800);
+	ret = write(self->fd, self->pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
+TEST_F(tun_vnet_udptnl_proto, outer_proto_bad_vlan_ipv6)
+{
+	int len, ret;
+
+	len = build_udptnl_frame(self->pkt, true, ETH_P_IPV6, 2800);
+	ret = write(self->fd, self->pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
+/* outer_th_offset does not account for the VLAN tag. */
+TEST_F(tun_vnet_udptnl_proto, outer_proto_bad_vlan_offset)
+{
+	struct virtio_net_hdr_v1_hash_tunnel *vh = (void *)self->pkt;
+	int len, ret;
+
+	len = build_udptnl_frame(self->pkt, true, ETH_P_IP, 2800);
+	vh->outer_th_offset -= 4;
+	ret = write(self->fd, self->pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
+/* The outer IPv4 header does not carry UDP. */
+TEST_F(tun_vnet_udptnl_proto, outer_proto_bad_l4)
+{
+	struct iphdr *iph;
+	int len, ret;
+
+	len = build_udptnl_frame(self->pkt, false, ETH_P_IP, 2800);
+	iph = (struct iphdr *)(self->pkt + TUN_VNET_TNL_SIZE + ETH_HLEN);
+	iph->protocol = IPPROTO_GRE;
+	iph->check = 0;
+	iph->check = build_ip_csum((uint8_t *)iph, iph->ihl << 2, 0);
+	ret = write(self->fd, self->pkt, len);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
 /* IFF_TUN device with a tun_pi header: skb->protocol comes from
  * userspace, and there is no Ethernet header.
  */
-- 
2.53.0

Re: [PATCH net v3 7/8] selftests: net: tun: test more outer header mismatches of UDP tunnel GSO

[netdev-bot+sashiko]

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

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

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] The new helper build_udptnl6_frame() in
  tools/testing/selftests/net/tun.c (lines 873-908) gets the header…

--

selftests: net: tun: test more outer header mismatches of UDP tunnel GSO

This commit adds tap tests for outer header checks on UDP tunnel GSO
packets. It adds a new 4in6 frame builder, build_udptnl6_frame(), with a
positive control and a family mismatch test. It also adds VLAN and L4
protocol mismatch tests that reuse the existing 4in4 builder.

Closes: https://lore.kernel.org/netdev/179386753718.488856.13433005125725728519@kernel.org/

> diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> index cfeca53863ce2..a135d3aa10d18 100644
> --- a/tools/testing/selftests/net/tun.c
> +++ b/tools/testing/selftests/net/tun.c
> @@ -869,6 +869,103 @@ TEST_F(tun_vnet_udptnl_proto, outer_proto_link_type)
>  	ASSERT_EQ(ret, len);
>  }
>  
> +/* Build a 4in6 Geneve GSO frame whose outer ethertype is @l3proto. */
> +static int build_udptnl6_frame(uint8_t *buf, uint16_t l3proto,
> +			       int payload_len)
> +{
> +	int hlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN;
> +	uint8_t *outer_udph, *cur = buf;
> +	int pktlen;
> +
> +	memset(buf, 0, TUN_VNET_TNL_SIZE + hlen + payload_len);
> +
> +	cur += build_virtio_net_hdr_v1_hash_tunnel(cur, true, hlen, 1400,
> +						   AF_INET6, AF_INET);
> +	cur += build_eth(cur, l3proto, param_hwaddr_outer_dst,
> +			 param_hwaddr_outer_src);
> +
> +	pktlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN - ETH_HLEN -
> +		 sizeof(struct ipv6hdr) + payload_len;

[Severity: Medium]
Are hlen and pktlen both 14 bytes too small here?

UDP_TUNNEL_GENEVE_4IN6_HDRLEN counts only the inner Ethernet header:

#define UDP_TUNNEL_GENEVE_4IN6_HDRLEN \
	(ETH_HLEN + sizeof(struct iphdr) + sizeof(struct ipv6hdr) + \
	 GENEVE_HLEN + 2 * sizeof(struct udphdr))

The 4in4 sibling build_udptnl_frame() adds the outer Ethernet header
separately. It subtracts only the outer L3 header:

	int hlen = ETH_HLEN + vlan_len + UDP_TUNNEL_GENEVE_4IN4_HDRLEN;
	...
	pktlen = UDP_TUNNEL_GENEVE_4IN4_HDRLEN - sizeof(struct iphdr) +
		 payload_len;

build_gso_packet_into_tun() follows the same convention for 4in6 on tap.

In build_udptnl6_frame(), the virtio header's hdr_len is set to 98. The
real header stack is 112 bytes. The memset() also clears 14 bytes less
than the frame that gets written.

pktlen works out to 44 + payload_len. The data after the outer IPv6 header
is outer UDP + Geneve + inner Ethernet + inner IPv4 + inner UDP + payload,
which is 58 + payload_len.

So ip6h->payload_len and the outer udph->len are both 14 bytes short.
build_udp_packet_csum() sums only ntohs(udph->len) bytes, so the outer UDP
checksum is also wrong.

outer_proto_ok_ipv6 passes today only because the tun write path does not
cross-check those lengths. Would it start failing if outer header
validation became stricter? And could outer_proto_bad_family then get
EINVAL from the length mismatch instead of the family check it is meant
to cover?

The later patch in the series, "selftests: net: tuntap: write UDP tunnel
offsets in little endian", leaves build_udptnl6_frame() unchanged.

Something like this would match build_udptnl_frame():

	int hlen = ETH_HLEN + UDP_TUNNEL_GENEVE_4IN6_HDRLEN;
	...
	pktlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN - sizeof(struct ipv6hdr) +
		 payload_len;

> +	cur += build_ipv6_header(cur, IPPROTO_UDP, 0, pktlen,
> +				 &param_ipaddr6_outer_dst,
> +				 &param_ipaddr6_outer_src);
> +
> +	outer_udph = cur;
> +	cur += build_udp_header(cur, UDP_SRC_PORT, VN_PORT,
> +				pktlen - sizeof(struct udphdr));

[ ... ]

> +	build_udp_packet_csum(outer_udph, AF_INET6, false);
> +
> +	return cur - buf;
> +}
> +
> +TEST_F(tun_vnet_udptnl_proto, outer_proto_ok_ipv6)
> +{
> +	int len, ret;
> +
> +	len = build_udptnl6_frame(self->pkt, ETH_P_IPV6, 2800);
> +	ret = write(self->fd, self->pkt, len);
> +	ASSERT_EQ(ret, len);
> +}
> +
> +/* An IPv4 ethertype on a declared IPv6 tunnel. */
> +TEST_F(tun_vnet_udptnl_proto, outer_proto_bad_family)
> +{
> +	int len, ret;
> +
> +	len = build_udptnl6_frame(self->pkt, ETH_P_IP, 2800);
> +	ret = write(self->fd, self->pkt, len);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);
> +}

[ ... ]

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

[PATCH net v3 8/8] selftests: net: tuntap: write UDP tunnel offsets in little endian

[Eric Dumazet]

outer_th_offset and inner_nh_offset of struct
virtio_net_hdr_v1_hash_tunnel are __le16, and the kernel always reads
them with le16_to_cpu(). build_virtio_net_hdr_v1_hash_tunnel() writes
them in host order, and tun.c adjusts them with native arithmetic, so
all UDP tunnel GSO tests fail on big-endian hosts: the kernel sees
offsets such as 0x2200 and rejects the frames before the checks the
tests target.

Write the offsets with htole16(), and add vnet_tnl_offset_add() to
adjust them. csum_start stays in host order: it follows the tun vnet
header endianness, which the tests do not change.

Fixes: a942fcd72e97 ("selftest: tun: Introduce tuntap_helpers.h header for TUN/TAP testing")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/179387332145.488856.1149073246990672028@kernel.org/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 tools/testing/selftests/net/tun.c            | 10 +++++-----
 tools/testing/selftests/net/tuntap_helpers.h | 19 +++++++++++++++----
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
index a135d3aa10d1..5f797a1bc604 100644
--- a/tools/testing/selftests/net/tun.c
+++ b/tools/testing/selftests/net/tun.c
@@ -738,8 +738,8 @@ static int build_udptnl_frame(uint8_t *buf, bool vlan, uint16_t l3proto,
 
 	cur += build_virtio_net_hdr_v1_hash_tunnel(cur, true, hlen, 1400,
 						   AF_INET, AF_INET);
-	vh->outer_th_offset += vlan_len;
-	vh->inner_nh_offset += vlan_len;
+	vnet_tnl_offset_add(&vh->outer_th_offset, vlan_len);
+	vnet_tnl_offset_add(&vh->inner_nh_offset, vlan_len);
 	vh->hash_hdr.hdr.csum_start += vlan_len;
 
 	if (vlan) {
@@ -944,7 +944,7 @@ TEST_F(tun_vnet_udptnl_proto, outer_proto_bad_vlan_offset)
 	int len, ret;
 
 	len = build_udptnl_frame(self->pkt, true, ETH_P_IP, 2800);
-	vh->outer_th_offset -= 4;
+	vnet_tnl_offset_add(&vh->outer_th_offset, -4);
 	ret = write(self->fd, self->pkt, len);
 	ASSERT_EQ(ret, -1);
 	ASSERT_EQ(errno, EINVAL);
@@ -1031,8 +1031,8 @@ static int build_udptnl_l3_frame(uint8_t *buf, uint16_t pi_proto,
 	cur = vnet + build_virtio_net_hdr_v1_hash_tunnel(vnet, false, hlen,
 							 1400, AF_INET,
 							 AF_INET);
-	vh->outer_th_offset += IPV4_OPT_LEN;
-	vh->inner_nh_offset += IPV4_OPT_LEN;
+	vnet_tnl_offset_add(&vh->outer_th_offset, IPV4_OPT_LEN);
+	vnet_tnl_offset_add(&vh->inner_nh_offset, IPV4_OPT_LEN);
 	vh->hash_hdr.hdr.csum_start += IPV4_OPT_LEN;
 
 	/* Build the frame without options first: build_udp_packet_csum()
diff --git a/tools/testing/selftests/net/tuntap_helpers.h b/tools/testing/selftests/net/tuntap_helpers.h
index ecad83fae158..dc85cd9bc0ae 100644
--- a/tools/testing/selftests/net/tuntap_helpers.h
+++ b/tools/testing/selftests/net/tuntap_helpers.h
@@ -3,6 +3,7 @@
 #ifndef _TUNTAP_HELPERS_H
 #define _TUNTAP_HELPERS_H
 
+#include <endian.h>
 #include <errno.h>
 #include <linux/if_packet.h>
 #include <linux/ipv6.h>
@@ -368,6 +369,7 @@ static inline int build_virtio_net_hdr_v1_hash_tunnel(uint8_t *buf, bool is_tap,
 	struct virtio_net_hdr_v1_hash_tunnel *vh_tunnel = (void *)buf;
 	struct virtio_net_hdr_v1 *vh = &vh_tunnel->hash_hdr.hdr;
 	int outer_iphlen, inner_iphlen, eth_hlen, gso_type;
+	int outer_th, inner_nh;
 
 	eth_hlen = is_tap ? ETH_HLEN : 0;
 	outer_iphlen = (outer_family == AF_INET) ? sizeof(struct iphdr) :
@@ -375,11 +377,14 @@ static inline int build_virtio_net_hdr_v1_hash_tunnel(uint8_t *buf, bool is_tap,
 	inner_iphlen = (inner_family == AF_INET) ? sizeof(struct iphdr) :
 						   sizeof(struct ipv6hdr);
 
-	vh_tunnel->outer_th_offset = eth_hlen + outer_iphlen;
-	vh_tunnel->inner_nh_offset = vh_tunnel->outer_th_offset + ETH_HLEN +
-				     GENEVE_HLEN + sizeof(struct udphdr);
+	outer_th = eth_hlen + outer_iphlen;
+	inner_nh = outer_th + ETH_HLEN + GENEVE_HLEN + sizeof(struct udphdr);
 
-	vh->csum_start = vh_tunnel->inner_nh_offset + inner_iphlen;
+	/* The tunnel offsets are always little endian. */
+	vh_tunnel->outer_th_offset = htole16(outer_th);
+	vh_tunnel->inner_nh_offset = htole16(inner_nh);
+
+	vh->csum_start = inner_nh + inner_iphlen;
 	vh->csum_offset = __builtin_offsetof(struct udphdr, check);
 	vh->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
 	vh->hdr_len = hdr_len;
@@ -395,4 +400,10 @@ static inline int build_virtio_net_hdr_v1_hash_tunnel(uint8_t *buf, bool is_tap,
 	return sizeof(struct virtio_net_hdr_v1_hash_tunnel);
 }
 
+/* Add @delta to a little endian tunnel offset of the virtio header. */
+static inline void vnet_tnl_offset_add(__le16 *off, int delta)
+{
+	*off = htole16(le16toh(*off) + delta);
+}
+
 #endif /* _TUNTAP_HELPERS_H */
-- 
2.53.0

Re: [PATCH net v3 8/8] selftests: net: tuntap: write UDP tunnel offsets in little endian

[netdev-bot+sashiko]

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

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

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] selftests: net: tun: UDP tunnel GSO tests still fail on big-
  endian hosts

--

> diff --git a/tools/testing/selftests/net/tuntap_helpers.h b/tools/testing/selftests/net/tuntap_helpers.h
> --- a/tools/testing/selftests/net/tuntap_helpers.h
> +++ b/tools/testing/selftests/net/tuntap_helpers.h
> @@ -375,11 +377,14 @@ static inline int build_virtio_net_hdr_v1_hash_tunnel(uint8_t *buf, bool is_tap,
>  	inner_iphlen = (inner_family == AF_INET) ? sizeof(struct iphdr) :
>  						   sizeof(struct ipv6hdr);
>  
> -	vh_tunnel->outer_th_offset = eth_hlen + outer_iphlen;
> -	vh_tunnel->inner_nh_offset = vh_tunnel->outer_th_offset + ETH_HLEN +
> -				     GENEVE_HLEN + sizeof(struct udphdr);
> +	outer_th = eth_hlen + outer_iphlen;
> +	inner_nh = outer_th + ETH_HLEN + GENEVE_HLEN + sizeof(struct udphdr);
>  
> -	vh->csum_start = vh_tunnel->inner_nh_offset + inner_iphlen;
> +	/* The tunnel offsets are always little endian. */
> +	vh_tunnel->outer_th_offset = htole16(outer_th);
> +	vh_tunnel->inner_nh_offset = htole16(inner_nh);
> +
> +	vh->csum_start = inner_nh + inner_iphlen;
>  	vh->csum_offset = __builtin_offsetof(struct udphdr, check);
>  	vh->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
>  	vh->hdr_len = hdr_len;

[Severity: Medium]
Does this fully fix the big-endian case? The commit message says
csum_start follows the tun vnet header endianness, but that holds for
only part of the kernel path.

tun_open() never issues TUNSETVNETLE, so on a big-endian host
tun_vnet_is_little_endian() is false. virtio_net_hdr_tnl_to_skb() does
read csum_start with __virtio16_to_cpu(little_endian, ...) for the
initial offset ordering check. It then calls
__virtio_net_hdr_to_skb(skb, hdr, true, ...) with little_endian
hardcoded to true.

Inside __virtio_net_hdr_to_skb(), csum_start, csum_offset and
gso_size get byte swapped. build_virtio_net_hdr_v1_hash_tunnel()
still writes all three in host order, so:

  csum_start 84 becomes 21504
  csum_offset 6 becomes 1536
  gso_size 1400 becomes 30725

The 2800 byte frames used by tun_vnet_udptnl_proto and
tun_vnet_udptnl_proto_l3 are much smaller than that. The
pskb_may_pull() for the checksum location therefore fails, and
write() returns -EINVAL before virtio_net_tnl_offsets_ok() is ever
reached. The outer_proto_bad* cases would still see EINVAL, but for
the wrong reason, and the positive cases would still fail.

The receive side has the same problem.
virtio_net_hdr_tnl_from_skb() calls
virtio_net_hdr_from_skb(skb, hdr, true, false, vlan_hlen), so
gso_size comes back little endian. recv_gso_packet() then compares
vh->gso_size directly against the host order variant->gso_size.

So the UDP tunnel tests in tun.c would still fail on big-endian
hosts after this patch. Which way do you want to fix it?

One option is for the tests to switch the device to little endian
with TUNSETVNETLE and write the base header fields with htole16() too.

The other is to treat the hardcoded true in
virtio_net_hdr_tnl_to_skb() and virtio_net_hdr_tnl_from_skb() as the
real bug. The same function already decodes csum_start with
little_endian a few lines earlier.

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