mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v2] net: gso: validate transport header before computing segment length
@ 2026-10-19 11:58 Michael S. Tsirkin
  2026-10-20 19:10 ` Willem de Bruijn
  2026-10-22  2:59 ` netdev-bot+sashiko
  0 siblings, 2 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2026-10-19 11:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Max Krasnyansky, netdev

If a TSO packet is processed in the stack before a transport
header was ever established, the TCP data offset is read through the
unset header offset, far outside the packet.

This was demonstrated by syzkaller, back when the input path did not
dissect such packets: a virtio net header supplying a malformed
network header was enough.  Qdisc length estimation has carried a
guard against exactly this since a0dce8752193 ("net: Skip GSO
length estimation if transport header is not set"); the GSO length
validators never got one.

We used to, but we no longer allow such packets in: since 44378d02c5aa
("net: always dissect GSO packets in __virtio_net_hdr_to_skb()") the
virtio net input path dissects all GSO packets.

But as a defence in depth measure, let's also check before we read: if
the TCP header is unset or outside the linear data, drop it.

Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
Cc: Willem de Bruijn <willemb@google.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
changes from v1:
- address Willem's comments, in particular, teach skb_gso_network_seglen
  to return a negative error, and propagate through callers.

 include/linux/skbuff.h |  5 +++
 net/core/gso.c         | 82 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a3ff380d43f0..aa5bc608de06 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3041,6 +3041,11 @@ static inline bool skb_inner_network_header_was_set(const struct sk_buff *skb)
 	return skb->inner_network_header > 0;
 }
 
+static inline bool skb_inner_transport_header_was_set(const struct sk_buff *skb)
+{
+	return skb->inner_transport_header > 0;
+}
+
 static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
 {
 	return skb->head + skb->inner_mac_header;
diff --git a/net/core/gso.c b/net/core/gso.c
index e96ef6350064..874e4c8b1f1b 100644
--- a/net/core/gso.c
+++ b/net/core/gso.c
@@ -131,6 +131,35 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
 }
 EXPORT_SYMBOL(__skb_gso_segment);
 
+/*
+ * TSO processing (but not UDP/SCTP GSO, which uses fixed header
+ * sizes) reads the TCP header as specified by
+ * skb->transport_header/inner_transport_header.  Make sure it is
+ * there to read.
+ */
+static bool skb_gso_transport_header_ok(const struct sk_buff *skb)
+{
+	const struct skb_shared_info *shinfo = skb_shinfo(skb);
+	int thoff;
+
+	if (unlikely(!(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))))
+		return true;
+
+	if (skb->encapsulation) {
+		if (!skb_inner_transport_header_was_set(skb))
+			return false;
+
+		thoff = skb_inner_transport_offset(skb);
+	} else {
+		if (!skb_transport_header_was_set(skb))
+			return false;
+
+		thoff = skb_transport_offset(skb);
+	}
+
+	return thoff >= 0 && thoff + sizeof(struct tcphdr) <= skb_headlen(skb);
+}
+
 /**
  * skb_gso_transport_seglen - Return length of individual segments of a gso packet
  *
@@ -140,12 +169,18 @@ EXPORT_SYMBOL(__skb_gso_segment);
  * individual segments, including Layer4 headers (TCP/UDP).
  *
  * The MAC/L2 or network (IP, IPv6) headers are not accounted for.
+ *
+ * Return: the length, or a negative errno if the TCP header is not
+ * there to read.
  */
