[PATCH net] net: reject a negative TCP header offset in qdisc_pkt_len_segs_init()

[PATCH net] net: reject a negative TCP header offset in qdisc_pkt_len_segs_init()

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

qdisc_pkt_len_segs_init() stores skb_transport_offset(), or the inner
offset, in the unsigned hdr_len. A header in front of skb->data gives a
negative offset. For an offset a little below zero, pskb_may_pull() gets a
small length and succeeds. Then a 64-bit kernel reads the TCP header far
after the skb and can crash. Further below zero pskb_may_pull() fails.

Commit 28a57fb2c5df ("net: iptunnel: fix stale transport header during
tunnel decapsulation") fixed one source of such offsets. Other code that
removes bytes from the front of a packet can leave the same stale header.

Return SKB_DROP_REASON_SKB_BAD_GSO for a negative offset in the TCP branch.
SKB_GSO_UDP_L4 and the other GSO types read no header and still get a wrong
qdisc_pkt_len(). On a 64-bit kernel only packets that cause the read after
the skb today get a new result. Then __dev_queue_xmit() drops them and
returns -EINVAL, but sch_handle_ingress() ignores the result. Their content
can be valid. Thus the code that leaves the header stale needs its own fix.

Fixes: 7fb4c1967011 ("net: pull headers in qdisc_pkt_len_segs_init()")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/dev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 3ac700d37e59..ccf4b02e08de 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4320,7 +4320,9 @@ static enum skb_drop_reason qdisc_pkt_len_segs_init(struct sk_buff *skb)
 	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
 		const struct tcphdr *th;
 
-		if (!pskb_may_pull(skb, hdr_len + sizeof(struct tcphdr)))
+		/* A header in front of skb->data gives a negative offset. */
+		if ((int)hdr_len < 0 ||
+		    !pskb_may_pull(skb, hdr_len + sizeof(struct tcphdr)))
 			return SKB_DROP_REASON_SKB_BAD_GSO;
 
 		th = (const struct tcphdr *)(skb->data + hdr_len);
-- 
2.55.0

Re: [PATCH net] net: reject a negative TCP header offset in qdisc_pkt_len_segs_init()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: reject a negative TCP header offset in qdisc_pkt_len_segs_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: negative hdr_len in qdisc_pkt_len_segs_init() only checked for TCP
--

commit dddca5a283a51c0b9bd1d1aef355eda12933179b
Author: Alexei Starovoitov <ast@kernel.org>

net: reject a negative TCP header offset in qdisc_pkt_len_segs_init()

This commit adds a negative offset check on hdr_len inside the TCP GSO
branch of qdisc_pkt_len_segs_init() to prevent an out-of-bounds read when
a stale transport header points in front of skb->data.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index f32353b0467cd..af948e4424035 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4320,7 +4320,9 @@ static enum skb_drop_reason qdisc_pkt_len_segs_init(struct sk_buff *skb)
>  	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
>  		const struct tcphdr *th;
>  
> -		if (!pskb_may_pull(skb, hdr_len + sizeof(struct tcphdr)))
> +		/* A header in front of skb->data gives a negative offset. */
> +		if ((int)hdr_len < 0 ||
> +		    !pskb_may_pull(skb, hdr_len + sizeof(struct tcphdr)))
>  			return SKB_DROP_REASON_SKB_BAD_GSO;

[Severity: High]
Should the (int)hdr_len < 0 check be placed immediately after hdr_len is
computed rather than only inside the SKB_GSO_TCPV4 | SKB_GSO_TCPV6 branch?

In net/core/dev.c:qdisc_pkt_len_segs_init(), leaving non-TCP GSO types
unguarded when skb_transport_offset(skb) or skb_inner_transport_offset(skb)
is negative appears to leave three issues:

First, for SKB_GSO_UDP_L4 packets:

	} else if (shinfo->gso_type & SKB_GSO_UDP_L4) {
		if (!pskb_may_pull(skb, hdr_len + sizeof(struct udphdr)))
			return SKB_DROP_REASON_SKB_BAD_GSO;
		hdr_len += sizeof(struct udphdr);
	}

When (int)hdr_len < -8, hdr_len + sizeof(struct udphdr) wraps to an
unsigned int in [0x80000008, 0xffffffff] (> INT_MAX), which triggers
DEBUG_NET_WARN_ON_ONCE(len > INT_MAX) in
include/linux/skbuff.h:pskb_may_pull_reason() before returning
SKB_DROP_REASON_SKB_BAD_GSO.

Second, when -8 <= (int)hdr_len < 0 on an SKB_GSO_UDP_L4 packet,
hdr_len + sizeof(struct udphdr) wraps to 0..7. pskb_may_pull() succeeds
without pulling the UDP header into skb->head, hdr_len wraps to 0..7, and
qdisc_pkt_len_segs_init() returns SKB_NOT_DROPPED_YET. If the packet is
then transmitted on a NETIF_F_GSO_UDP_L4 device whose driver uses the TSO
helpers in net/core/tso.c (such as bnxt_sw_udp_gso_xmit(),
enetc_map_tx_tso_buffs(), or otx2_sq_append_tso()), tso_build_hdr()
advances hdr by the negative transport offset:

	hdr += skb_transport_offset(skb);
	if (tso->tlen != sizeof(struct udphdr)) {
		...
	} else {
		struct udphdr *uh = (struct udphdr *)hdr;

		/* size is after segmentation. */
		udp_set_len_short(uh, sizeof(*uh) + size);
	}

