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

[PATCH net v2 0/3] 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 fixes a selftest helper computing UDP checksums over a length
in network byte order.

Patch 3 adds tun selftests, on tap and IFF_TUN devices: valid outer
IPv4 frames (plain and VLAN tagged) are accepted, mismatched or non-IP
outer headers are rejected.

v2:
- 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 (3):
  net: virtio: validate outer protocol of UDP tunnel GSO packets
  selftests: net: tuntap: fix UDP checksum length in
    build_udp_packet_csum()
  selftests: net: tun: test outer protocol of UDP tunnel GSO packets

 include/linux/virtio_net.h                   | 169 ++++++++++++
 tools/testing/selftests/net/tun.c            | 269 +++++++++++++++++++
 tools/testing/selftests/net/tuntap_helpers.h |  10 +-
 3 files changed, 447 insertions(+), 1 deletion(-)


base-commit: 787268686c64be60023743b527645a907e38ccca
-- 
2.53.0

[PATCH net v2 1/3] 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>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 include/linux/virtio_net.h | 169 +++++++++++++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index d6466f96cdd0..b9c8babb1fd7 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,146 @@ 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, AH included.
+			 */
+			*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 +425,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 +504,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 v2 1/3] net: virtio: validate outer protocol of UDP tunnel GSO packets

[Willem de Bruijn]

Eric Dumazet wrote:
> 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>
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

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

Thanks Eric!


My bot points out a preexisting issue with this udp tunnel feature.
_Not_ reason for changing this patch.

virtio_net_hdr_tnl_to_skb already does not support IFF_TUN, as it has

    outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6);

So the only reachable path with IFF_TUN is L3 with IPV4_OPT_LEN 16
(or greater) as indeed test patch 3/3 introduces.

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

[Michael S. Tsirkin]

On Wed, Nov 04, 2026 at 06:01:29AM +0100, Eric Dumazet wrote:
> 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>
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

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

> ---
>  include/linux/virtio_net.h | 169 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 169 insertions(+)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index d6466f96cdd0..b9c8babb1fd7 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,146 @@ 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, AH included.
> +			 */
> +			*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 +425,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 +504,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 v2 1/3] net: virtio: validate outer protocol of UDP tunnel GSO packets

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: virtio: L2 check bypass in virtio_net_l3_start() via TUNSETLINK
--

commit 546d87e0158fb3d32de1c338228f7d9a8f02bb4e
Author: Eric Dumazet <edumazet@kernel.org>

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

