[PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

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

- Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
  with min_t(u16, ...). This truncates skb->len and returns a bogus small
  transport offset when skb->len modulo 65536 is smaller than the
  transport offset. Offsets that do not fit in the u16 thoff now fail the
  dissection instead of being silently truncated.

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

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

v3:
 - New patch 1: avoid u16 truncation of skb->len when computing thoff in
   __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
   frames before eth_type_trans(), with skb->len up to 65549 for a GSO
   frame carrying a maximal IPv4 packet (Sashiko).
 - Patch 3: truncate the TCP header after 10 bytes so that the test
   requires the transport offset found by flow dissection, and add a
   65540-byte frame test (Sashiko).
 - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/

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

Eric Dumazet (3):
  flow_dissector: avoid u16 truncation of skb->len when computing thoff
  net: always dissect GSO packets in __virtio_net_hdr_to_skb()
  selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM

 arch/um/drivers/vector_transports.c |   1 +
 drivers/net/tun.c                   |  23 +++--
 drivers/net/virtio_net.c            |   2 +
 include/linux/virtio_net.h          |  60 +++++-------
 net/core/flow_dissector.c           |   9 +-
 tools/testing/selftests/net/tun.c   | 136 ++++++++++++++++++++++++++++
 6 files changed, 186 insertions(+), 45 deletions(-)

-- 
2.56.0.rc1.315.gc6ed9934b7-goog

[PATCH v3 net 1/3] flow_dissector: avoid u16 truncation of skb->len when computing thoff

[Eric Dumazet]

__skb_flow_dissect() computes key_control->thoff using:

  key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);

min_t(u16, ...) casts both arguments to u16 before comparing them.
Whenever skb->len (or hlen) modulo 65536 is smaller than nhoff, the
truncated length wins and thoff is set to a bogus small value, even
when the dissection succeeded. For example, an IPv4/TCP frame with
skb->len == 65540 and nhoff == 34 gets thoff == 4, so callers such as
skb_probe_transport_header() point the transport header inside the
Ethernet header.

