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

On Mon, Sep 28, 2026 at 10:10 AM Michael S. Tsirkin [off-list ref] wrote:

On Mon, Sep 28, 2026 at 08:31:53AM +0200, Eric Dumazet wrote:

quoted

On Mon, Sep 28, 2026 at 8:22 AM Eric Dumazet [off-list ref] wrote:

quoted

On Mon, Sep 28, 2026 at 3:31 AM Michael S. Tsirkin [off-list ref] wrote:

quoted

On Sun, Sep 27, 2026 at 07:55:36PM +0000, Eric Dumazet wrote:

quoted

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

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

Hmm. I have:

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

#define HH_DATA_MOD     16

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

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

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

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

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

net: always dissect GSO packets in __virtio_net_hdr_to_skb()

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

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

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

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

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

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

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

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

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

Thanks for the reproducer!

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

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

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

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

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 8902d5a5c418c46af0a6e7b26ae89948c20b0b54..d6466f96cdd00cdf059d4fa8842ba1672780e556

100644

--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h

@@ -118,15 +118,9 @@ static inline int __virtio_net_hdr_to_skb(struct

sk_buff *skb,
                struct flow_keys_basic keys;

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

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