[PATCH 1/2] net: gso: validate TCP headers before segment length checks

[PATCH 1/2] net: gso: validate TCP headers before segment length checks

[Weiming Shi]

skb_gso_transport_seglen() derives the TCP header length with
tcp_hdrlen() or inner_tcp_hdrlen(). Both helpers dereference transport
header metadata without validating it first.

A TUN user can supply a TCP GSO packet without NEEDS_CSUM and with an
invalid IP header. The skb remains GSO while transport_header keeps the
unset sentinel. TBF and police can then reach the length validator and
read tcp->doff outside the skb head.

On the RX path, CONFIG_DEBUG_NET currently lets the unset marker survive
to ingress while non-debug builds still apply a temporary compatibility
reset. Validate the consumer instead of relying on that reset.

Validate TCP header ordering, linear bounds and fixed header presence
before either public GSO length check. For encapsulated TCP, validate the
inner offsets while allowing the outer and inner transport offsets to be
equal, as required by IPIP. Also validate the MAC header for the MAC
length variant.

Leave non-TCP GSO behavior unchanged. Those paths do not dereference a
TCP header, and valid FCoE skbs can have no transport header.

KASAN reports:

  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)

Cc: stable@vger.kernel.org
Fixes: 4d0820cf6a55 ("sch_tbf: handle too small burst")
Reported-by: <co+1fe9b56e2c61be5e@bugs.sh>
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 net/core/gso.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/net/core/gso.c b/net/core/gso.c
index bcd156372f4df..7c76f721fe24e 100644
--- a/net/core/gso.c
+++ b/net/core/gso.c
@@ -240,6 +240,29 @@ static inline bool skb_gso_size_check(const struct sk_buff *skb,
 	return true;
 }
 
+/* TCP segment length reads doff, so validate its header offsets first. */
+static bool skb_gso_tcp_header_valid(const struct sk_buff *skb)
+{
+	unsigned int transport = skb->transport_header;
+	unsigned int tail = skb_tail_pointer(skb) - skb->head;
+
+	if (!skb_is_gso_tcp(skb))
+		return true;
+
+	if (!skb_transport_header_was_set(skb) ||
+	    transport <= skb->network_header || transport > tail)
+		return false;
+
+	if (skb->encapsulation) {
+		transport = skb->inner_transport_header;
+		if (transport <= skb->inner_network_header ||
+		    transport < skb->transport_header || transport > tail)
+			return false;
+	}
+
+	return sizeof(struct tcphdr) <= tail - transport;
+}
+
 /**
  * skb_gso_validate_network_len - Will a split GSO skb fit into a given MTU?
  *
@@ -252,6 +275,9 @@ 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)
 {
+	if (unlikely(!skb_gso_tcp_header_valid(skb)))
+		return false;
+
 	return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
 }
 EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
@@ -267,7 +293,12 @@ EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
  */
 bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
 {
+	if (unlikely(!skb_gso_tcp_header_valid(skb) ||
+		     (skb_is_gso_tcp(skb) &&
+		      (!skb_mac_header_was_set(skb) ||
+		       skb->transport_header <= skb->mac_header))))
+		return false;
+
 	return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
 }
 EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
-
-- 
2.55.0

[PATCH 2/2] selftests: tc-testing: cover unset TCP transport header in TBF

[Weiming Shi]

A TCP GSO skb created through TUN can retain an unset transport
header. Send one such packet through TBF, then send another through
police and TBF. The police limit makes the vulnerable kernel stop at
one TBF drop while the fixed kernel reaches two. This checks behavior
without relying on KASAN or global logs.

Use the existing tc-testing namespace and JSON verification. The
helper creates the TUN packet, changes the ingress filter, and waits
for TBF counters.

Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 tools/testing/selftests/tc-testing/config     |  3 +
 .../tc-testing/tc-tests/qdiscs/tbf.json       | 28 ++++++
 .../testing/selftests/tc-testing/tdc_vnet.py  | 87 +++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 tools/testing/selftests/tc-testing/tdc_vnet.py

diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
index 0e5618be03359..7d1a140464948 100644
--- a/tools/testing/selftests/tc-testing/config
+++ b/tools/testing/selftests/tc-testing/config
@@ -5,6 +5,9 @@
 CONFIG_DUMMY=y
 CONFIG_VETH=y
 CONFIG_IFB=y
+CONFIG_TUN=y
+CONFIG_DEBUG_KERNEL=y
+CONFIG_DEBUG_NET=y
 
 #
 # Core Netfilter Configuration
diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/tbf.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/tbf.json
index 547a449100411..ef850a5d28ffe 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/tbf.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/tbf.json
@@ -189,5 +189,33 @@
         "teardown": [
             "$TC qdisc del dev $DUMMY handle 1: root"
         ]
+    },
+    {
+        "id": "7f31",
+        "name": "Drop GSO packet with unset transport header",
+        "category": [
+            "qdisc",
+            "tbf"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link set dev $IFB mtu 256",
+            "$TC qdisc add dev $IFB handle 1: root tbf limit 4096 burst 300 rate 1mbit"
+        ],
+        "cmdUnderTest": "python3 ./tdc_vnet.py $IP $TC $IFB",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j qdisc show dev $IFB root",
+        "matchJSON": [
+            {
+                "kind": "tbf",
+                "handle": "1:",
+                "drops": 2
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $IFB handle 1: root"
+        ]
     }
 ]