Such skbs are not exotic:
- At the time of commit d0c081b49137 ("flow_dissector: properly cap
  thoff field"), AF_PACKET with PACKET_VNET_HDR could already build GSO
  skbs larger than 64KB (MTU checks are skipped for GSO, and
  alloc_skb_with_frags() accepted up to MAX_SKB_FRAGS (17) order-0
  pages on top of the linear part).
- BIG TCP now makes skbs larger than 64KB common.
- The following patch makes tun_get_user() dissect IFF_TAP frames
  before eth_type_trans() pulls the Ethernet header, so a GSO frame
  carrying a 65522..65535 byte L3 packet will be dissected with
  skb->len in [65536, 65549].

Compare as u32 instead. If the resulting offset cannot be represented
in the u16 key_control->thoff, cap it to U16_MAX and report the
dissection as failed rather than silently returning a wrong transport
offset. thoff is still set on failure, as some callers (such as
eth_get_headlen()) use it regardless of the return value.

Fixes: d0c081b49137 ("flow_dissector: properly cap thoff field")
Assisted-by: LLM
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/core/flow_dissector.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 8aa4f9b4df81016a3d9a97e0d561f6dfc51aa88d..27d8a01bc92306ff043389b4fde2d24af97d3106 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1071,6 +1071,7 @@ bool __skb_flow_dissect(const struct net *net,
 	int mpls_lse = 0;
 	int num_hdrs = 0;
 	u8 ip_proto = 0;
+	u32 thoff;
 	bool ret;
 
 	if (!data) {
@@ -1692,7 +1693,13 @@ bool __skb_flow_dissect(const struct net *net,
 	ret = true;
 
 out:
-	key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
+	thoff = min_t(u32, nhoff, skb ? skb->len : hlen);
+	if (unlikely(thoff > U16_MAX)) {
+		/* Cannot be represented in key_control->thoff. */
+		thoff = U16_MAX;
+		ret = false;
+	}
+	key_control->thoff = thoff;
 	key_basic->n_proto = proto;
 	key_basic->ip_proto = ip_proto;
 
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH v3 net 1/3] flow_dissector: avoid u16 truncation of skb->len when computing thoff

[Willem de Bruijn]

Eric Dumazet wrote:
> __skb_flow_dissect() computes key_control->thoff using:
> 
>   key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
> 
> min_t(u16, ...) casts both arguments to u16 before comparing them.
> Whenever skb->len (or hlen) modulo 65536 is smaller than nhoff, the
> truncated length wins and thoff is set to a bogus small value, even
> when the dissection succeeded. For example, an IPv4/TCP frame with
> skb->len == 65540 and nhoff == 34 gets thoff == 4, so callers such as
> skb_probe_transport_header() point the transport header inside the
> Ethernet header.
> 
> Such skbs are not exotic:
> - At the time of commit d0c081b49137 ("flow_dissector: properly cap
>   thoff field"), AF_PACKET with PACKET_VNET_HDR could already build GSO
>   skbs larger than 64KB (MTU checks are skipped for GSO, and
>   alloc_skb_with_frags() accepted up to MAX_SKB_FRAGS (17) order-0
>   pages on top of the linear part).
> - BIG TCP now makes skbs larger than 64KB common.
> - The following patch makes tun_get_user() dissect IFF_TAP frames
>   before eth_type_trans() pulls the Ethernet header, so a GSO frame
>   carrying a 65522..65535 byte L3 packet will be dissected with
>   skb->len in [65536, 65549].
> 
> Compare as u32 instead. If the resulting offset cannot be represented
> in the u16 key_control->thoff, cap it to U16_MAX and report the
> dissection as failed rather than silently returning a wrong transport
> offset. thoff is still set on failure, as some callers (such as
> eth_get_headlen()) use it regardless of the return value.
> 
> Fixes: d0c081b49137 ("flow_dissector: properly cap thoff field")
> Assisted-by: LLM
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

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

Re: [PATCH v3 net 1/3] flow_dissector: avoid u16 truncation of skb->len when computing thoff

[netdev-bot+sashiko]

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

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

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

- [Medium] The fix is incomplete: __skb_flow_dissect() still has a second
  path that truncates a length to u16 when it computes thoff, the BPF…

--

flow_dissector: avoid u16 truncation of skb->len when computing thoff

This patch changes the thoff computation at the end of
__skb_flow_dissect() to compare in u32 instead of u16. If the result
does not fit in key_control->thoff, it is capped to U16_MAX and the
dissection is reported as failed.

> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 8aa4f9b4df810..27d8a01bc9230 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c

[ ... ]

> @@ -1692,7 +1693,13 @@ bool __skb_flow_dissect(const struct net *net,
>  	ret = true;
>  
>  out:
> -	key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
> +	thoff = min_t(u32, nhoff, skb ? skb->len : hlen);
> +	if (unlikely(thoff > U16_MAX)) {
> +		/* Cannot be represented in key_control->thoff. */
> +		thoff = U16_MAX;
> +		ret = false;
> +	}
> +	key_control->thoff = thoff;

[Severity: Medium]
Does the BPF flow dissector path in the same function still have the
same u16 truncation?

When a BPF flow dissector is attached to init_net or the netns,
__skb_flow_dissect() returns early and never reaches this out: label:

net/core/flow_dissector.c:__skb_flow_dissect() {
    ...
	result = bpf_flow_dissect(prog, &ctx, n_proto, nhoff,
				  hlen, flags);
	if (result != BPF_FLOW_DISSECTOR_CONTINUE) {
		__skb_flow_bpf_to_target(&flow_keys, flow_dissector,
					 target_container);
		rcu_read_unlock();
		return result == BPF_OK;
	}
    ...
}

bpf_flow_dissect() still clamps as u16, so the int hlen (skb_headlen())
is truncated before the comparison:

net/core/flow_dissector.c:bpf_flow_dissect() {
    ...
	flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, nhoff, hlen);
	flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
				   flow_keys->nhoff, hlen);
    ...
}

Then __skb_flow_bpf_to_target() copies the value as is:

	key_control->thoff = flow_keys->thoff;

Take a linear head of 65540 bytes and a program that reports nhoff=14,
thoff=34 and returns BPF_OK. (u16)hlen is 4, so both offsets collapse
to 4, and __skb_flow_dissect() returns true.

That seems to be the case the commit message describes:

  "Whenever skb->len (or hlen) modulo 65536 is smaller than nhoff, the
  truncated length wins and thoff is set to a bogus small value, even
  when the dissection succeeded."

and:

  "report the dissection as failed rather than silently returning a
  wrong transport offset."

skb_headlen() above 65535 looks reachable. packet_snd() skips the MTU
check when vnet_hdr.gso_type is set. Both packet_alloc_skb() and
tun_alloc_skb() grow the linear part when len exceeds the fragment
capacity:

	linear = len - MAX_SKB_FRAGS * (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER);

So a GSO write of roughly 622KB on 4K pages gives a linear head over
64KB. Attaching the BPF dissector does require privilege.

Callers that trust a successful return would then put the transport
header inside the L2/L3 header. Examples are skb_probe_transport_header()
calling skb_set_transport_header(skb, keys.control.thoff). Another is
__virtio_net_hdr_to_skb(), after "net: always dissect GSO packets in
__virtio_net_hdr_to_skb()" later in the series. The later patches in
the series leave bpf_flow_dissect() unchanged.

Should bpf_flow_dissect() also compare in u32, or cap hlen at U16_MAX,
and report failure when the offset cannot be represented?

>  	key_basic->n_proto = proto;
>  	key_basic->ip_proto = ip_proto;
>

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

Re: [PATCH v3 net 1/3] flow_dissector: avoid u16 truncation of skb->len when computing thoff

[Eric Dumazet]

On 10/5/26 17:30, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · 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 fix is incomplete: __skb_flow_dissect() still has a second
>    path that truncates a length to u16 when it computes thoff, the BPF…
> 
> --
> 
> flow_dissector: avoid u16 truncation of skb->len when computing thoff
> 
> This patch changes the thoff computation at the end of
> __skb_flow_dissect() to compare in u32 instead of u16. If the result
> does not fit in key_control->thoff, it is capped to U16_MAX and the
> dissection is reported as failed.
> 
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index 8aa4f9b4df810..27d8a01bc9230 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
> 
> [ ... ]
> 
>> @@ -1692,7 +1693,13 @@ bool __skb_flow_dissect(const struct net *net,
>>   	ret = true;
>>   
>>   out:
>> -	key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
>> +	thoff = min_t(u32, nhoff, skb ? skb->len : hlen);
>> +	if (unlikely(thoff > U16_MAX)) {
>> +		/* Cannot be represented in key_control->thoff. */
>> +		thoff = U16_MAX;
>> +		ret = false;
>> +	}
>> +	key_control->thoff = thoff;
> 
> [Severity: Medium]
> Does the BPF flow dissector path in the same function still have the
> same u16 truncation?


Seems legit, I will take care of this in a separate and stand alone 
patch next week (after LPC)

> 
> When a BPF flow dissector is attached to init_net or the netns,
> __skb_flow_dissect() returns early and never reaches this out: label:
> 
> net/core/flow_dissector.c:__skb_flow_dissect() {
>      ...
> 	result = bpf_flow_dissect(prog, &ctx, n_proto, nhoff,
> 				  hlen, flags);
> 	if (result != BPF_FLOW_DISSECTOR_CONTINUE) {
> 		__skb_flow_bpf_to_target(&flow_keys, flow_dissector,
> 					 target_container);
> 		rcu_read_unlock();
> 		return result == BPF_OK;
> 	}
>      ...
> }
> 
> bpf_flow_dissect() still clamps as u16, so the int hlen (skb_headlen())
> is truncated before the comparison:
> 
> net/core/flow_dissector.c:bpf_flow_dissect() {
>      ...
> 	flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, nhoff, hlen);
> 	flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
> 				   flow_keys->nhoff, hlen);
>      ...
> }
> 
> Then __skb_flow_bpf_to_target() copies the value as is:
> 
> 	key_control->thoff = flow_keys->thoff;
> 
> Take a linear head of 65540 bytes and a program that reports nhoff=14,
> thoff=34 and returns BPF_OK. (u16)hlen is 4, so both offsets collapse
> to 4, and __skb_flow_dissect() returns true.
> 
> That seems to be the case the commit message describes:
> 
>    "Whenever skb->len (or hlen) modulo 65536 is smaller than nhoff, the
>    truncated length wins and thoff is set to a bogus small value, even
>    when the dissection succeeded."
> 
> and:
> 
>    "report the dissection as failed rather than silently returning a
>    wrong transport offset."
> 
> skb_headlen() above 65535 looks reachable. packet_snd() skips the MTU
> check when vnet_hdr.gso_type is set. Both packet_alloc_skb() and
> tun_alloc_skb() grow the linear part when len exceeds the fragment
> capacity:
> 
> 	linear = len - MAX_SKB_FRAGS * (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER);
> 
> So a GSO write of roughly 622KB on 4K pages gives a linear head over
> 64KB. Attaching the BPF dissector does require privilege.
> 
> Callers that trust a successful return would then put the transport
> header inside the L2/L3 header. Examples are skb_probe_transport_header()
> calling skb_set_transport_header(skb, keys.control.thoff). Another is
> __virtio_net_hdr_to_skb(), after "net: always dissect GSO packets in
> __virtio_net_hdr_to_skb()" later in the series. The later patches in
> the series leave bpf_flow_dissect() unchanged.
> 
> Should bpf_flow_dissect() also compare in u32, or cap hlen at U16_MAX,
> and report failure when the offset cannot be represented?
> 
>>   	key_basic->n_proto = proto;
>>   	key_basic->ip_proto = ip_proto;
>>
> 

[PATCH v3 net 2/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Eric Dumazet]

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

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

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

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

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

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

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

Fixes: 9e8db5913264 ("net: avoid false positives in untrusted gso validation")
Fixes: d5be7f632bad ("net: validate untrusted gso packets without csum offload")
Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/
Assisted-by: LLM
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
---
 arch/um/drivers/vector_transports.c |  1 +
 drivers/net/tun.c                   | 23 ++++++-----
 drivers/net/virtio_net.c            |  2 +
 include/linux/virtio_net.h          | 60 ++++++++++++-----------------
 4 files changed, 42 insertions(+), 44 deletions(-)

diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
index ddd127ee96785daa4c485b2a06f078686efd2046..e1fb2a76fdff59ef3032d091be06a06fa58046cc 100644
--- a/arch/um/drivers/vector_transports.c
+++ b/arch/um/drivers/vector_transports.c
@@ -209,6 +209,7 @@ static int raw_verify_header(
 	if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
 		return 1;
 
+	skb_set_network_header(skb, ETH_HLEN);
 	virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
 	return 0;
 }
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5a302709a68aa3308b0c850b4a2957df3260b352..242899f7fd0711c5c59ff85accbf5bd1be9c6f39 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1897,12 +1897,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		}
 	}
 