-static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
+static int skb_gso_transport_seglen(const struct sk_buff *skb)
 {
 	const struct skb_shared_info *shinfo = skb_shinfo(skb);
 	unsigned int thlen = 0;
 
+	if (unlikely(!skb_gso_transport_header_ok(skb)))
+		return -EINVAL;
+
 	if (skb->encapsulation) {
 		thlen = skb_inner_transport_header(skb) -
 			skb_transport_header(skb);
@@ -175,13 +210,21 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
  * individual segments, including Layer3 (IP, IPv6) and L4 headers (TCP/UDP).
  *
  * The MAC/L2 header is not accounted for.
+ *
+ * Return: the length, or a negative errno if the TCP header is not
+ * there to read.
  */
-static unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
+static int skb_gso_network_seglen(const struct sk_buff *skb)
 {
-	unsigned int hdr_len = skb_transport_header(skb) -
-			       skb_network_header(skb);
+	unsigned int hdr_len;
+	int tlen = skb_gso_transport_seglen(skb);
 
-	return hdr_len + skb_gso_transport_seglen(skb);
+	if (tlen < 0)
+		return tlen;
+
+	hdr_len = skb_transport_header(skb) - skb_network_header(skb);
+
+	return hdr_len + tlen;
 }
 
 /**
@@ -192,12 +235,21 @@ static unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
  * skb_gso_mac_seglen is used to determine the real size of the
  * individual segments, including MAC/L2, Layer3 (IP, IPv6) and L4
  * headers (TCP/UDP).
+ *
+ * Return: the length, or a negative errno if the TCP header is not
+ * there to read.
  */
-static unsigned int skb_gso_mac_seglen(const struct sk_buff *skb)
+static int skb_gso_mac_seglen(const struct sk_buff *skb)
 {
-	unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
+	unsigned int hdr_len;
+	int tlen = skb_gso_transport_seglen(skb);
 
-	return hdr_len + skb_gso_transport_seglen(skb);
+	if (tlen < 0)
+		return tlen;
+
+	hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
+
+	return hdr_len + tlen;
 }
 
 /**
@@ -253,7 +305,12 @@ static inline bool skb_gso_size_check(const struct sk_buff *skb,
  */
 bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu)
 {
-	return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
+	int seg_len = skb_gso_network_seglen(skb);
+
+	if (seg_len < 0)
+		return false;
+
+	return skb_gso_size_check(skb, seg_len, mtu);
 }
 EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
 
@@ -268,7 +325,12 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
  */
 bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
 {
-	return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
+	int seg_len = skb_gso_mac_seglen(skb);
+
+	if (seg_len < 0)
+		return false;
+
+	return skb_gso_size_check(skb, seg_len, len);
 }
 EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
 
-- 
MST


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] net: gso: validate transport header before computing segment length
  2026-10-19 11:58 [PATCH net v2] net: gso: validate transport header before computing segment length Michael S. Tsirkin