diff --git a/tools/testing/selftests/tc-testing/tdc_vnet.py b/tools/testing/selftests/tc-testing/tdc_vnet.py
new file mode 100644
index 0000000000000..bdd663c5795ac
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tdc_vnet.py
@@ -0,0 +1,87 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Exercise TBF and police with a TCP GSO skb lacking a transport header."""
+
+import fcntl
+import json
+import os
+import struct
+import subprocess
+import sys
+import time
+
+
+# These architectures use a different _IOW direction encoding.
+TUNSETIFF = (0x800454ca if os.uname().machine.startswith(
+    ("alpha", "hppa", "mips", "parisc", "ppc", "sparc")) else 0x400454ca)
+IFF_TUN = 0x0001
+IFF_NO_PI = 0x1000
+IFF_VNET_HDR = 0x4000
+TUN = "tuntdc0"
+DEADLINE = time.monotonic() + 18
+
+
+def run(*argv):
+    remaining = DEADLINE - time.monotonic()
+    if remaining <= 0:
+        raise RuntimeError("selftest deadline expired")
+    return subprocess.check_output(argv, stderr=subprocess.STDOUT,
+                                   text=True, timeout=min(3, remaining))
+
+
+def tbf_drops(tc, ifb):
+    qdiscs = json.loads(run(tc, "-s", "-j", "qdisc", "show", "dev", ifb,
+                           "root"))
+    return next(qdisc["drops"] for qdisc in qdiscs
+                if qdisc["kind"] == "tbf" and qdisc["handle"] == "1:")
+
+
+def wait_for_drops(tc, ifb, expected):
+    deadline = min(DEADLINE, time.monotonic() + 5)
+    while time.monotonic() < deadline:
+        if tbf_drops(tc, ifb) >= expected:
+            return
+        time.sleep(0.05)
+    raise RuntimeError(f"TBF did not reach {expected} drops")
+
+
+def main(ip, tc, ifb):
+    tun = os.open("/dev/net/tun", os.O_RDWR | os.O_CLOEXEC | os.O_NONBLOCK)
+    try:
+        ifreq = struct.pack("16sH", TUN.encode(),
+                            IFF_TUN | IFF_NO_PI | IFF_VNET_HDR)
+        fcntl.ioctl(tun, TUNSETIFF, ifreq)
+        run(ip, "link", "set", "dev", TUN, "up")
+        run(tc, "qdisc", "add", "dev", TUN, "clsact")
+        run(tc, "filter", "add", "dev", TUN, "ingress", "pref", "1",
+            "matchall", "action", "mirred", "egress", "redirect",
+            "dev", ifb)
+
+        # GSO without NEEDS_CSUM leaves transport_header unset. IPv4 IHL=0
+        # prevents the later transport-header probe from filling it in.
+        packet = struct.pack("=BBHHHH", 0, 1, 0, 8, 0, 0) + b"\x40" + bytes(999)
+        if os.write(tun, packet) != len(packet):
+            raise RuntimeError("short TUN write")
+        wait_for_drops(tc, ifb, 1)
+
+        run(tc, "filter", "delete", "dev", TUN, "ingress", "pref", "1")
+        run(tc, "filter", "add", "dev", TUN, "ingress", "pref", "1",
+            "matchall", "action", "police", "mtu", "65700",
+            "conform-exceed", "pipe/drop", "action", "mirred", "egress",
+            "redirect", "dev", ifb)
+        if os.write(tun, packet) != len(packet):
+            raise RuntimeError("short TUN write")
+        wait_for_drops(tc, ifb, 2)
+    finally:
+        os.close(tun)
+
+
+if __name__ == "__main__":
+    if len(sys.argv) != 4:
+        sys.exit(f"usage: {sys.argv[0]} IP TC IFB")
+    try:
+        main(*sys.argv[1:])
+    except (OSError, ValueError, RuntimeError, StopIteration,
+            subprocess.SubprocessError) as error:
+        sys.exit(f"tdc_vnet: {error}")
-- 
2.55.0

Re: [PATCH 1/2] net: gso: validate TCP headers before segment length checks

[Eric Dumazet]

On Sun, Sep 27, 2026 at 6:31 PM Weiming Shi <bestswngs@gmail.com> wrote:
>
> skb_gso_transport_seglen() derives the TCP header length with
> tcp_hdrlen() or inner_tcp_hdrlen(). Both helpers dereference transport
> header metadata without validating it first.
>
> A TUN user can supply a TCP GSO packet without NEEDS_CSUM and with an
> invalid IP header. The skb remains GSO while transport_header keeps the
> unset sentinel. TBF and police can then reach the length validator and
> read tcp->doff outside the skb head.

Do you realize this leaves non-TCP GSO broken with unset transport_header ?

Please fix tun instead of adding numerous workarounds in our fast paths.

__virtio_net_hdr_to_skb() needs hardening instead.

virtio_net/tun/af_packet MUST not feed 'gso' packets without transport
header being set,
among other things.

pw-bot: rejected

Re: [PATCH 1/2] net: gso: validate TCP headers before segment length checks

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · 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] Over-strict ordering check rejects valid decapsulated TCP GSO
  traffic.
- [Medium] skb_gso_tcp_header_valid() ends with `return sizeof(struct
  tcphdr) <= tail - transport;`, where tail is the linear tail.

--

net: gso: validate TCP headers before segment length checks

This adds skb_gso_tcp_header_valid(). The helper checks TCP transport
header ordering, linear bounds and fixed header presence before
skb_gso_validate_network_len() and skb_gso_validate_mac_len() compute
segment lengths. It targets a KASAN out-of-bounds read of tcp->doff when
malformed TUN-supplied TCP GSO skbs reach TBF or police.

> diff --git a/net/core/gso.c b/net/core/gso.c
> index bcd156372f4df..7c76f721fe24e 100644
> --- a/net/core/gso.c
> +++ b/net/core/gso.c
> @@ -240,6 +240,29 @@ static inline bool skb_gso_size_check(const struct sk_buff *skb,
>  	return true;
>  }
>  
> +/* TCP segment length reads doff, so validate its header offsets first. */
> +static bool skb_gso_tcp_header_valid(const struct sk_buff *skb)
> +{
> +	unsigned int transport = skb->transport_header;
> +	unsigned int tail = skb_tail_pointer(skb) - skb->head;
> +
> +	if (!skb_is_gso_tcp(skb))
> +		return true;
> +
> +	if (!skb_transport_header_was_set(skb) ||
> +	    transport <= skb->network_header || transport > tail)
> +		return false;

[Severity: High]
Can this ordering check reject valid decapsulated TCP GSO skbs on the RX
path?

After UDP tunnel decapsulation, a valid TCP GSO skb can keep a stale
transport_header. This happens when the tunnel device skips GRO, either
because NETIF_F_GRO is off or because an XDP program is attached, so
netif_elide_gro() is true:

ip_rcv_core()
    skb->transport_header = skb->network_header + iph->ihl*4;  <- outer UDP

vxlan_rcv()->__iptunnel_pull_header()->iptunnel_pull_offloads()
    clears the encap GSO bits and encapsulation,
    leaves transport_header alone

gro_cells_receive()
    if (!gcells->cells || skb_cloned(skb) || netif_elide_gro(dev)) {
        res = netif_rx(skb);
    so inet_gro_receive() never refreshes the transport header

__netif_receive_skb_core()
    skb_reset_network_header(skb);
    ...
    if (!skb_transport_header_was_set(skb))
        skb_reset_transport_header(skb);

That leaves gso_type=TCPV4 and encapsulation=0, with transport_header still
pointing at the outer UDP header. That offset comes before both the inner
mac header and the inner network header.

In that case transport <= skb->network_header is true. The extra
skb->transport_header <= skb->mac_header test in skb_gso_validate_mac_len()
is also true. So both validators return false.

Callers read false as "segments do not fit". tcf_police_mtu_check() runs
this for every policed GSO packet:

    if (skb_is_gso(skb))
        return skb_gso_validate_mac_len(skb, limit);

tcfp_mtu defaults to ~0 when no mtu is given. Before this patch, the
unsigned wrapped arithmetic happened to fit and these packets passed.

Would a police action on the tunnel device's ingress now send every such
GSO packet to the exceed action?

tbf_enqueue() would likewise drop the packet with qdisc_drop() instead of
calling tbf_segment(). skb_tunnel_check_pmtu() would treat it as a PMTU
violation, which produces ICMP FRAG_NEEDED on bridge ports.

This patch is tagged for stable. Would it be safer to reject only an unset
transport header or offsets outside the linear head, or to read doff with
skb_header_pointer(), rather than rely on header ordering?

> +
> +	if (skb->encapsulation) {
> +		transport = skb->inner_transport_header;
> +		if (transport <= skb->inner_network_header ||
> +		    transport < skb->transport_header || transport > tail)
> +			return false;
> +	}
> +
> +	return sizeof(struct tcphdr) <= tail - transport;

[Severity: Medium]
Does this reject TCP GSO skbs whose headers are valid but whose fixed TCP
header is partly in paged frags?

tail here is the linear tail, so the whole 20-byte tcphdr has to be in the
linear area.

Take the IFF_TAP path of tun_get_user() without
VIRTIO_NET_HDR_F_NEEDS_CSUM, which is the producer the commit message
describes. __virtio_net_hdr_to_skb() has no network header yet, so it takes
this branch:

    } else if (gso_type) {
        p_off = nh_min_len + thlen;
        if (!pskb_may_pull(skb, p_off))
            return -EINVAL;
    }

That pulls only 40 bytes counted from the MAC header. With a small hdr_len
and a packet larger than a page, tun_alloc_skb() leaves the rest in frags.
After eth_type_trans(), skb_probe_transport_header() sets transport to
network + 20 for a valid IPv4 header. Only 6 TCP bytes are then linear.

Before this patch, such a packet was forwarded, or segmented by
skb_gso_segment(), which pulls headers properly. tcp_hdrlen() did read
stale tailroom bytes on that path.

Now both validators report the packet as too big. For example, in
ip_exceeds_mtu():

    if (skb_is_gso(skb) && skb_gso_validate_network_len(skb, mtu))
        return false;

the packet gets ICMP_FRAG_NEEDED and is dropped. ip6_pkt_too_big(),
skb_tunnel_check_pmtu(), tbf_enqueue(), execute_check_pkt_len() and
tcf_police_act() get the same false result.

The commit message doesn't mention that header linearity failures are now
reported as MTU violations.

The validators take a const skb and cannot pull. Would reading doff through
skb_header_pointer() avoid the out-of-bounds read without rejecting
non-linear but valid headers?

> +}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927163117.746432-2-bestswngs%40gmail.com