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

[PATCH net v5 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 fixes the producer side: virtio_net_hdr_tnl_from_skb() did
not account for the VLAN tag tun inserts in the frame, so
outer_th_offset and inner_nh_offset were 4 bytes short. Without it,
patch 2 would drop such frames. Hosts without patch 1 still send
these offsets, so it should go to stable along with patch 2.

Patch 2 validates the outer headers, using the parsing helpers from
Michael's series "net: resolve uncommon partial checksums at ingress",
as Willem suggested. The callers tell whether frames start with an
Ethernet header, as the device type of a tun device can be changed
with TUNSETLINK. The rest of Michael's series (checksum validation and
completion) is left to him.

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

The selftests of v3 will follow in a separate series, with the fixes
Sashiko found in them.

v5:
- New patch 1: account for the VLAN tag in the tunnel offsets built by
  virtio_net_hdr_tnl_from_skb() (Sashiko).
- Patch 2: add Michael's Acked-by, no code change.

v4: https://lore.kernel.org/netdev/20261106095340.2113683-1-edumazet@kernel.org/
- Squash v3 patch 2 into patch 1 (Willem). Dropped Michael's Acked-by,
  as virtio_net_l3_start() now takes a has_eth argument.
- Patch 2: clear skb->protocol in tun_napi_alloc_frags(), the only
  place that needs it (Willem).
- Drop the selftests from this series (Willem).

v3: https://lore.kernel.org/netdev/20261105115715.1466924-1-edumazet@kernel.org/
- Patch 1: fix a comment, ipv6_find_hdr() stops at AH (Sashiko).
- New patch 2: do not use the device type to find the outer L3 header,
  TUNSETLINK can change it (Sashiko).
- New patch 3: clear a stale skb->protocol on IFF_NAPI_FRAGS skbs
  (Sashiko).
- More selftests, and big-endian fixes for them (Sashiko).

v2: https://lore.kernel.org/netdev/20261104050131.629967-1-edumazet@kernel.org/
- 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).
- Selftest fixes, IFF_TUN tests.

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

Eric Dumazet (3):
  net: virtio: account for the VLAN tag in UDP tunnel GSO offsets
  net: virtio: validate outer protocol of UDP tunnel GSO packets
  tun: clear stale skb->protocol of NAPI frags skbs

 drivers/net/tun.c          |  15 +++-
 drivers/net/tun_vnet.h     |   5 +-
 drivers/net/virtio_net.c   |   2 +-
 include/linux/virtio_net.h | 176 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 191 insertions(+), 7 deletions(-)


base-commit: 1d5e2c57e9ffc134e14b3f837c8d7c53b508bd22
-- 
2.53.0

[PATCH net v5 1/3] net: virtio: account for the VLAN tag in UDP tunnel GSO offsets

[Eric Dumazet]

When the skb carries a hwaccel VLAN tag, tun_put_user() inserts the
802.1Q header in the frame it copies to user space or vhost-net, and
passes vlan_hlen = VLAN_HLEN to virtio_net_hdr_tnl_from_skb().

virtio_net_hdr_from_skb() adds vlan_hlen to csum_start, but
virtio_net_hdr_tnl_from_skb() does not add it to inner_nh_offset and
outer_th_offset. Both offsets are then 4 bytes short of the real inner
L3 and outer UDP headers in the emitted frame.

The receiver sets inner_network_header and transport_header from these
wrong offsets. This goes unnoticed for local delivery, but a forwarded
packet that gets segmented again uses the wrong inner headers. A
receiver that checks the outer headers against outer_th_offset drops
the packet instead.

tun advertises NETIF_F_HW_VLAN_CTAG_TX and NETIF_F_HW_VLAN_STAG_TX in
hw_enc_features, and its vlan_features keep the UDP tunnel GSO
features, so this happens with VLAN tagged UDP tunnel GSO traffic, for
example from a VLAN filtering bridge with a tagged port, or from a vlan
device on top of a tap.

Add vlan_hlen to both offsets, like csum_start.