This commit validates the outer L2/L3/L4 headers of UDP tunnel GSO packets in
virtio_net_hdr_tnl_to_skb() so that frames carrying mismatched outer protocols
or offsets are rejected before GSO segmentation.

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..b9c8babb1fd73 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
[ ... ]
> +/* 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;
> +}

[Severity: High]
Can a tap writer bypass the Ethernet header check in virtio_net_l3_start() by
changing tun->dev->type via the TUNSETLINK ioctl?

In drivers/net/tun.c, __tun_chr_ioctl() allows changing tun->dev->type while
the device is down without changing tun->flags:

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

Meanwhile, tun_get_user() (and tun_xdp_one()) decides whether the frame starts
with an Ethernet header based on (tun->flags & TUN_TYPE_MASK) == IFF_TAP,
setting the network header to ETH_HLEN before calling
tun_vnet_hdr_tnl_to_skb(), and only populating skb->protocol via
eth_type_trans() afterwards:

drivers/net/tun.c:tun_get_user() {
	switch (tun->flags & TUN_TYPE_MASK) {
	...
	case IFF_TAP:
		...
		skb_set_network_header(skb, ETH_HLEN);
		break;
	}

	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
		...
	}

	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
		skb->protocol = eth_type_trans(skb, tun->dev);
}

If an IFF_TAP device has tun->dev->type set to another type such as
ARPHRD_NONE via TUNSETLINK, virtio_net_l3_start() takes the else branch, reads
*nhoff = skb_network_offset(skb) (which is ETH_HLEN), derives *proto from the
IP version nibble at offset 14, and skips the skb->protocol check because
skb->protocol is still 0.

The Ethernet header at offset 0 is never inspected by virtio_net_l3_start(),
so eth_type_trans() will subsequently set skb->protocol from the unvalidated
eth_hdr(skb)->h_proto (for example ETH_P_NSH) on a packet with
SKB_GSO_UDP_TUNNEL set, triggering the same skb_under_panic() during GSO
segmentation.

Conversely, on an IFF_TUN device whose tun->dev->type is changed to
ARPHRD_ETHER via TUNSETLINK, virtio_net_l3_start() validates a 14-byte
Ethernet header at offset 0 and an outer IP header at offset 14, but
tun_get_user() does not call eth_type_trans() and resets skb->network_header
to offset 0, leaving the outer network header pointing at the fake Ethernet
header.

Should tun_get_user() and tun_xdp_one() set skb->protocol before calling
tun_vnet_hdr_tnl_to_skb(), or should virtio_net_l3_start() avoid relying on
skb->dev->type matching the caller's L2 framing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104050131.629967-1-edumazet@kernel.org?part=1

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · Low: 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] The new outer-header validator can still be bypassed, so the
  NSH-over-UDP-tunnel GSO crash this commit claims to fix is still…
- [Low] In virtio_net_l4_start() (include/linux/virtio_net.h), the
  comment before the ipv6_find_hdr() call says "Find the transport…

--

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

This commit makes virtio_net_hdr_tnl_to_skb() validate the outer headers
of UDP tunnel GSO packets. The frame must carry an outer IPv4 or IPv6
header that matches the declared tunnel type, with UDP at
outer_th_offset. The L3 start is taken from the device type: either an
Ethernet/VLAN header, or the network header the caller set.

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..b9c8babb1fd73 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h

[ ... ]

> @@ -48,6 +51,146 @@ 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) {
> +		__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;
> +}

[Severity: High]

Can a tap owner push this function into the wrong branch, so the NSH
crash from the commit message is still reachable?

The branch is chosen from skb->dev->type. In tun, the real framing and the
later eth_type_trans() call both follow tun->flags. The tap owner can set
dev->type to any value with TUNSETLINK while the device is down:

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

Take an IFF_TAP device with dev->type set to, for example, ARPHRD_NONE.
tun_get_user() does:

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

In that case virtio_net_l3_start() takes the else branch with
nhoff = ETH_HLEN and only reads the IP version nibble at byte 14.

The skb->protocol cross-check is skipped because skb->protocol is still 0
at this point. h_proto in bytes 12-13 is never read.

With a valid IPv4 header at offset 14 and UDP at offset 34,
virtio_net_tnl_offsets_ok() returns true. eth_type_trans() then sets
skb->protocol from the unchecked h_proto, for example ETH_P_NSH, while
SKB_GSO_UDP_TUNNEL is set. tun_xdp_one() has the same order: validation
first, then an unconditional eth_type_trans().

If that skb is redirected with tc mirred to a gretap, does it reach the
same crash shown in the commit message?

validate_xmit_skb()
  skb_mac_gso_segment()
    nsh_gso_segment()
...
gre_tap_xmit()
  __gre_xmit()
    skb_push()
      skb_under_panic()

Compared with the original reproducer, the only extra step seems to be
one TUNSETLINK ioctl before bringing the tap up.

There is a second variant: an IFF_TUN device with a packet-info header
and dev->type set to ARPHRD_ETHER.

With pi.proto = 0x8100 and bytes 12-13 = 0x8100, the cross-check passes.
The validator then walks a VLAN tag at 14 to IPv4 at 18 and UDP at 38.

The stack handles the packet as raw L3, though. skb_vlan_untag() reads
the VLAN header at data[0], so attacker-chosen bytes 2-3 (for example
NSH) become skb->protocol.

The skb->protocol cross-check cannot catch either case. In the first it
is 0, and in the second the sender controls both values.

vhost-net guests cannot change dev->type, so the guest path looks closed.
The tap writer path named in the commit message still seems open.

A smaller point: dev->type is read here without READ_ONCE() or a lock,
while TUNSETLINK writes it under rtnl. The bypass above does not need
that race, though, since a stable mismatched type is enough.

Could the caller tell the validator whether the frame starts with an
Ethernet header instead? tun knows this from tun->flags, and virtio_net
always has one. Another option would be to always cross-check h_proto
whenever eth_type_trans() will be applied afterwards.

[ ... ]

> +	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, AH included.
> +			 */
> +			*ip_proto = ipv6_find_hdr(skb, thoff, -1, NULL,
> +						  &flags);