-	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
-		atomic_long_inc(&tun->rx_frame_errors);
-		err = -EINVAL;
-		goto free_skb;
-	}
-
+	skb->dev = tun->dev;
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case IFF_TUN:
 		if (tun->flags & IFF_NO_PI) {
@@ -1927,9 +1922,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			}
 		}
 
-		skb_reset_mac_header(skb);
+		skb_reset_network_header(skb);
 		skb->protocol = pi.proto;
-		skb->dev = tun->dev;
 		break;
 	case IFF_TAP:
 		if (!pskb_may_pull(skb, ETH_HLEN)) {
@@ -1937,10 +1931,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			drop_reason = SKB_DROP_REASON_HDR_TRUNC;
 			goto drop;
 		}
-		skb->protocol = eth_type_trans(skb, tun->dev);
+		skb_set_network_header(skb, ETH_HLEN);
 		break;
 	}
 
+	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
+		atomic_long_inc(&tun->rx_frame_errors);
+		err = -EINVAL;
+		goto free_skb;
+	}
+
+	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
+		skb->protocol = eth_type_trans(skb, tun->dev);
+
 	/* copy skb_ubuf_info for callback when skb has no error */
 	if (zerocopy) {
 		skb_zcopy_init(skb, msg_control);
@@ -2600,6 +2603,8 @@ static int tun_xdp_one(struct tun_struct *tun,
 
 	features = tun_vnet_hdr_guest_features(READ_ONCE(tun->vnet_hdr_sz));
 	tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
+	skb->dev = tun->dev;
+	skb_set_network_header(skb, ETH_HLEN);
 	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
 		atomic_long_inc(&tun->rx_frame_errors);
 		kfree_skb(skb);
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index bf82ef9874abb4094931496fb6064a12ee75b789..daab43ac92ce4276b0f8f88684991e3993022ce6 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2515,6 +2515,8 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
 		goto frame_err;
 	}
 
+	skb->dev = dev;
+	skb_set_network_header(skb, ETH_HLEN);
 	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
 				      vi->rx_tnl_csum,
 				      virtio_is_little_endian(vi->vdev))) {
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index c381b916c1b54afacba5473888af589a24fe087c..d6466f96cdd00cdf059d4fa8842ba1672780e556 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -111,48 +111,38 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
 		p_off = nh_min_len + thlen;
 		if (!pskb_may_pull(skb, p_off))
 			return -EINVAL;
-	} else {
+	} else if (gso_type) {
 		/* gso packets without NEEDS_CSUM do not set transport_offset.
 		 * probe and drop if does not match one of the above types.
 		 */
-		if (gso_type && skb->network_header) {
-			struct flow_keys_basic keys;
-
-			if (!skb->protocol) {
-				__be16 protocol = dev_parse_header_protocol(skb);
-
-				if (!protocol)
-					virtio_net_hdr_set_proto(skb, hdr);
-				else if (!virtio_net_hdr_match_proto(protocol,
-								 hdr_gso_type))
-					return -EINVAL;
-				else
-					skb->protocol = protocol;
-			}
+		struct flow_keys_basic keys;
+
+		if (!skb->protocol) {
+			skb->protocol = dev_parse_header_protocol(skb);
+			if (!skb->protocol)
+				virtio_net_hdr_set_proto(skb, hdr);
+		}
 retry:
-			if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
-							      NULL, 0, 0, 0,
-							      0)) {
-				/* UFO does not specify ipv4 or 6: try both */
-				if (gso_type & SKB_GSO_UDP &&
-				    skb->protocol == htons(ETH_P_IP)) {
-					skb->protocol = htons(ETH_P_IPV6);
-					goto retry;
-				}
-				return -EINVAL;
+		if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
+						      NULL, 0, 0, 0,
+						      0)) {
+			/* UFO does not specify ipv4 or 6: try both */
+			if (gso_type & SKB_GSO_UDP &&
+			    skb->protocol == htons(ETH_P_IP)) {
+				skb->protocol = htons(ETH_P_IPV6);
+				goto retry;
 			}
+			return -EINVAL;
+		}
 
-			p_off = keys.control.thoff + thlen;
-			if (!pskb_may_pull(skb, p_off) ||
-			    keys.basic.ip_proto != ip_proto)
-				return -EINVAL;
+		p_off = keys.control.thoff + thlen;
+		if (!pskb_may_pull(skb, p_off) ||
+		    keys.basic.ip_proto != ip_proto ||
+		    !virtio_net_hdr_match_proto(keys.basic.n_proto,
+						hdr_gso_type))
+			return -EINVAL;
 
-			skb_set_transport_header(skb, keys.control.thoff);
-		} else if (gso_type) {
-			p_off = nh_min_len + thlen;
-			if (!pskb_may_pull(skb, p_off))
-				return -EINVAL;
-		}
+		skb_set_transport_header(skb, keys.control.thoff);
 	}
 
 	if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

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

[Eric Dumazet]

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

Also verify that the following are rejected with -EINVAL:
- a mismatched GSO type (VIRTIO_NET_HDR_GSO_TCPV6 on a VLAN-tagged IPv4
  packet).
- a frame whose TCP header is truncated after 10 bytes. Pulling only
  sizeof(struct iphdr) + sizeof(struct tcphdr) bytes would accept it,
  so this requires the transport offset found by flow dissection.

Finally, verify that a 65540-byte frame is accepted. Its skb->len is
above U16_MAX while it is flow-dissected, before eth_type_trans() pulls
the Ethernet header.

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

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

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

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