Can this write uh->len (and uh->check in bnxt_sw_udp_gso_xmit() and
enetc_tso_hdr_csum()) before the start of the header buffer when
skb_transport_offset(skb) < -4?

Third, for other GSO types (such as SKB_GSO_FRAGLIST, SKB_GSO_UDP,
SKB_GSO_SCTP, or SKB_GSO_ESP), hdr_len remains 2^32 - |offset| at the end
of qdisc_pkt_len_segs_init():

	qdisc_skb_cb(skb)->pkt_len += (gso_segs - 1) * hdr_len;

When (gso_segs - 1) * |offset| > skb->len, can qdisc_skb_cb(skb)->pkt_len
underflow in 32-bit unsigned arithmetic to ~4.29 GiB (exceeding
QDISC_PKT_LEN_MAX), causing deficit-based schedulers such as drr_dequeue()
and ets_dequeue() to spin in their deficit-refill loops?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101194433.2637113-1-alexei.starovoitov@gmail.com?part=1

Re: [PATCH net] net: reject a negative TCP header offset in qdisc_pkt_len_segs_init()

[Alexei Starovoitov]

On Mon, Nov 02, 2026 at 11:44 AM sashiko-bot@kernel.org <sashiko-bot@kernel.org> wrote:
>> +		/* A header in front of skb->data gives a negative offset. */
>> +		if ((int)hdr_len < 0 ||
>> +		    !pskb_may_pull(skb, hdr_len + sizeof(struct tcphdr)))
>>  			return SKB_DROP_REASON_SKB_BAD_GSO;
>
> [Severity: High]
> Should the (int)hdr_len < 0 check be placed immediately after hdr_len is
> computed rather than only inside the SKB_GSO_TCPV4 | SKB_GSO_TCPV6 branch?

No. It's already explained in the commit log.

> When (int)hdr_len < -8, hdr_len + sizeof(struct udphdr) wraps to an
> unsigned int in [0x80000008, 0xffffffff] (> INT_MAX), which triggers
> DEBUG_NET_WARN_ON_ONCE(len > INT_MAX) in
> include/linux/skbuff.h:pskb_may_pull_reason() before returning
> SKB_DROP_REASON_SKB_BAD_GSO.

Not a bug. That's CONFIG_DEBUG_NET only and the warn is right:
the header is stale.

> Can this write uh->len (and uh->check in bnxt_sw_udp_gso_xmit() and
> enetc_tso_hdr_csum()) before the start of the header buffer when
> skb_transport_offset(skb) < -4?

No. The driver doesn't see this offset.

> Third, for other GSO types (such as SKB_GSO_FRAGLIST, SKB_GSO_UDP,
> SKB_GSO_SCTP, or SKB_GSO_ESP), hdr_len remains 2^32 - |offset| at the end
> of qdisc_pkt_len_segs_init():
>
> When (gso_segs - 1) * |offset| > skb->len, can qdisc_skb_cb(skb)->pkt_len
> underflow in 32-bit unsigned arithmetic to ~4.29 GiB (exceeding
> QDISC_PKT_LEN_MAX), causing deficit-based schedulers such as drr_dequeue()
> and ets_dequeue() to spin in their deficit-refill loops?

No.

Re: [PATCH net] net: reject a negative TCP header offset in qdisc_pkt_len_segs_init()

[Eric Dumazet]

Le dim. 1 nov. 2026 à 20:44, Alexei Starovoitov
<alexei.starovoitov@gmail.com> a écrit :
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> qdisc_pkt_len_segs_init() stores skb_transport_offset(), or the inner
> offset, in the unsigned hdr_len. A header in front of skb->data gives a
> negative offset. For an offset a little below zero, pskb_may_pull() gets a
> small length and succeeds. Then a 64-bit kernel reads the TCP header far
> after the skb and can crash. Further below zero pskb_may_pull() fails.
>
> Commit 28a57fb2c5df ("net: iptunnel: fix stale transport header during
> tunnel decapsulation") fixed one source of such offsets. Other code that
> removes bytes from the front of a packet can leave the same stale header.
>
> Return SKB_DROP_REASON_SKB_BAD_GSO for a negative offset in the TCP branch.
> SKB_GSO_UDP_L4 and the other GSO types read no header and still get a wrong
> qdisc_pkt_len(). On a 64-bit kernel only packets that cause the read after
> the skb today get a new result. Then __dev_queue_xmit() drops them and
> returns -EINVAL, but sch_handle_ingress() ignores the result. Their content
> can be valid. Thus the code that leaves the header stale needs its own fix.
>
> Fixes: 7fb4c1967011 ("net: pull headers in qdisc_pkt_len_segs_init()")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>