virtio_net passes vlan_hlen = 0 and is not affected.

Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/179421814766.994134.3669228181251158303@kernel.org/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 include/linux/virtio_net.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index d6466f96cdd0..ab44342cf09f 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -461,8 +461,11 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
 	if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM)
 		hdr->flags |= VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM;
 
-	inner_nh = skb->inner_network_header - skb_headroom(skb);
-	outer_th = skb->transport_header - skb_headroom(skb);
+	/* Like csum_start, the offsets must account for a VLAN tag
+	 * the caller inserts in the frame.
+	 */
+	inner_nh = skb->inner_network_header - skb_headroom(skb) + vlan_hlen;
+	outer_th = skb->transport_header - skb_headroom(skb) + vlan_hlen;
 	vhdr->inner_nh_offset = cpu_to_le16(inner_nh);
 	vhdr->outer_th_offset = cpu_to_le16(outer_th);
 	return 0;
-- 
2.53.0

Re: [PATCH net v5 1/3] net: virtio: account for the VLAN tag in UDP tunnel GSO offsets

[netdev-bot+sashiko]

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

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

- [High] virtio_net_hdr_tnl_from_skb() (include/linux/virtio_net.h)
  chooses the outer-family GSO bit with `if (skb->protocol ==…

--

net: virtio: account for the VLAN tag in UDP tunnel GSO offsets

This commit adds vlan_hlen to inner_nh_offset and outer_th_offset in
virtio_net_hdr_tnl_from_skb(), the same way virtio_net_hdr_from_skb()
already adjusts csum_start. It targets the case where tun_put_user()
inserts a hwaccel VLAN tag into the frame, which leaves both tunnel
offsets 4 bytes short.

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

> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index d6466f96cdd00..ab44342cf09f7 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -461,8 +461,11 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>  	if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM)
>  		hdr->flags |= VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM;
>  
> -	inner_nh = skb->inner_network_header - skb_headroom(skb);
> -	outer_th = skb->transport_header - skb_headroom(skb);
> +	/* Like csum_start, the offsets must account for a VLAN tag
> +	 * the caller inserts in the frame.
> +	 */
> +	inner_nh = skb->inner_network_header - skb_headroom(skb) + vlan_hlen;
> +	outer_th = skb->transport_header - skb_headroom(skb) + vlan_hlen;

[Severity: High]
This fixes the hwaccel case. There, vlan_hlen is VLAN_HLEN and
skb->protocol still holds the real L3 protocol. What happens when the
skb carries its VLAN tag inside the packet data?

Just above this hunk, the outer family comes 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;
    ...
}

For an in-band tagged frame, vlan_insert_tag_set_proto() has already set
skb->protocol to ETH_P_8021Q or ETH_P_8021AD. Wouldn't an IPv6 outer
header then be reported as VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4?

The commit message says:

    virtio_net passes vlan_hlen = 0 and is not affected.

virtio_net has no NETIF_F_HW_VLAN_CTAG_TX, so validate_xmit_vlan() pushes
every tag into the data:

net/core/dev.c:validate_xmit_vlan() {
	if (skb_vlan_tag_present(skb) &&
	    !vlan_hw_offload_capable(features, skb->vlan_proto))
		skb = __vlan_hwaccel_push_inside(skb);
    ...
}

Its vlan_features and hw_enc_features still include
NETIF_F_GSO_UDP_TUNNEL. Take a VXLAN or Geneve device over an IPv6
underlay on a VLAN subinterface: it sends unsegmented skbs with
skb->protocol == ETH_P_8021Q into
xmit_skb()->virtio_net_hdr_tnl_from_skb(). Is virtio_net really
unaffected in that case?

tun seems to reach the same path in three cases: ethtool -K tapX txvlan
off, QinQ (outer tag in hwaccel, inner tag in the data), and a vlan
device created with reorder_hdr off.

Before this series, a Linux receiver used this bit only to compute a
looser outer_l3min, so the wrong value did no harm. Later in the series,
"net: virtio: validate outer protocol of UDP tunnel GSO packets" adds
virtio_net_tnl_offsets_ok(). That function walks the VLAN tags and
rejects the mismatch:

	if (!virtio_net_l3_start(skb, has_eth, &nhoff, &proto))
		return false;
	if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
		return false;

Once that check is in place, will tun_get_user(), tun_xdp_one() and
virtnet_receive_done() drop every VLAN tagged UDP tunnel GSO packet with
an IPv6 outer header from these senders?

Could the outer family be taken from vlan_get_protocol(skb) here instead
of skb->protocol?

>  	vhdr->inner_nh_offset = cpu_to_le16(inner_nh);
>  	vhdr->outer_th_offset = cpu_to_le16(outer_th);
>  	return 0;

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

Re: [PATCH net v5 1/3] net: virtio: account for the VLAN tag in UDP tunnel GSO offsets

[Eric Dumazet]

Le mar. 10 nov. 2026 à 20:38, <netdev-bot+sashiko@kernel.org> a écrit :
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · 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).
>
> - [High] virtio_net_hdr_tnl_from_skb() (include/linux/virtio_net.h)
>   chooses the outer-family GSO bit with `if (skb->protocol ==…
>
> --
>
> net: virtio: account for the VLAN tag in UDP tunnel GSO offsets
>
> This commit adds vlan_hlen to inner_nh_offset and outer_th_offset in
> virtio_net_hdr_tnl_from_skb(), the same way virtio_net_hdr_from_skb()
> already adjusts csum_start. It targets the case where tun_put_user()
> inserts a hwaccel VLAN tag into the frame, which leaves both tunnel
> offsets 4 bytes short.
>
> Closes: https://lore.kernel.org/netdev/179421814766.994134.3669228181251158303@kernel.org/
>
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index d6466f96cdd00..ab44342cf09f7 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -461,8 +461,11 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >       if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM)
> >               hdr->flags |= VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM;
> >
> > -     inner_nh = skb->inner_network_header - skb_headroom(skb);
> > -     outer_th = skb->transport_header - skb_headroom(skb);
> > +     /* Like csum_start, the offsets must account for a VLAN tag
> > +      * the caller inserts in the frame.
> > +      */
> > +     inner_nh = skb->inner_network_header - skb_headroom(skb) + vlan_hlen;
> > +     outer_th = skb->transport_header - skb_headroom(skb) + vlan_hlen;
>
> [Severity: High]
> This fixes the hwaccel case. There, vlan_hlen is VLAN_HLEN and
> skb->protocol still holds the real L3 protocol. What happens when the
> skb carries its VLAN tag inside the packet data?
>
> Just above this hunk, the outer family comes 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;
>     ...
> }
>
> For an in-band tagged frame, vlan_insert_tag_set_proto() has already set
> skb->protocol to ETH_P_8021Q or ETH_P_8021AD. Wouldn't an IPv6 outer
> header then be reported as VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4?
>
> The commit message says:
>
>     virtio_net passes vlan_hlen = 0 and is not affected.
>
> virtio_net has no NETIF_F_HW_VLAN_CTAG_TX, so validate_xmit_vlan() pushes
> every tag into the data:
>
> net/core/dev.c:validate_xmit_vlan() {
>         if (skb_vlan_tag_present(skb) &&
>             !vlan_hw_offload_capable(features, skb->vlan_proto))
>                 skb = __vlan_hwaccel_push_inside(skb);
>     ...
> }
>
> Its vlan_features and hw_enc_features still include
> NETIF_F_GSO_UDP_TUNNEL. Take a VXLAN or Geneve device over an IPv6
> underlay on a VLAN subinterface: it sends unsegmented skbs with
> skb->protocol == ETH_P_8021Q into
> xmit_skb()->virtio_net_hdr_tnl_from_skb(). Is virtio_net really
> unaffected in that case?
>
> tun seems to reach the same path in three cases: ethtool -K tapX txvlan
> off, QinQ (outer tag in hwaccel, inner tag in the data), and a vlan
> device created with reorder_hdr off.
>
> Before this series, a Linux receiver used this bit only to compute a
> looser outer_l3min, so the wrong value did no harm. Later in the series,
> "net: virtio: validate outer protocol of UDP tunnel GSO packets" adds
> virtio_net_tnl_offsets_ok(). That function walks the VLAN tags and
> rejects the mismatch:
>
>         if (!virtio_net_l3_start(skb, has_eth, &nhoff, &proto))
>                 return false;
>         if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
>                 return false;
>
> Once that check is in place, will tun_get_user(), tun_xdp_one() and
> virtnet_receive_done() drop every VLAN tagged UDP tunnel GSO packet with
> an IPv6 outer header from these senders?
>
> Could the outer family be taken from vlan_get_protocol(skb) here instead
> of skb->protocol?