[Willem de Bruijn]

Eric Dumazet wrote:
> Add selftests in tun.c verifying that a VLAN-tagged (802.1Q) TCPv4 GSO
> packet without VIRTIO_NET_HDR_F_NEEDS_CSUM (both flags = 0 and
> flags = VIRTIO_NET_HDR_F_DATA_VALID) is accepted when written to a TAP
> device (/dev/net/tun with IFF_TAP | IFF_NO_PI | IFF_VNET_HDR).
> 
> Also verify that the following are rejected with -EINVAL:
> - a mismatched GSO type (VIRTIO_NET_HDR_GSO_TCPV6 on a VLAN-tagged IPv4
>   packet).
> - a frame whose TCP header is truncated after 10 bytes. Pulling only
>   sizeof(struct iphdr) + sizeof(struct tcphdr) bytes would accept it,
>   so this requires the transport offset found by flow dissection.
> 
> Finally, verify that a 65540-byte frame is accepted. Its skb->len is
> above U16_MAX while it is flow-dissected, before eth_type_trans() pulls
> the Ethernet header.
> 
> Based on a reproducer by Michael S. Tsirkin <mst@redhat.com>.
> 
> Assisted-by: LLM
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

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

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

iff respinning and not relevant for this fix, which targets the
!VIRTIO_NET_HDR_F_NEEDS_CSUM branch.

But would it be worthwhile to also run this test with
VIRTIO_NET_HDR_F_NEEDS_CSUM? Adds coverage of the other branch.

> +	vh.flags = 0;
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, len);
> +
> +	/* Valid VLAN-tagged TCPv4 GSO with flags = DATA_VALID */
> +	vh.flags = VIRTIO_NET_HDR_F_DATA_VALID;
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, len);
> +
> +	/* Mismatched GSO type (TCPV6 on VLAN-tagged IPv4 packet) */
> +	vh.flags = 0;
> +	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, len);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);
> +
> +	/* TCP header truncated after 10 bytes, without NEEDS_CSUM.
> +	 * Requires the transport header offset found by flow dissection:
> +	 * pulling only sizeof(struct iphdr) + sizeof(struct tcphdr) bytes
> +	 * from the mac header would accept this frame.
> +	 */
> +	vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> +	vh.hdr_len = ETH_HLEN + 4 + sizeof(struct iphdr) + 10;
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, sizeof(vh) + vh.hdr_len);
> +	ASSERT_EQ(ret, -1);
> +	ASSERT_EQ(errno, EINVAL);
> +}
> +
> +TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum_64k)
> +{
> +	/* Ethernet frame of 65540 bytes: skb->len is above U16_MAX when the
> +	 * frame is flow-dissected, before eth_type_trans() pulls ETH_HLEN.
> +	 */
> +	const int payload_len = 65540 - ETH_HLEN - 4 -
> +				sizeof(struct iphdr) - sizeof(struct tcphdr);
> +	struct virtio_net_hdr vh;
> +	uint8_t *pkt;
> +	int len, ret;
> +
> +	pkt = calloc(1, sizeof(vh) + 65540);
> +	ASSERT_NE(pkt, NULL);
> +
> +	len = build_vlan_tcpv4_gso_packet(pkt, payload_len);
> +	ASSERT_EQ(len, sizeof(vh) + 65540);
> +	memcpy(&vh, pkt, sizeof(vh));
> +
> +	vh.flags = 0;
> +	memcpy(pkt, &vh, sizeof(vh));
> +	ret = write(self->fd, pkt, len);
> +	free(pkt);
> +	ASSERT_EQ(ret, len);
> +}
> +
>  FIXTURE(tun_vnet_udptnl)
>  {
>  	char ifname[IFNAMSIZ];
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
> 

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Michael S. Tsirkin]

On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> This series fixes a bypass of untrusted GSO flow dissection in
> __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> 
> - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
>   with min_t(u16, ...). This truncates skb->len and returns a bogus small
>   transport offset when skb->len modulo 65536 is smaller than the
>   transport offset. Offsets that do not fit in the u16 thoff now fail the
>   dissection instead of being silently truncated.
> 
> - Patch 2 initializes skb->dev and skb->network_header before calling
>   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
>   virtnet_receive_done(), and raw_verify_header(), removes the
>   '&& skb->network_header' condition and the unvalidated
>   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
>   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
>   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
>   than the outer L2 protocol.
> 
> - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
>   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
>   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
>   TAP device, that mismatched GSO types and truncated TCP headers without
>   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
>   accepted.
> 
> v3:
>  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
>    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
>    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
>    frame carrying a maximal IPv4 packet (Sashiko).
>  - Patch 3: truncate the TCP header after 10 bytes so that the test
>    requires the transport offset found by flow dissection, and add a
>    65540-byte frame test (Sashiko).
>  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> 
> v2:
>  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
>    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
>    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
>  - Patch 2: clarify the changelog regarding why skb->network_header was 0
>    in those callers and why skb_reset_mac_header() is dropped in
>    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
>  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
>    Michael's reproducer.
>  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/


Not without trepidation about the amount of stuff we are shoving
into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
of trivial code in 2019:

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

> Eric Dumazet (3):
>   flow_dissector: avoid u16 truncation of skb->len when computing thoff
>   net: always dissect GSO packets in __virtio_net_hdr_to_skb()
>   selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM
> 
>  arch/um/drivers/vector_transports.c |   1 +
>  drivers/net/tun.c                   |  23 +++--
>  drivers/net/virtio_net.c            |   2 +
>  include/linux/virtio_net.h          |  60 +++++-------
>  net/core/flow_dissector.c           |   9 +-
>  tools/testing/selftests/net/tun.c   | 136 ++++++++++++++++++++++++++++
>  6 files changed, 186 insertions(+), 45 deletions(-)
> 
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Willem de Bruijn]

On Tue, Oct 6, 2026 at 6:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> > This series fixes a bypass of untrusted GSO flow dissection in
> > __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> > and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> >
> > - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
> >   with min_t(u16, ...). This truncates skb->len and returns a bogus small
> >   transport offset when skb->len modulo 65536 is smaller than the
> >   transport offset. Offsets that do not fit in the u16 thoff now fail the
> >   dissection instead of being silently truncated.
> >
> > - Patch 2 initializes skb->dev and skb->network_header before calling
> >   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
> >   virtnet_receive_done(), and raw_verify_header(), removes the
> >   '&& skb->network_header' condition and the unvalidated
> >   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
> >   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
> >   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
> >   than the outer L2 protocol.
> >
> > - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
> >   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
> >   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
> >   TAP device, that mismatched GSO types and truncated TCP headers without
> >   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
> >   accepted.
> >
> > v3:
> >  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
> >    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
> >    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
> >    frame carrying a maximal IPv4 packet (Sashiko).
> >  - Patch 3: truncate the TCP header after 10 bytes so that the test
> >    requires the transport offset found by flow dissection, and add a
> >    65540-byte frame test (Sashiko).
> >  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> >
> > v2:
> >  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
> >    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
> >    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
> >  - Patch 2: clarify the changelog regarding why skb->network_header was 0
> >    in those callers and why skb_reset_mac_header() is dropped in
> >    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
> >  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
> >    Michael's reproducer.
> >  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/
>
>
> Not without trepidation about the amount of stuff we are shoving
> into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
> of trivial code in 2019:

Unfortunately that let through many bad packets and unintentional
(ab)uses of the API.

The current state is the result of numerous fixes we had to apply
since then to protect the kernel. Generally there are two approaches:

1. make every reachable path in the kernel robust against unexpected
input. Frequently that means checks in the hot path that penalizes all
normal traffic, only to catch a bad actor or fuzzer. And it's not
straightforward to prove that all reachable paths are protected.
2. strict input validation.

With strict input validation from the start the checks could have been
simpler (hindsight is 20/20). Unfortunately, now we are stuck with
weird input (GSO without NEEDS_CSUM, skb protocol 0, encapsulation
headers, ..) that we now have to work around and try to not break,
that may or may not have real users.

This patch actually makes the function simpler. By reducing the
differences between the various callers of the function. This is great.

It sucks how complex this function has become, hopefully we can
find more such ways of making it simpler. The strict validation itself
is a good thing imho.

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Michael S. Tsirkin]

On Tue, Oct 06, 2026 at 07:26:46PM -0400, Willem de Bruijn wrote:
> On Tue, Oct 6, 2026 at 6:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> > > This series fixes a bypass of untrusted GSO flow dissection in
> > > __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> > > and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> > >
> > > - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
> > >   with min_t(u16, ...). This truncates skb->len and returns a bogus small
> > >   transport offset when skb->len modulo 65536 is smaller than the
> > >   transport offset. Offsets that do not fit in the u16 thoff now fail the
> > >   dissection instead of being silently truncated.
> > >
> > > - Patch 2 initializes skb->dev and skb->network_header before calling
> > >   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
> > >   virtnet_receive_done(), and raw_verify_header(), removes the
> > >   '&& skb->network_header' condition and the unvalidated
> > >   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
> > >   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
> > >   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
> > >   than the outer L2 protocol.
> > >
> > > - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
> > >   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
> > >   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
> > >   TAP device, that mismatched GSO types and truncated TCP headers without
> > >   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
> > >   accepted.
> > >
> > > v3:
> > >  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
> > >    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
> > >    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
> > >    frame carrying a maximal IPv4 packet (Sashiko).
> > >  - Patch 3: truncate the TCP header after 10 bytes so that the test
> > >    requires the transport offset found by flow dissection, and add a
> > >    65540-byte frame test (Sashiko).
> > >  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> > >
> > > v2:
> > >  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
> > >    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
> > >    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
> > >  - Patch 2: clarify the changelog regarding why skb->network_header was 0
> > >    in those callers and why skb_reset_mac_header() is dropped in
> > >    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
> > >  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
> > >    Michael's reproducer.
> > >  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/
> >
> >
> > Not without trepidation about the amount of stuff we are shoving
> > into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
> > of trivial code in 2019:
> 
> Unfortunately that let through many bad packets and unintentional
> (ab)uses of the API.
> 
> The current state is the result of numerous fixes we had to apply
> since then to protect the kernel. Generally there are two approaches:
> 
> 1. make every reachable path in the kernel robust against unexpected
> input. Frequently that means checks in the hot path that penalizes all
> normal traffic, only to catch a bad actor or fuzzer. And it's not
> straightforward to prove that all reachable paths are protected.
> 2. strict input validation.
> 
> With strict input validation from the start the checks could have been
> simpler (hindsight is 20/20). Unfortunately, now we are stuck with
> weird input (GSO without NEEDS_CSUM, skb protocol 0, encapsulation
> headers, ..) that we now have to work around and try to not break,
> that may or may not have real users.
> 
> This patch actually makes the function simpler. By reducing the
> differences between the various callers of the function. This is great.
> 
> It sucks how complex this function has become, hopefully we can
> find more such ways of making it simpler. The strict validation itself
> is a good thing imho.


Yes indeed. I have a vague idea how to do it: check some
performance-critical types of packets and for the rest just calculate
the checksum then and there.


-- 
MST

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Willem de Bruijn]

On Tue, Oct 6, 2026 at 7:50 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Oct 06, 2026 at 07:26:46PM -0400, Willem de Bruijn wrote:
> > On Tue, Oct 6, 2026 at 6:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> > > > This series fixes a bypass of untrusted GSO flow dissection in
> > > > __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> > > > and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> > > >
> > > > - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
> > > >   with min_t(u16, ...). This truncates skb->len and returns a bogus small
> > > >   transport offset when skb->len modulo 65536 is smaller than the
> > > >   transport offset. Offsets that do not fit in the u16 thoff now fail the
> > > >   dissection instead of being silently truncated.
> > > >
> > > > - Patch 2 initializes skb->dev and skb->network_header before calling
> > > >   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
> > > >   virtnet_receive_done(), and raw_verify_header(), removes the
> > > >   '&& skb->network_header' condition and the unvalidated
> > > >   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
> > > >   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
> > > >   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
> > > >   than the outer L2 protocol.
> > > >
> > > > - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
> > > >   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
> > > >   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
> > > >   TAP device, that mismatched GSO types and truncated TCP headers without
> > > >   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
> > > >   accepted.
> > > >
> > > > v3:
> > > >  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
> > > >    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
> > > >    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
> > > >    frame carrying a maximal IPv4 packet (Sashiko).
> > > >  - Patch 3: truncate the TCP header after 10 bytes so that the test
> > > >    requires the transport offset found by flow dissection, and add a
> > > >    65540-byte frame test (Sashiko).
> > > >  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> > > >
> > > > v2:
> > > >  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
> > > >    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
> > > >    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
> > > >  - Patch 2: clarify the changelog regarding why skb->network_header was 0
> > > >    in those callers and why skb_reset_mac_header() is dropped in
> > > >    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
> > > >  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
> > > >    Michael's reproducer.
> > > >  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/
> > >
> > >
> > > Not without trepidation about the amount of stuff we are shoving
> > > into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
> > > of trivial code in 2019:
> >
> > Unfortunately that let through many bad packets and unintentional
> > (ab)uses of the API.
> >
> > The current state is the result of numerous fixes we had to apply
> > since then to protect the kernel. Generally there are two approaches:
> >
> > 1. make every reachable path in the kernel robust against unexpected
> > input. Frequently that means checks in the hot path that penalizes all
> > normal traffic, only to catch a bad actor or fuzzer. And it's not
> > straightforward to prove that all reachable paths are protected.
> > 2. strict input validation.
> >
> > With strict input validation from the start the checks could have been
> > simpler (hindsight is 20/20). Unfortunately, now we are stuck with
> > weird input (GSO without NEEDS_CSUM, skb protocol 0, encapsulation
> > headers, ..) that we now have to work around and try to not break,
> > that may or may not have real users.
> >
> > This patch actually makes the function simpler. By reducing the
> > differences between the various callers of the function. This is great.
> >
> > It sucks how complex this function has become, hopefully we can
> > find more such ways of making it simpler. The strict validation itself
> > is a good thing imho.
>
>
> Yes indeed. I have a vague idea how to do it: check some
> performance-critical types of packets and for the rest just calculate
> the checksum then and there.