[Severity: Low]

This isn't a bug, but the comment says the walk goes past AH. With
flags = IP6_FH_F_AUTH and target = -1, ipv6_find_hdr() actually stops at
the AH header:

net/ipv6/exthdrs_core.c:ipv6_find_hdr() {
    ...
		} else if (nexthdr == NEXTHDR_AUTH) {
			if (flags && (*flags & IP6_FH_F_AUTH) && (target < 0))
				break;
    ...
}

It returns NEXTHDR_AUTH, so an outer IPv6+AH+UDP frame is rejected. That
matches the commit message ("ipv6_find_hdr() stops at an AH header").

Could the comment say that the walk stops at AH? As written, a reader
might take it as a reason to drop IP6_FH_F_AUTH later.

> +			if (*ip_proto < 0 || flags & IP6_FH_F_FRAG)
> +				return false;
> +		}
> +		return true;
> +	}

[ ... ]

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

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

[Eric Dumazet]

Le jeu. 5 nov. 2026 à 11:08, <netdev-bot+sashiko@kernel.org> a écrit :
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 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] The new outer-header validator can still be bypassed, so the
>   NSH-over-UDP-tunnel GSO crash this commit claims to fix is still…
> - [Low] In virtio_net_l4_start() (include/linux/virtio_net.h), the
>   comment before the ipv6_find_hdr() call says "Find the transport…
>
> --
>
> net: virtio: validate outer protocol of UDP tunnel GSO packets
>
> This commit makes virtio_net_hdr_tnl_to_skb() validate the outer headers
> of UDP tunnel GSO packets. The frame must carry an outer IPv4 or IPv6
> header that matches the declared tunnel type, with UDP at
> outer_th_offset. The L3 start is taken from the device type: either an
> Ethernet/VLAN header, or the network header the caller set.
>
> 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..b9c8babb1fd73 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
>
> [ ... ]
>
> > @@ -48,6 +51,146 @@ 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) {
> > +             __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;
> > +}
>
> [Severity: High]
>
> Can a tap owner push this function into the wrong branch, so the NSH
> crash from the commit message is still reachable?
>
> The branch is chosen from skb->dev->type. In tun, the real framing and the
> later eth_type_trans() call both follow tun->flags. The tap owner can set
> dev->type to any value with TUNSETLINK while the device is down:
>
> drivers/net/tun.c:__tun_chr_ioctl() {
>     ...
>                         tun->dev->type = (int) arg;
>     ...
> }
>
> Take an IFF_TAP device with dev->type set to, for example, ARPHRD_NONE.
> tun_get_user() does:
>
>                 skb_set_network_header(skb, ETH_HLEN);
>         ...
>         if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
>         ...
>         if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
>                 skb->protocol = eth_type_trans(skb, tun->dev);
>
> In that case virtio_net_l3_start() takes the else branch with
> nhoff = ETH_HLEN and only reads the IP version nibble at byte 14.
>
> The skb->protocol cross-check is skipped because skb->protocol is still 0
> at this point. h_proto in bytes 12-13 is never read.
>
> With a valid IPv4 header at offset 14 and UDP at offset 34,
> virtio_net_tnl_offsets_ok() returns true. eth_type_trans() then sets
> skb->protocol from the unchecked h_proto, for example ETH_P_NSH, while
> SKB_GSO_UDP_TUNNEL is set. tun_xdp_one() has the same order: validation
> first, then an unconditional eth_type_trans().
>
> If that skb is redirected with tc mirred to a gretap, does it reach the
> same crash shown in the commit message?
>
> validate_xmit_skb()
>   skb_mac_gso_segment()
>     nsh_gso_segment()
> ...
> gre_tap_xmit()
>   __gre_xmit()
>     skb_push()
>       skb_under_panic()
>
> Compared with the original reproducer, the only extra step seems to be
> one TUNSETLINK ioctl before bringing the tap up.
>
> There is a second variant: an IFF_TUN device with a packet-info header
> and dev->type set to ARPHRD_ETHER.
>
> With pi.proto = 0x8100 and bytes 12-13 = 0x8100, the cross-check passes.
> The validator then walks a VLAN tag at 14 to IPv4 at 18 and UDP at 38.
>
> The stack handles the packet as raw L3, though. skb_vlan_untag() reads
> the VLAN header at data[0], so attacker-chosen bytes 2-3 (for example
> NSH) become skb->protocol.
>
> The skb->protocol cross-check cannot catch either case. In the first it
> is 0, and in the second the sender controls both values.
>
> vhost-net guests cannot change dev->type, so the guest path looks closed.
> The tap writer path named in the commit message still seems open.