@ 2026-10-20 19:10 ` Willem de Bruijn
  2026-10-22  2:59 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: Willem de Bruijn @ 2026-10-20 19:10 UTC (permalink / raw)
  To: Michael S. Tsirkin, linux-kernel
  Cc: Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Max Krasnyansky, netdev

Michael S. Tsirkin wrote:
> If a TSO packet is processed in the stack before a transport
> header was ever established, the TCP data offset is read through the
> unset header offset, far outside the packet.
> 
> This was demonstrated by syzkaller, back when the input path did not
> dissect such packets: a virtio net header supplying a malformed
> network header was enough.  Qdisc length estimation has carried a
> guard against exactly this since a0dce8752193 ("net: Skip GSO

please insert commit here and before 44378d02c5aa. I'm surprised
checkpatch did not complain.

> length estimation if transport header is not set"); the GSO length
> validators never got one.
> 
> We used to, but we no longer allow such packets in: since 44378d02c5aa
> ("net: always dissect GSO packets in __virtio_net_hdr_to_skb()") the
> virtio net input path dissects all GSO packets.
> 
> But as a defence in depth measure, let's also check before we read: if
> the TCP header is unset or outside the linear data, drop it.

is the packet indeed dropped?

> 
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> Cc: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> changes from v1:
> - address Willem's comments, in particular, teach skb_gso_network_seglen
>   to return a negative error, and propagate through callers.
> 
>  include/linux/skbuff.h |  5 +++
>  net/core/gso.c         | 82 ++++++++++++++++++++++++++++++++++++------
>  2 files changed, 77 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index a3ff380d43f0..aa5bc608de06 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3041,6 +3041,11 @@ static inline bool skb_inner_network_header_was_set(const struct sk_buff *skb)
>  	return skb->inner_network_header > 0;
>  }
>  
> +static inline bool skb_inner_transport_header_was_set(const struct sk_buff *skb)
> +{
> +	return skb->inner_transport_header > 0;
> +}

It may be possible that skb_reset_inner_headers gets called when
skb->transport_header is ~0U. For safety, also check ~0U here?

>  static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
>  {
>  	return skb->head + skb->inner_mac_header;
> diff --git a/net/core/gso.c b/net/core/gso.c
> index e96ef6350064..874e4c8b1f1b 100644
> --- a/net/core/gso.c
> +++ b/net/core/gso.c
> @@ -131,6 +131,35 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>  }
>  EXPORT_SYMBOL(__skb_gso_segment);
>  
> +/*
> + * TSO processing (but not UDP/SCTP GSO, which uses fixed header
> + * sizes) reads the TCP header as specified by
> + * skb->transport_header/inner_transport_header.  Make sure it is
> + * there to read.
> + */
> +static bool skb_gso_transport_header_ok(const struct sk_buff *skb)
> +{
> +	const struct skb_shared_info *shinfo = skb_shinfo(skb);
> +	int thoff;
> +
> +	if (unlikely(!(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))))
> +		return true;

maybe skb_tso_transport_header_ok given this limitation
> +
> +	if (skb->encapsulation) {
> +		if (!skb_inner_transport_header_was_set(skb))
> +			return false;
> +
> +		thoff = skb_inner_transport_offset(skb);

skb_gso_transport_seglen reads both transport_offset and
inner_transport_offset, so if skb->encapsulation we probably should
not check only inner, but both?

> +	} else {
> +		if (!skb_transport_header_was_set(skb))
> +			return false;
> +
> +		thoff = skb_transport_offset(skb);
> +	}
> +
> +	return thoff >= 0 && thoff + sizeof(struct tcphdr) <= skb_headlen(skb);
> +}
> +
>  /**
>   * skb_gso_transport_seglen - Return length of individual segments of a gso packet
>   *
> @@ -140,12 +169,18 @@ EXPORT_SYMBOL(__skb_gso_segment);
>   * individual segments, including Layer4 headers (TCP/UDP).
>   *
>   * The MAC/L2 or network (IP, IPv6) headers are not accounted for.
> + *
> + * Return: the length, or a negative errno if the TCP header is not
> + * there to read.
>   */
> -static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
> +static int skb_gso_transport_seglen(const struct sk_buff *skb)
>  {
>  	const struct skb_shared_info *shinfo = skb_shinfo(skb);
>  	unsigned int thlen = 0;
>  
> +	if (unlikely(!skb_gso_transport_header_ok(skb)))
> +		return -EINVAL;
> +
>  	if (skb->encapsulation) {
>  		thlen = skb_inner_transport_header(skb) -
>  			skb_transport_header(skb);
> @@ -175,13 +210,21 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
>   * individual segments, including Layer3 (IP, IPv6) and L4 headers (TCP/UDP).
>   *
>   * The MAC/L2 header is not accounted for.
> + *
> + * Return: the length, or a negative errno if the TCP header is not
> + * there to read.
>   */
> -static unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
> +static int skb_gso_network_seglen(const struct sk_buff *skb)
>  {
> -	unsigned int hdr_len = skb_transport_header(skb) -
> -			       skb_network_header(skb);
> +	unsigned int hdr_len;
> +	int tlen = skb_gso_transport_seglen(skb);

nit: reverse xmas tree

>  
> -	return hdr_len + skb_gso_transport_seglen(skb);
> +	if (tlen < 0)
> +		return tlen;
> +
> +	hdr_len = skb_transport_header(skb) - skb_network_header(skb);
> +
> +	return hdr_len + tlen;
>  }
>  
>  /**
> @@ -192,12 +235,21 @@ static unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
>   * skb_gso_mac_seglen is used to determine the real size of the
>   * individual segments, including MAC/L2, Layer3 (IP, IPv6) and L4
>   * headers (TCP/UDP).
> + *
> + * Return: the length, or a negative errno if the TCP header is not
> + * there to read.
>   */
> -static unsigned int skb_gso_mac_seglen(const struct sk_buff *skb)
> +static int skb_gso_mac_seglen(const struct sk_buff *skb)
>  {
> -	unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
> +	unsigned int hdr_len;
> +	int tlen = skb_gso_transport_seglen(skb);
>  
> -	return hdr_len + skb_gso_transport_seglen(skb);
> +	if (tlen < 0)
> +		return tlen;
> +
> +	hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
> +
> +	return hdr_len + tlen;
>  }
>  
>  /**
> @@ -253,7 +305,12 @@ static inline bool skb_gso_size_check(const struct sk_buff *skb,
>   */
>  bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu)
>  {
> -	return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
> +	int seg_len = skb_gso_network_seglen(skb);
> +
> +	if (seg_len < 0)
> +		return false;
> +
> +	return skb_gso_size_check(skb, seg_len, mtu);
>  }
>  EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
>  
> @@ -268,7 +325,12 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
>   */
>  bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
>  {
> -	return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
> +	int seg_len = skb_gso_mac_seglen(skb);
> +
> +	if (seg_len < 0)
> +		return false;
> +
> +	return skb_gso_size_check(skb, seg_len, len);
>  }
>  EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
>  
> -- 
> MST
> 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] net: gso: validate transport header before computing segment length
  2026-10-19 11:58 [PATCH net v2] net: gso: validate transport header before computing segment length Michael S. Tsirkin
  2026-10-20 19:10 ` Willem de Bruijn