Right. When __vlan_hwaccel_push_inside() inserts the tag, skb->protocol
becomes the VLAN protocol, and an outer IPv6 header is reported as
IPv4. The offsets are correct in that case, only the family is wrong.
This is the default for virtio_net, which has no VLAN TX offload, and
happens with tun when VLAN TX offload is off or with QinQ.

v6 fixes both issues in this patch, using vlan_get_protocol() for the
outer family. The patch is renamed to
"net: virtio: fix UDP tunnel GSO header of VLAN tagged packets".

pw-bot: cr

[PATCH net v5 2/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: 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:

- the callers tell whether the frame starts with an Ethernet header,
  with a new has_eth argument of virtio_net_hdr_tnl_to_skb(), instead
  of using the device type. tun frames packets from its IFF_TAP and
  IFF_TUN flags, and TUNSETLINK can set the device type to any value.
  virtio_net and tun_xdp_one() always pass true, tun_get_user() passes
  whether the device is IFF_TAP.
- 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>
---
 drivers/net/tun.c          |   9 +-
 drivers/net/tun_vnet.h     |   5 +-
 drivers/net/virtio_net.c   |   2 +-
 include/linux/virtio_net.h | 169 +++++++++++++++++++++++++++++++++++++
 4 files changed, 180 insertions(+), 5 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 ab44342cf09f..0b7a8569812f 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,145 @@ 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?
+ * 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, bool has_eth, int *nhoff,
+		    __be16 *proto)
+{
+	if (has_eth) {
+		__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,9 +424,33 @@ 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 has_eth, bool outer_isv6,
+			  unsigned int outer_th)
+{
+	int nhoff;
+	__be16 proto;
+	unsigned int thoff;
+	int ip_proto;
+
+	if (!virtio_net_l3_start(skb, has_eth, &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,
+			  bool has_eth,
 			  bool tnl_hdr_negotiated,
 			  bool tnl_csum_negotiated,
 			  bool little_endian)
@@ -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, has_eth, 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 v5 2/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: 0 · Medium: 2 (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 check in virtio_net_tnl_offsets_ok()
  (include/linux/virtio_net.h:442), `if (outer_isv6 != (proto ==…
- [Medium] virtio_net_l3_start() (include/linux/virtio_net.h:112) rejects
  the frame when a non-zero skb->protocol differs from the Ethernet…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the patch's
  new has_eth=false path is effectively unreachable for valid frames.

--

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

This patch makes virtio_net_hdr_tnl_to_skb() check that the frame really
carries the outer IPv4/IPv6 and UDP headers that the virtio UDP tunnel GSO
header claims. It rejects frames with mismatched ethertypes, fragments, or
wrong offsets. Callers now pass a new has_eth argument that says whether
the frame starts with an Ethernet header.

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

[ ... ]

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

[ ... ]

> +static inline bool
> +virtio_net_l3_start(struct sk_buff *skb, bool has_eth, int *nhoff,
> +		    __be16 *proto)
> +{
> +	if (has_eth) {
> +		__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;

[Severity: Medium]
On the tun IFF_NAPI_FRAGS path, can a stale skb->protocol cause valid
frames to be rejected here?

In that mode tun_get_user() gets the skb from napi_get_frags(). If GRO
merged the previous frame with GRO_MERGED_FREE, napi_reuse_skb() recycles
the skb without clearing skb->protocol. That field still holds the
previous frame's h_proto, set by napi_frags_skb().

tun_get_user() reaches this check before eth_type_trans() updates
skb->protocol:

tun_get_user()
    tun_vnet_hdr_tnl_to_skb()
        virtio_net_hdr_tnl_to_skb()
            virtio_net_tnl_offsets_ok()
                virtio_net_l3_start()
    ...
    skb->protocol = eth_type_trans(skb, tun->dev);

With only this patch applied, a valid UDP tunnel GSO frame can have a
different ethertype from the previous frame (IPv6 after IPv4, or VLAN
tagged after untagged). Would that frame be dropped with -EINVAL and
counted in rx_frame_errors?

The next patch in the series, "tun: clear stale skb->protocol of NAPI
frags skbs", fixes this by setting skb->protocol = 0 in
tun_napi_alloc_frags() right after napi_get_frags(). Could that fix go
before this patch, so this regression never shows up between the two
commits?

> +		*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: Medium]
This isn't a bug introduced by this patch, but can this has_eth == false
branch ever accept a valid frame?

For IFF_TUN, tun_get_user() resets the network header to offset 0 and
passes has_eth = false, so nhoff is 0 here. virtio_net_tnl_offsets_ok()
then requires outer_th to equal the real transport offset. That is 20 for
IPv4 without options, or 40 for IPv6 without extension headers.

Before that check, virtio_net_hdr_tnl_to_skb() still applies an
Ethernet-sized minimum, whatever has_eth is:

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;
    ...
}

That minimum is 34 or 54. A frame only passes both checks if it carries at
least 16 bytes of IPv4 options or 14 bytes of IPv6 extension headers.

The TX side also produces offsets that the RX side rejects. In
tun_put_user()->virtio_net_hdr_tnl_from_skb(), outer_th is computed
relative to skb->data, and an IFF_TUN frame has no Ethernet header:

	outer_th = skb->transport_header - skb_headroom(skb) + vlan_hlen;

tun_set_offload() also lets IFF_TUN enable UDP tunnel GSO.

Should outer_l3min leave out the ETH_HLEN term when has_eth is false?

[ ... ]

> @@ -282,9 +424,33 @@ 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 has_eth, bool outer_isv6,
> +			  unsigned int outer_th)
> +{
> +	int nhoff;
> +	__be16 proto;
> +	unsigned int thoff;
> +	int ip_proto;
> +
> +	if (!virtio_net_l3_start(skb, has_eth, &nhoff, &proto))
> +		return false;
> +	if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
> +		return false;

[Severity: Medium]
Could this check drop frames built by the kernel's own producer?

virtio_net_hdr_tnl_from_skb() picks the tunnel family from skb->protocol:

	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;

When the VLAN tag is inline, skb->protocol is ETH_P_8021Q or
ETH_P_8021AD, so a frame with an IPv6 outer header gets labelled IPV4.

This looks deterministic for a virtio_net guest that sends VXLAN or Geneve
over an IPv6 underlay on a VLAN subinterface. virtio_net never advertises
NETIF_F_HW_VLAN_CTAG_TX, but it keeps NETIF_F_GSO_UDP_TUNNEL in
vlan_features, so the tag is pushed inline while the skb is still GSO:

vlan_dev_hard_start_xmit()          /* tag kept as hwaccel */
...
validate_xmit_vlan()
  __vlan_hwaccel_push_inside()
    vlan_insert_tag_set_proto()     /* skb->protocol = vlan_proto */
...
xmit_skb()
  virtio_net_hdr_tnl_from_skb()     /* labelled GSO_UDP_TUNNEL_IPV4 */

Before this patch, the host only checked outer_th against ETH_HLEN plus
the IPv4 minimum, so it accepted the frame. GSO later parsed
8021Q -> IPv6 -> UDP and segmented it correctly.

With this patch, virtio_net_l3_start() walks past the tag and finds
ETH_P_IPV6 while outer_isv6 is false. tun_get_user() and tun_xdp_one()
then drop the frame with -EINVAL and increment rx_frame_errors.

The same mislabel can happen from host to guest through tun_put_user()
when the C-tag is inline, for example QinQ with a hwaccel S-tag. In that
case virtnet_receive_done() would drop the frame as "bad gso".

In all these cases the offsets are correct and only the family label is
wrong. The commit message does not mention this change in behaviour.

Should virtio_net_hdr_tnl_from_skb() derive the family with something like
vlan_get_protocol() or skb_network_protocol() as part of this fix?

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

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

[Eric Dumazet]

Le mar. 10 nov. 2026 à 20:38, <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: 2 (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 check in virtio_net_tnl_offsets_ok()
>   (include/linux/virtio_net.h:442), `if (outer_isv6 != (proto ==…
> - [Medium] virtio_net_l3_start() (include/linux/virtio_net.h:112) rejects
>   the frame when a non-zero skb->protocol differs from the Ethernet…
>
> Pre-existing issues:
> - [Medium] This problem wasn't introduced by this patch, but the patch's
>   new has_eth=false path is effectively unreachable for valid frames.
>
> --
>
> net: virtio: validate outer protocol of UDP tunnel GSO packets
>
> This patch makes virtio_net_hdr_tnl_to_skb() check that the frame really
> carries the outer IPv4/IPv6 and UDP headers that the virtio UDP tunnel GSO
> header claims. It rejects frames with mismatched ethertypes, fragments, or
> wrong offsets. Callers now pass a new has_eth argument that says whether
> the frame starts with an Ethernet header.
>
> 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 ab44342cf09f7..0b7a8569812fe 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
>
> [ ... ]
>
> > @@ -48,6 +51,145 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
>
> [ ... ]
>
> > +static inline bool
> > +virtio_net_l3_start(struct sk_buff *skb, bool has_eth, int *nhoff,
> > +                 __be16 *proto)
> > +{
> > +     if (has_eth) {
> > +             __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;
>
> [Severity: Medium]
> On the tun IFF_NAPI_FRAGS path, can a stale skb->protocol cause valid
> frames to be rejected here?
>
> In that mode tun_get_user() gets the skb from napi_get_frags(). If GRO
> merged the previous frame with GRO_MERGED_FREE, napi_reuse_skb() recycles
> the skb without clearing skb->protocol. That field still holds the
> previous frame's h_proto, set by napi_frags_skb().
>
> tun_get_user() reaches this check before eth_type_trans() updates
> skb->protocol:
>
> tun_get_user()
>     tun_vnet_hdr_tnl_to_skb()
>         virtio_net_hdr_tnl_to_skb()
>             virtio_net_tnl_offsets_ok()
>                 virtio_net_l3_start()
>     ...
>     skb->protocol = eth_type_trans(skb, tun->dev);
>
> With only this patch applied, a valid UDP tunnel GSO frame can have a
> different ethertype from the previous frame (IPv6 after IPv4, or VLAN
> tagged after untagged). Would that frame be dropped with -EINVAL and
> counted in rx_frame_errors?
>
> The next patch in the series, "tun: clear stale skb->protocol of NAPI
> frags skbs", fixes this by setting skb->protocol = 0 in
> tun_napi_alloc_frags() right after napi_get_frags(). Could that fix go
> before this patch, so this regression never shows up between the two
> commits?

Yes, v6 moves it before this patch.

>
> > +             *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: Medium]
> This isn't a bug introduced by this patch, but can this has_eth == false
> branch ever accept a valid frame?
>

As said in the v4 review, this predates this series: UDP tunnel GSO
over IFF_TUN has never worked with plain IPv4/IPv6 outer headers. Both
directions would need changes, which would enable something that never
worked rather than fix a regression. I will look at it separately, for
net-next.

> For IFF_TUN, tun_get_user() resets the network header to offset 0 and
> passes has_eth = false, so nhoff is 0 here. virtio_net_tnl_offsets_ok()
> then requires outer_th to equal the real transport offset. That is 20 for
> IPv4 without options, or 40 for IPv6 without extension headers.
>
> Before that check, virtio_net_hdr_tnl_to_skb() still applies an
> Ethernet-sized minimum, whatever has_eth is:
>
> 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;
>     ...
> }
>
> That minimum is 34 or 54. A frame only passes both checks if it carries at
> least 16 bytes of IPv4 options or 14 bytes of IPv6 extension headers.
>
> The TX side also produces offsets that the RX side rejects. In
> tun_put_user()->virtio_net_hdr_tnl_from_skb(), outer_th is computed
> relative to skb->data, and an IFF_TUN frame has no Ethernet header:
>
>         outer_th = skb->transport_header - skb_headroom(skb) + vlan_hlen;
>
> tun_set_offload() also lets IFF_TUN enable UDP tunnel GSO.
>
> Should outer_l3min leave out the ETH_HLEN term when has_eth is false?
>
> [ ... ]
>
> > @@ -282,9 +424,33 @@ 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 has_eth, bool outer_isv6,
> > +                       unsigned int outer_th)
> > +{
> > +     int nhoff;
> > +     __be16 proto;
> > +     unsigned int thoff;
> > +     int ip_proto;
> > +
> > +     if (!virtio_net_l3_start(skb, has_eth, &nhoff, &proto))
> > +             return false;
> > +     if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
> > +             return false;
>
> [Severity: Medium]
> Could this check drop frames built by the kernel's own producer?
>
> virtio_net_hdr_tnl_from_skb() picks the tunnel family from skb->protocol:
>
>         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;
>
> When the VLAN tag is inline, skb->protocol is ETH_P_8021Q or
> ETH_P_8021AD, so a frame with an IPv6 outer header gets labelled IPV4.
>
> This looks deterministic for a virtio_net guest that sends VXLAN or Geneve
> over an IPv6 underlay on a VLAN subinterface. virtio_net never advertises
> NETIF_F_HW_VLAN_CTAG_TX, but it keeps NETIF_F_GSO_UDP_TUNNEL in
> vlan_features, so the tag is pushed inline while the skb is still GSO:
>
> vlan_dev_hard_start_xmit()          /* tag kept as hwaccel */
> ...
> validate_xmit_vlan()
>   __vlan_hwaccel_push_inside()
>     vlan_insert_tag_set_proto()     /* skb->protocol = vlan_proto */
> ...
> xmit_skb()
>   virtio_net_hdr_tnl_from_skb()     /* labelled GSO_UDP_TUNNEL_IPV4 */
>
> Before this patch, the host only checked outer_th against ETH_HLEN plus
> the IPv4 minimum, so it accepted the frame. GSO later parsed
> 8021Q -> IPv6 -> UDP and segmented it correctly.
>
> With this patch, virtio_net_l3_start() walks past the tag and finds
> ETH_P_IPV6 while outer_isv6 is false. tun_get_user() and tun_xdp_one()
> then drop the frame with -EINVAL and increment rx_frame_errors.
>
> The same mislabel can happen from host to guest through tun_put_user()
> when the C-tag is inline, for example QinQ with a hwaccel S-tag. In that
> case virtnet_receive_done() would drop the frame as "bad gso".
>
> In all these cases the offsets are correct and only the family label is
> wrong. The commit message does not mention this change in behaviour.
>
> Should virtio_net_hdr_tnl_from_skb() derive the family with something like
> vlan_get_protocol() or skb_network_protocol() as part of this fix?

Same issue as the one reported on patch 1, fixed there in v6 with
vlan_get_protocol().

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

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, and IFF_NAPI_FRAGS needs
CAP_NET_ADMIN.

Clear skb->protocol in tun_napi_alloc_frags(), like skbs allocated
by tun_alloc_skb() or build_skb(). eth_type_trans() sets it later.

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/
Suggested-by: Willem de Bruijn <willemb@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 drivers/net/tun.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3920b3dff1e8..9aca5056f7c5 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1500,6 +1500,12 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
 	if (!skb)
 		return ERR_PTR(-ENOMEM);
 
+	/* An skb recycled by napi_reuse_skb() keeps the protocol of a
+	 * previous frame. eth_type_trans() sets it after the virtio
+	 * header checks, which must not see a stale one.
+	 */
+	skb->protocol = 0;
+
 	linear = iov_iter_single_seg_count(it);
 	err = __skb_grow(skb, linear);
 	if (err)
-- 
2.53.0