Right, both variants are real. They need CAP_NET_ADMIN over the
device: TUNSETLINK only works while the device is down, and it has to
be brought up again. A vhost-net guest cannot use them, but a tap
writer in a user namespace can.

v3 follows your suggestion: virtio_net_hdr_tnl_to_skb() gets a has_eth
argument from its callers, and virtio_net_l3_start() no longer looks
at the device type. tun passes whether the device is IFF_TAP (and true
from tun_xdp_one(), which always calls eth_type_trans()), virtio_net
always passes true. v3 also adds tests for both variants (tap set to
ARPHRD_NONE, IFF_TUN set to ARPHRD_ETHER).


>
> A smaller point: dev->type is read here without READ_ONCE() or a lock,
> while TUNSETLINK writes it under rtnl. The bypass above does not need
> that race, though, since a stable mismatched type is enough.
>

Since tun_get_user() only drops packets of a down device after the
checks, a check of dev->type in tun alone would have been racy too.
With has_eth, dev->type is no longer read on this path.


> Could the caller tell the validator whether the frame starts with an
> Ethernet header instead? tun knows this from tun->flags, and virtio_net
> always has one. Another option would be to always cross-check h_proto
> whenever eth_type_trans() will be applied afterwards.
>
> [ ... ]
>
> > +     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, AH included.
> > +                      */
> > +                     *ip_proto = ipv6_find_hdr(skb, thoff, -1, NULL,
> > +                                               &flags);
>
> [Severity: Low]
>
> This isn't a bug, but the comment says the walk goes past AH. With
> flags = IP6_FH_F_AUTH and target = -1, ipv6_find_hdr() actually stops at
> the AH header:

Indeed, the comment comes from Michael's helper, which walked past
AH. I will fix it in v3.

pw-bot: cr


>
> net/ipv6/exthdrs_core.c:ipv6_find_hdr() {
>     ...
>                 } else if (nexthdr == NEXTHDR_AUTH) {
>                         if (flags && (*flags & IP6_FH_F_AUTH) && (target < 0))
>                                 break;
>     ...
> }
>
> It returns NEXTHDR_AUTH, so an outer IPv6+AH+UDP frame is rejected. That
> matches the commit message ("ipv6_find_hdr() stops at an AH header").
>
> Could the comment say that the walk stops at AH? As written, a reader
> might take it as a reason to drop IP6_FH_F_AUTH later.
>
> > +                     if (*ip_proto < 0 || flags & IP6_FH_F_FRAG)
> > +                             return false;
> > +             }
> > +             return true;
> > +     }
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261104050131.629967-1-edumazet%40kernel.org

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

Re: [PATCH net v2 2/3] selftests: net: tuntap: fix UDP checksum length in build_udp_packet_csum()

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104050131.629967-1-edumazet@kernel.org?part=2

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

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

Re: [PATCH net v2 3/3] selftests: net: tun: test outer protocol of UDP tunnel GSO packets

[Willem de Bruijn]

Eric Dumazet wrote:
> 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.
> 
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

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

Re: [PATCH net v2 3/3] selftests: net: tun: test outer protocol of UDP tunnel GSO packets

[sashiko-bot]

> 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.
> 
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104050131.629967-1-edumazet@kernel.org?part=3

Re: [PATCH net v2 3/3] selftests: net: tun: test outer protocol of UDP tunnel GSO packets

[netdev-bot+sashiko]

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

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

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

- [Medium] The new tun_vnet_udptnl_proto and tun_vnet_udptnl_proto_l3
  tests do not work on big-endian hosts.
- [Low] The new comment in virtio_net_l4_start()
  (include/linux/virtio_net.h) says the code does something it does not.

--

selftests: net: tun: test outer protocol of UDP tunnel GSO packets