That sounds promising.

Checksumming is only one of the risks. Segmentation is another.

Perhaps the general idea can be extended: harden the kernel to the
small set of well known types, strict validation and even processing
for the long tail of other traffic.

I'd even (optionally, maybe behind a sysctl/static-branch) run
flow_dissection to ensure the packets are what they claim. When
only unencapsulated TCP/IP is expected, this is cheap enough. And
it avoids the manual sort-of parser code that we have now.

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Michael S. Tsirkin]

On Tue, Oct 06, 2026 at 08:13:43PM -0400, Willem de Bruijn wrote:
> On Tue, Oct 6, 2026 at 7:50 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Oct 06, 2026 at 07:26:46PM -0400, Willem de Bruijn wrote:
> > > On Tue, Oct 6, 2026 at 6:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> > > > > This series fixes a bypass of untrusted GSO flow dissection in
> > > > > __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> > > > > and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> > > > >
> > > > > - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
> > > > >   with min_t(u16, ...). This truncates skb->len and returns a bogus small
> > > > >   transport offset when skb->len modulo 65536 is smaller than the
> > > > >   transport offset. Offsets that do not fit in the u16 thoff now fail the
> > > > >   dissection instead of being silently truncated.
> > > > >
> > > > > - Patch 2 initializes skb->dev and skb->network_header before calling
> > > > >   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
> > > > >   virtnet_receive_done(), and raw_verify_header(), removes the
> > > > >   '&& skb->network_header' condition and the unvalidated
> > > > >   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
> > > > >   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
> > > > >   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
> > > > >   than the outer L2 protocol.
> > > > >
> > > > > - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
> > > > >   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
> > > > >   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
> > > > >   TAP device, that mismatched GSO types and truncated TCP headers without
> > > > >   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
> > > > >   accepted.
> > > > >
> > > > > v3:
> > > > >  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
> > > > >    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
> > > > >    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
> > > > >    frame carrying a maximal IPv4 packet (Sashiko).
> > > > >  - Patch 3: truncate the TCP header after 10 bytes so that the test
> > > > >    requires the transport offset found by flow dissection, and add a
> > > > >    65540-byte frame test (Sashiko).
> > > > >  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> > > > >
> > > > > v2:
> > > > >  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
> > > > >    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
> > > > >    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
> > > > >  - Patch 2: clarify the changelog regarding why skb->network_header was 0
> > > > >    in those callers and why skb_reset_mac_header() is dropped in
> > > > >    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
> > > > >  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
> > > > >    Michael's reproducer.
> > > > >  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/
> > > >
> > > >
> > > > Not without trepidation about the amount of stuff we are shoving
> > > > into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
> > > > of trivial code in 2019:
> > >
> > > Unfortunately that let through many bad packets and unintentional
> > > (ab)uses of the API.
> > >
> > > The current state is the result of numerous fixes we had to apply
> > > since then to protect the kernel. Generally there are two approaches:
> > >
> > > 1. make every reachable path in the kernel robust against unexpected
> > > input. Frequently that means checks in the hot path that penalizes all
> > > normal traffic, only to catch a bad actor or fuzzer. And it's not
> > > straightforward to prove that all reachable paths are protected.
> > > 2. strict input validation.
> > >
> > > With strict input validation from the start the checks could have been
> > > simpler (hindsight is 20/20). Unfortunately, now we are stuck with
> > > weird input (GSO without NEEDS_CSUM, skb protocol 0, encapsulation
> > > headers, ..) that we now have to work around and try to not break,
> > > that may or may not have real users.
> > >
> > > This patch actually makes the function simpler. By reducing the
> > > differences between the various callers of the function. This is great.
> > >
> > > It sucks how complex this function has become, hopefully we can
> > > find more such ways of making it simpler. The strict validation itself
> > > is a good thing imho.
> >
> >
> > Yes indeed. I have a vague idea how to do it: check some
> > performance-critical types of packets and for the rest just calculate
> > the checksum then and there.
> 
> That sounds promising.
> 
> Checksumming is only one of the risks. Segmentation is another.

Same approach for segmentation would be great but how do we know how to
segment at input?

> Perhaps the general idea can be extended: harden the kernel to the
> small set of well known types, strict validation and even processing
> for the long tail of other traffic.
> 
> I'd even (optionally, maybe behind a sysctl/static-branch) run
> flow_dissection to ensure the packets are what they claim. When
> only unencapsulated TCP/IP is expected, this is cheap enough. And
> it avoids the manual sort-of parser code that we have now.

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Willem de Bruijn]