@ 2026-10-22  2:59 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-10-22  2:59 UTC (permalink / raw)
  To: mst
  Cc: linux-kernel, willemb, davem, edumazet, kuba, pabeni, horms,
	maxk, netdev

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

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

- [High] This patch makes legitimate encapsulated TCP fraglist GRO
  packets fail the GSO length checks.
- [High] net: gso: skb_gso_transport_header_ok() drops XFRM tunnel TCP
  GSO packets
- [Medium] net: skb_inner_transport_header_was_set() returns true for ~0U
  sentinel

--

net: gso: validate transport header before computing segment length

This patch adds skb_gso_transport_header_ok(). With it, the TCP GSO
length validators reject an skb when its transport header (or inner
transport header) is unset or lies outside the linear data.
skb_gso_network_seglen() and skb_gso_mac_seglen() now return a negative
errno, and skb_gso_validate_network_len() and skb_gso_validate_mac_len()
turn that into false.

> diff --git a/net/core/gso.c b/net/core/gso.c
> index e96ef63500648..874e4c8b1f1bb 100644
> --- a/net/core/gso.c
> +++ b/net/core/gso.c
> @@ -131,6 +131,35 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>  }
>  EXPORT_SYMBOL(__skb_gso_segment);
>  
> +/*
> + * TSO processing (but not UDP/SCTP GSO, which uses fixed header
> + * sizes) reads the TCP header as specified by
> + * skb->transport_header/inner_transport_header.  Make sure it is
> + * there to read.
> + */
> +static bool skb_gso_transport_header_ok(const struct sk_buff *skb)
> +{
> +	const struct skb_shared_info *shinfo = skb_shinfo(skb);
> +	int thoff;
> +
> +	if (unlikely(!(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))))
> +		return true;
> +
> +	if (skb->encapsulation) {
> +		if (!skb_inner_transport_header_was_set(skb))
> +			return false;

[Severity: High]
Does this break TCP GSO over IPsec tunnel and BEET mode SAs that use
ESP offload?

xfrm_dev_offload_ok() succeeds for software ESP GSO via
esp4_offload/esp6_offload and also for XFRM_DEV_OFFLOAD_CRYPTO. When
it does, xfrm_output() sets skb->encapsulation on a GSO skb before the
outer mode has run:

net/xfrm/xfrm_output.c:xfrm_output() {
	...
		skb->encapsulation = 1;

		if (skb_is_gso(skb)) {
			...
			skb_shinfo(skb)->gso_type |= SKB_GSO_ESP;
			goto out;
		}
	...
}

For a plain TCP skb, inner_protocol is 0, so xfrm_output_gso() is not
taken. The skb goes through xfrm_output2(), xfrm_output_resume()
and xfrm_output_one() to xfrm4_prepare_output(). That calls
xfrm_inner_extract_output(), which reaches xfrm4_tunnel_check_size()
and skb_gso_validate_network_len(). Only after that does
xfrm4_tunnel_encap_add() call skb_set_inner_transport_header(). In
BEET mode, xfrm4_beet_encap_add() does not set it at all.
xfrm6_prepare_output() and xfrm6_tunnel_check_size() follow the same
order.

So the skb seen by skb_gso_transport_header_ok() has encapsulation
set, SKB_GSO_TCPV4 or SKB_GSO_TCPV6 in gso_type and
inner_transport_header == 0. Before this patch,
skb_gso_network_seglen() returned a bogus but small value that passed
the MTU check. The unsigned subtraction of transport_header in
skb_gso_transport_seglen() cancelled against the transport_header -
network_header term. Now the check fails every time.

xfrm4_tunnel_check_size() (with DF set) and xfrm6_tunnel_check_size()
then return -EMSGSIZE and report an MTU error. For local sockets that
goes through xfrm_local_error(), otherwise through ICMP_FRAG_NEEDED or
ICMPV6_PKT_TOOBIG. The reported MTU is one the flow already fits in.
As far as I can tell, every TCP GSO packet on such an SA is dropped.
That makes this a regression for what looks like a common IPsec
setup, and the Fixes: tag means it is likely to be backported to
stable.

Should xfrm set the inner headers before xfrm_inner_extract_output()
runs? Or should this check key off the tunnel gso_type bits rather
than skb->encapsulation alone? Falling back to the outer transport
header when inner_transport_header is unset might be another option,
though skb_gso_transport_seglen() would need the same treatment.

[Severity: Medium]
Does this treat an inner transport header that was copied from an
unset outer one as set?

skb_inner_network_header_was_set() can get away with a > 0 test
because network_header starts at 0 and is never set to ~0U. That does
not hold for transport_header. __finalize_skb_around() initialises it
to (typeof(skb->transport_header))~0U and skb_unset_transport_header()
puts it back there. skb_reset_inner_headers() copies it verbatim:

include/linux/skbuff.h:skb_reset_inner_headers() {
	...
	skb->inner_transport_header = skb->transport_header;
}

bpf_skb_net_grow() does the same. iptunnel_handle_offloads(),
bpf_lwt_push_ip_encap(), bpf_push_seg6_encap() and gtp_dev_xmit() all
go through skb_reset_inner_headers(). An skb whose transport header
was never set, or was unset as gro_cells_receive() does, therefore
ends up with skb->encapsulation = 1 and inner_transport_header ==
0xffff.

For such an skb, skb_inner_transport_header_was_set() returns true.
skb_gso_transport_header_ok() then only rejects it through the bounds
test on thoff = 0xffff - skb_headroom(skb). Consider a large linear
GSO skb where
skb_headroom(skb) + skb_headlen(skb) >= 65535 + sizeof(struct tcphdr),
for example one that bpf_skb_net_grow() has just linearized. It passes
that test, and inner_tcp_hdrlen() in skb_gso_transport_seglen() then
reads payload at skb->head + 0xffff as a TCP header. The access stays
within the linear area, but the segment length is then computed from
arbitrary payload bytes. That is what the unset check is meant to
prevent.

The helper lives in include/linux/skbuff.h, so other callers may pick
it up with the same blind spot. Should it also treat
(typeof(skb->inner_transport_header))~0U as unset, the way
skb_transport_header_was_set() does for the outer header?

[Severity: High]
Can this reject legitimate TCP fraglist GRO packets that arrive inside a
UDP tunnel?

udp_gro_complete() sets skb->encapsulation before it calls the tunnel's
inner gro_complete. It also clears encap_mark so that inner frag_list
completion can run:

net/ipv4/udp_offload.c:udp_gro_complete() {
    ...
		NAPI_GRO_CB(skb)->encap_mark = 0;
		...
		skb->encapsulation = 1;
		err = udp_sk(sk)->gro_complete(sk, skb,
				nhoff + sizeof(struct udphdr));
    ...
}

tcp4_check_fraglist_gro() sets is_flist = !sk && !skb_is_gso(skb) and
never looks at encapsulation. Any inner flow without a local socket
qualifies, for example forwarded traffic or containers behind a VXLAN
bridge. tcp4_gro_complete() then returns early:

net/ipv4/tcp_offload.c:tcp4_gro_complete() {
    ...
	if (unlikely(NAPI_GRO_CB(skb)->is_flist)) {
		skb_shinfo(skb)->gso_type |= SKB_GSO_FRAGLIST | SKB_GSO_TCPV4;
		skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;

		__skb_incr_checksum_unnecessary(skb);

		return 0;
	}
    ...
	tcp_gro_complete(skb);
    ...
}

tcp_gro_complete() is the only TCP GRO code that sets the inner
transport header:

net/ipv4/tcp_offload.c:tcp_gro_complete() {
    ...
	if (skb->encapsulation)
		skb->inner_transport_header = skb->transport_header;
    ...
}

tcp6_gro_complete() has the same flist early return.

The resulting skb has encapsulation set, a gso_type that includes
SKB_GSO_UDP_TUNNEL, SKB_GSO_FRAGLIST and SKB_GSO_TCPV4, and
inner_transport_header == 0.

Before this patch, skb_gso_transport_seglen() read the TCP data offset
relative to skb->head, which is inside the headroom. The unsigned
arithmetic gave a value that passed the size check. After this patch,
skb_gso_validate_network_len() and skb_gso_validate_mac_len() always
return false for these skbs.

Some effects of that:

tcf_police_mtu_check() always takes the exceed action, even with the
default tcfp_mtu of ~0.

ip_exceeds_mtu() returns true, so ip_forward() sends ICMP_FRAG_NEEDED
with the route MTU and drops DF transit tunnel traffic. GRO applies to
that traffic when it matches a wildcard-bound local tunnel socket. The
sender already honours that MTU, so the flow stays blackholed.

ip_finish_output_gso() falls back to software segmentation and
fragmentation.

Triggering this needs rx-gro-list enabled on the NIC and a UDP tunnel
socket with gro_receive, but the packets themselves come from remote
traffic. The patch also has a Fixes: tag, so it will likely be
backported to stable kernels that have TCP fraglist GRO.

Would it make sense to set inner_transport_header in the flist branch of
tcp4_gro_complete() and tcp6_gro_complete() when skb->encapsulation is
set? Another option is to skip the encapsulated check here for
SKB_GSO_FRAGLIST skbs.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/a53dbcdf068b3813689cf2359949578f627f9edd.1792410745.git.mst%40redhat.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-22  2:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-19 11:58 [PATCH net v2] net: gso: validate transport header before computing segment length Michael S. Tsirkin
2026-10-20 19:10 ` Willem de Bruijn
2026-10-22  2:59 ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®