This adds tun selftests that write single 4in4 Geneve GSO frames to tap
and IFF_TUN devices with UDP tunnel offloads enabled. They check that
frames with a matching outer IPv4 header are accepted. They also check
that a mismatched outer ethertype, tun_pi protocol or IP version is
rejected with EINVAL.

> diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> index 34de2580e32ec..81410afbbcc8d 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)

[ ... ]

> +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;

[Severity: Medium]
Do these new tests work on big-endian hosts? In struct
virtio_net_hdr_v1_hash_tunnel, outer_th_offset and inner_nh_offset are
__le16. Here they get native-endian arithmetic, applied on top of the
host-order values that build_virtio_net_hdr_v1_hash_tunnel() in
tuntap_helpers.h already wrote:

    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);

The kernel always decodes the tunnel offsets as little-endian. csum_start,
on the other hand, follows the tun vnet endianness:

include/linux/virtio_net.h:virtio_net_hdr_tnl_to_skb() {
    ...
    inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start);
    inner_nh = le16_to_cpu(vhdr->inner_nh_offset);
    outer_th = le16_to_cpu(vhdr->outer_th_offset);
    if (outer_th < outer_l3min ||
        inner_nh < outer_th + sizeof(struct udphdr) ||
        inner_th < inner_nh + inner_l3min)
        return -EINVAL;
    ...
}

tun_open() never issues TUNSETVNETLE. On s390x or BE ppc64, csum_start is
therefore read in host order, but outer_th 34 is read as 0x2200 and
inner_nh 64 as 0x4000. The offset check above then returns -EINVAL before
virtio_net_tnl_offsets_ok() is reached.

Wouldn't outer_proto_ok then fail on BE? And wouldn't outer_proto_bad
pass there without ever reaching the new outer protocol validation?

build_udptnl_l3_frame() does the same:

    vh->outer_th_offset += IPV4_OPT_LEN;
    vh->inner_nh_offset += IPV4_OPT_LEN;

The helper's host-order writes predate this patch, and the existing
tun_vnet_udptnl tests share the problem. Could these fields be converted
with htole16()/le16toh() here and in the helper?

> +
> +	if (vlan) {
> +		uint16_t vlan_tag[2] = { htons(100), htons(l3proto) };

[ ... ]

The following is about the first patch in this series, commit
fca58b5faf90 ("net: virtio: validate outer protocol of UDP tunnel GSO
packets"), which adds the code that the outer_proto_bad tests exercise.

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

[ ... ]

> +	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, AH included.
> +			 */
> +			*ip_proto = ipv6_find_hdr(skb, thoff, -1, NULL,
> +						  &flags);

[Severity: Low]
This isn't a bug, but does this comment in virtio_net_l4_start() match
what ipv6_find_hdr() does with these arguments? With flags = IP6_FH_F_AUTH
and target = -1, it stops at the AH header instead of walking past it:

net/ipv6/exthdrs_core.c:ipv6_find_hdr() {
    ...
    } else if (nexthdr == NEXTHDR_AUTH) {
        if (flags && (*flags & IP6_FH_F_AUTH) && (target < 0))
            break;
    ...
}

So for an outer IPv6 header followed by AH, ip_proto comes back as
NEXTHDR_AUTH and virtio_net_tnl_offsets_ok() rejects the frame. The commit
message says this is intended:

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

Could the comment say that the walk stops at AH?

> +			if (*ip_proto < 0 || flags & IP6_FH_F_FRAG)
> +				return false;
> +		}
> +		return true;
> +	}

[ ... ]

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

Re: [PATCH net v2 3/3] selftests: net: tun: test outer protocol of UDP tunnel GSO packets

[Eric Dumazet]