On Tue, Oct 6, 2026 at 8:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Oct 06, 2026 at 08:13:43PM -0400, Willem de Bruijn wrote:
> > On Tue, Oct 6, 2026 at 7:50 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Tue, Oct 06, 2026 at 07:26:46PM -0400, Willem de Bruijn wrote:
> > > > On Tue, Oct 6, 2026 at 6:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > >
> > > > > On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> > > > > > This series fixes a bypass of untrusted GSO flow dissection in
> > > > > > __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> > > > > > and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> > > > > >
> > > > > > - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
> > > > > >   with min_t(u16, ...). This truncates skb->len and returns a bogus small
> > > > > >   transport offset when skb->len modulo 65536 is smaller than the
> > > > > >   transport offset. Offsets that do not fit in the u16 thoff now fail the
> > > > > >   dissection instead of being silently truncated.
> > > > > >
> > > > > > - Patch 2 initializes skb->dev and skb->network_header before calling
> > > > > >   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
> > > > > >   virtnet_receive_done(), and raw_verify_header(), removes the
> > > > > >   '&& skb->network_header' condition and the unvalidated
> > > > > >   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
> > > > > >   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
> > > > > >   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
> > > > > >   than the outer L2 protocol.
> > > > > >
> > > > > > - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
> > > > > >   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
> > > > > >   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
> > > > > >   TAP device, that mismatched GSO types and truncated TCP headers without
> > > > > >   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
> > > > > >   accepted.
> > > > > >
> > > > > > v3:
> > > > > >  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
> > > > > >    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
> > > > > >    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
> > > > > >    frame carrying a maximal IPv4 packet (Sashiko).
> > > > > >  - Patch 3: truncate the TCP header after 10 bytes so that the test
> > > > > >    requires the transport offset found by flow dissection, and add a
> > > > > >    65540-byte frame test (Sashiko).
> > > > > >  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> > > > > >
> > > > > > v2:
> > > > > >  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
> > > > > >    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
> > > > > >    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
> > > > > >  - Patch 2: clarify the changelog regarding why skb->network_header was 0
> > > > > >    in those callers and why skb_reset_mac_header() is dropped in
> > > > > >    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
> > > > > >  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
> > > > > >    Michael's reproducer.
> > > > > >  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/
> > > > >
> > > > >
> > > > > Not without trepidation about the amount of stuff we are shoving
> > > > > into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
> > > > > of trivial code in 2019:
> > > >
> > > > Unfortunately that let through many bad packets and unintentional
> > > > (ab)uses of the API.
> > > >
> > > > The current state is the result of numerous fixes we had to apply
> > > > since then to protect the kernel. Generally there are two approaches:
> > > >
> > > > 1. make every reachable path in the kernel robust against unexpected
> > > > input. Frequently that means checks in the hot path that penalizes all
> > > > normal traffic, only to catch a bad actor or fuzzer. And it's not
> > > > straightforward to prove that all reachable paths are protected.
> > > > 2. strict input validation.
> > > >
> > > > With strict input validation from the start the checks could have been
> > > > simpler (hindsight is 20/20). Unfortunately, now we are stuck with
> > > > weird input (GSO without NEEDS_CSUM, skb protocol 0, encapsulation
> > > > headers, ..) that we now have to work around and try to not break,
> > > > that may or may not have real users.
> > > >
> > > > This patch actually makes the function simpler. By reducing the
> > > > differences between the various callers of the function. This is great.
> > > >
> > > > It sucks how complex this function has become, hopefully we can
> > > > find more such ways of making it simpler. The strict validation itself
> > > > is a good thing imho.
> > >
> > >
> > > Yes indeed. I have a vague idea how to do it: check some
> > > performance-critical types of packets and for the rest just calculate
> > > the checksum then and there.
> >
> > That sounds promising.
> >
> > Checksumming is only one of the risks. Segmentation is another.
>
> Same approach for segmentation would be great but how do we know how to
> segment at input?

gso_type?

The main issue I see is that we do not have a way to then process the
chain of segs. So maybe we cannot realistically do it at this stage.

Come to think of it, I previously considered this and ended up with
commit 121d57af308d ("gso: validate gso_type in GSO handlers")
instead.

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Michael S. Tsirkin]

On Tue, Oct 06, 2026 at 08:30:50PM -0400, Willem de Bruijn wrote:
> On Tue, Oct 6, 2026 at 8:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Oct 06, 2026 at 08:13:43PM -0400, Willem de Bruijn wrote:
> > > On Tue, Oct 6, 2026 at 7:50 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Tue, Oct 06, 2026 at 07:26:46PM -0400, Willem de Bruijn wrote:
> > > > > On Tue, Oct 6, 2026 at 6:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > >
> > > > > > On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> > > > > > > This series fixes a bypass of untrusted GSO flow dissection in
> > > > > > > __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> > > > > > > and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> > > > > > >
> > > > > > > - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
> > > > > > >   with min_t(u16, ...). This truncates skb->len and returns a bogus small
> > > > > > >   transport offset when skb->len modulo 65536 is smaller than the
> > > > > > >   transport offset. Offsets that do not fit in the u16 thoff now fail the
> > > > > > >   dissection instead of being silently truncated.
> > > > > > >
> > > > > > > - Patch 2 initializes skb->dev and skb->network_header before calling
> > > > > > >   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
> > > > > > >   virtnet_receive_done(), and raw_verify_header(), removes the
> > > > > > >   '&& skb->network_header' condition and the unvalidated
> > > > > > >   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
> > > > > > >   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
> > > > > > >   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
> > > > > > >   than the outer L2 protocol.
> > > > > > >
> > > > > > > - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
> > > > > > >   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
> > > > > > >   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
> > > > > > >   TAP device, that mismatched GSO types and truncated TCP headers without
> > > > > > >   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
> > > > > > >   accepted.
> > > > > > >
> > > > > > > v3:
> > > > > > >  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
> > > > > > >    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
> > > > > > >    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
> > > > > > >    frame carrying a maximal IPv4 packet (Sashiko).
> > > > > > >  - Patch 3: truncate the TCP header after 10 bytes so that the test
> > > > > > >    requires the transport offset found by flow dissection, and add a
> > > > > > >    65540-byte frame test (Sashiko).
> > > > > > >  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> > > > > > >
> > > > > > > v2:
> > > > > > >  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
> > > > > > >    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
> > > > > > >    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
> > > > > > >  - Patch 2: clarify the changelog regarding why skb->network_header was 0
> > > > > > >    in those callers and why skb_reset_mac_header() is dropped in
> > > > > > >    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
> > > > > > >  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
> > > > > > >    Michael's reproducer.
> > > > > > >  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/
> > > > > >
> > > > > >
> > > > > > Not without trepidation about the amount of stuff we are shoving
> > > > > > into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
> > > > > > of trivial code in 2019:
> > > > >
> > > > > Unfortunately that let through many bad packets and unintentional
> > > > > (ab)uses of the API.
> > > > >
> > > > > The current state is the result of numerous fixes we had to apply
> > > > > since then to protect the kernel. Generally there are two approaches:
> > > > >
> > > > > 1. make every reachable path in the kernel robust against unexpected
> > > > > input. Frequently that means checks in the hot path that penalizes all
> > > > > normal traffic, only to catch a bad actor or fuzzer. And it's not
> > > > > straightforward to prove that all reachable paths are protected.
> > > > > 2. strict input validation.
> > > > >
> > > > > With strict input validation from the start the checks could have been
> > > > > simpler (hindsight is 20/20). Unfortunately, now we are stuck with
> > > > > weird input (GSO without NEEDS_CSUM, skb protocol 0, encapsulation
> > > > > headers, ..) that we now have to work around and try to not break,
> > > > > that may or may not have real users.
> > > > >
> > > > > This patch actually makes the function simpler. By reducing the
> > > > > differences between the various callers of the function. This is great.
> > > > >
> > > > > It sucks how complex this function has become, hopefully we can
> > > > > find more such ways of making it simpler. The strict validation itself
> > > > > is a good thing imho.
> > > >
> > > >
> > > > Yes indeed. I have a vague idea how to do it: check some
> > > > performance-critical types of packets and for the rest just calculate
> > > > the checksum then and there.
> > >
> > > That sounds promising.
> > >
> > > Checksumming is only one of the risks. Segmentation is another.
> >
> > Same approach for segmentation would be great but how do we know how to
> > segment at input?
> 
> gso_type?
> 
> The main issue I see is that we do not have a way to then process the
> chain of segs. So maybe we cannot realistically do it at this stage.