Le jeu. 5 nov. 2026 à 11:08, <netdev-bot+sashiko@kernel.org> a écrit :
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The new tun_vnet_udptnl_proto and tun_vnet_udptnl_proto_l3
>   tests do not work on big-endian hosts.
> - [Low] The new comment in virtio_net_l4_start()
>   (include/linux/virtio_net.h) says the code does something it does not.
>
> --
>
> selftests: net: tun: test outer protocol of UDP tunnel GSO packets
>
> This adds tun selftests that write single 4in4 Geneve GSO frames to tap
> and IFF_TUN devices with UDP tunnel offloads enabled. They check that
> frames with a matching outer IPv4 header are accepted. They also check
> that a mismatched outer ethertype, tun_pi protocol or IP version is
> rejected with EINVAL.
>
> > diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> > index 34de2580e32ec..81410afbbcc8d 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)
>
> [ ... ]
>
> > +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;
>
> [Severity: Medium]
> Do these new tests work on big-endian hosts? In struct
> virtio_net_hdr_v1_hash_tunnel, outer_th_offset and inner_nh_offset are
> __le16. Here they get native-endian arithmetic, applied on top of the
> host-order values that build_virtio_net_hdr_v1_hash_tunnel() in
> tuntap_helpers.h already wrote:
>
>     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);
>
> The kernel always decodes the tunnel offsets as little-endian. csum_start,
> on the other hand, follows the tun vnet endianness:
>
> include/linux/virtio_net.h:virtio_net_hdr_tnl_to_skb() {
>     ...
>     inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start);
>     inner_nh = le16_to_cpu(vhdr->inner_nh_offset);
>     outer_th = le16_to_cpu(vhdr->outer_th_offset);
>     if (outer_th < outer_l3min ||
>         inner_nh < outer_th + sizeof(struct udphdr) ||
>         inner_th < inner_nh + inner_l3min)
>         return -EINVAL;
>     ...
> }
>
> tun_open() never issues TUNSETVNETLE. On s390x or BE ppc64, csum_start is
> therefore read in host order, but outer_th 34 is read as 0x2200 and
> inner_nh 64 as 0x4000. The offset check above then returns -EINVAL before
> virtio_net_tnl_offsets_ok() is reached.
>
> Wouldn't outer_proto_ok then fail on BE? And wouldn't outer_proto_bad
> pass there without ever reaching the new outer protocol validation?
>
> build_udptnl_l3_frame() does the same:
>
>     vh->outer_th_offset += IPV4_OPT_LEN;
>     vh->inner_nh_offset += IPV4_OPT_LEN;
>
> The helper's host-order writes predate this patch, and the existing
> tun_vnet_udptnl tests share the problem. Could these fields be converted
> with htole16()/le16toh() here and in the helper?


Right. build_virtio_net_hdr_v1_hash_tunnel() writes the tunnel offsets
in host order since commit a942fcd72e97 ("selftest: tun: Introduce
tuntap_helpers.h header for TUN/TAP testing"), so all UDP tunnel tests
in tun.c are broken on big-endian hosts, not only the new ones.

v3 adds a patch converting the offsets with htole16() in the helper,
and le16toh()/htole16() where the tests adjust them.


>
> > +
> > +     if (vlan) {
> > +             uint16_t vlan_tag[2] = { htons(100), htons(l3proto) };
>
> [ ... ]
>
> The following is about the first patch in this series, commit
> fca58b5faf90 ("net: virtio: validate outer protocol of UDP tunnel GSO
> packets"), which adds the code that the outer_proto_bad tests exercise.
>
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index d6466f96cdd00..b9c8babb1fd73 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
>
> [ ... ]
>
> > +     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, AH included.
> > +                      */
> > +                     *ip_proto = ipv6_find_hdr(skb, thoff, -1, NULL,
> > +                                               &flags);
>
> [Severity: Low]
> This isn't a bug, but does this comment in virtio_net_l4_start() match
> what ipv6_find_hdr() does with these arguments?

Same as for patch 1: the comment is wrong, I will fix it in v3.


>With flags = IP6_FH_F_AUTH
> and target = -1, it stops at the AH header instead of walking past it:
>
> net/ipv6/exthdrs_core.c:ipv6_find_hdr() {
>     ...
>     } else if (nexthdr == NEXTHDR_AUTH) {
>         if (flags && (*flags & IP6_FH_F_AUTH) && (target < 0))
>             break;
>     ...
> }
>
> So for an outer IPv6 header followed by AH, ip_proto comes back as
> NEXTHDR_AUTH and virtio_net_tnl_offsets_ok() rejects the frame. The commit
> message says this is intended:
>
>   Unlike the original helpers, ipv6_find_hdr() stops at an AH header,
>   as Willem suggested in the review of that series.
>
> Could the comment say that the walk stops at AH?
>
> > +                     if (*ip_proto < 0 || flags & IP6_FH_F_FRAG)
> > +                             return false;
> > +             }
> > +             return true;
> > +     }
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261104050131.629967-1-edumazet%40kernel.org