I mean, there are what, 8 callers of this? That's much less of
a brain surgery than carefully calculating offsets within packets
as far as I am concerned.

> Come to think of it, I previously considered this and ended up with
> commit 121d57af308d ("gso: validate gso_type in GSO handlers")
> instead.

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[Willem de Bruijn]

On Wed, Oct 7, 2026 at 4:04 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Oct 06, 2026 at 08:30:50PM -0400, Willem de Bruijn wrote:
> > On Tue, Oct 6, 2026 at 8:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Tue, Oct 06, 2026 at 08:13:43PM -0400, Willem de Bruijn wrote:
> > > > On Tue, Oct 6, 2026 at 7:50 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > >
> > > > > On Tue, Oct 06, 2026 at 07:26:46PM -0400, Willem de Bruijn wrote:
> > > > > > On Tue, Oct 6, 2026 at 6:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > >
> > > > > > > On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> > > > > > > > This series fixes a bypass of untrusted GSO flow dissection in
> > > > > > > > __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> > > > > > > > and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> > > > > > > >
> > > > > > > > - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
> > > > > > > >   with min_t(u16, ...). This truncates skb->len and returns a bogus small
> > > > > > > >   transport offset when skb->len modulo 65536 is smaller than the
> > > > > > > >   transport offset. Offsets that do not fit in the u16 thoff now fail the
> > > > > > > >   dissection instead of being silently truncated.
> > > > > > > >
> > > > > > > > - Patch 2 initializes skb->dev and skb->network_header before calling
> > > > > > > >   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
> > > > > > > >   virtnet_receive_done(), and raw_verify_header(), removes the
> > > > > > > >   '&& skb->network_header' condition and the unvalidated
> > > > > > > >   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
> > > > > > > >   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
> > > > > > > >   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
> > > > > > > >   than the outer L2 protocol.
> > > > > > > >
> > > > > > > > - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
> > > > > > > >   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
> > > > > > > >   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
> > > > > > > >   TAP device, that mismatched GSO types and truncated TCP headers without
> > > > > > > >   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
> > > > > > > >   accepted.
> > > > > > > >
> > > > > > > > v3:
> > > > > > > >  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
> > > > > > > >    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
> > > > > > > >    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
> > > > > > > >    frame carrying a maximal IPv4 packet (Sashiko).
> > > > > > > >  - Patch 3: truncate the TCP header after 10 bytes so that the test
> > > > > > > >    requires the transport offset found by flow dissection, and add a
> > > > > > > >    65540-byte frame test (Sashiko).
> > > > > > > >  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> > > > > > > >
> > > > > > > > v2:
> > > > > > > >  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
> > > > > > > >    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
> > > > > > > >    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
> > > > > > > >  - Patch 2: clarify the changelog regarding why skb->network_header was 0
> > > > > > > >    in those callers and why skb_reset_mac_header() is dropped in
> > > > > > > >    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
> > > > > > > >  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
> > > > > > > >    Michael's reproducer.
> > > > > > > >  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/
> > > > > > >
> > > > > > >
> > > > > > > Not without trepidation about the amount of stuff we are shoving
> > > > > > > into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
> > > > > > > of trivial code in 2019:
> > > > > >
> > > > > > Unfortunately that let through many bad packets and unintentional
> > > > > > (ab)uses of the API.
> > > > > >
> > > > > > The current state is the result of numerous fixes we had to apply
> > > > > > since then to protect the kernel. Generally there are two approaches:
> > > > > >
> > > > > > 1. make every reachable path in the kernel robust against unexpected
> > > > > > input. Frequently that means checks in the hot path that penalizes all
> > > > > > normal traffic, only to catch a bad actor or fuzzer. And it's not
> > > > > > straightforward to prove that all reachable paths are protected.
> > > > > > 2. strict input validation.
> > > > > >
> > > > > > With strict input validation from the start the checks could have been
> > > > > > simpler (hindsight is 20/20). Unfortunately, now we are stuck with
> > > > > > weird input (GSO without NEEDS_CSUM, skb protocol 0, encapsulation
> > > > > > headers, ..) that we now have to work around and try to not break,
> > > > > > that may or may not have real users.
> > > > > >
> > > > > > This patch actually makes the function simpler. By reducing the
> > > > > > differences between the various callers of the function. This is great.
> > > > > >
> > > > > > It sucks how complex this function has become, hopefully we can
> > > > > > find more such ways of making it simpler. The strict validation itself
> > > > > > is a good thing imho.
> > > > >
> > > > >
> > > > > Yes indeed. I have a vague idea how to do it: check some
> > > > > performance-critical types of packets and for the rest just calculate
> > > > > the checksum then and there.
> > > >
> > > > That sounds promising.
> > > >
> > > > Checksumming is only one of the risks. Segmentation is another.
> > >
> > > Same approach for segmentation would be great but how do we know how to
> > > segment at input?
> >
> > gso_type?
> >
> > The main issue I see is that we do not have a way to then process the
> > chain of segs. So maybe we cannot realistically do it at this stage.
>
> I mean, there are what, 8 callers of this? That's much less of
> a brain surgery than carefully calculating offsets within packets
> as far as I am concerned.

We can definitely take a stab.

Another option is to convert from an untrusted GSO packet
(SKB_GSO_DODGY) into a GSO packet that has been validated
through the software segmentation check. The

```
        if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
                /* Packet is from an untrusted source, reset gso_segs. */

                skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);

                segs = NULL;
                goto out;
        }
```

Path in tcp_gso_segment (and similar for USO). Packets still only
go through software segmentation once, which all these untrusted
packets do (__virtio_net_hdr_to_skb always sets SKB_GSO_DODGY).

A variant of that is to convert from untrusted GSO to trusted chain
of segments that are very cheap to segment later, SKB_GSO_FRAGLIST.

But just changing the callers to loop over segs[] can certainly be
done too.

Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  1 Oct 2026 19:11:37 +0000 you wrote:
> This series fixes a bypass of untrusted GSO flow dissection in
> __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> 
> - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
>   with min_t(u16, ...). This truncates skb->len and returns a bogus small
>   transport offset when skb->len modulo 65536 is smaller than the
>   transport offset. Offsets that do not fit in the u16 thoff now fail the
>   dissection instead of being silently truncated.
> 
> [...]

Here is the summary with links:
  - [v3,net,1/3] flow_dissector: avoid u16 truncation of skb->len when computing thoff
    https://git.kernel.org/netdev/net/c/99bda1ecbd56
  - [v3,net,2/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()
    https://git.kernel.org/netdev/net/c/44378d02c5aa
  - [v3,net,3/3] selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM
    https://git.kernel.org/netdev/net/c/ee2d4c2d7cc1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html