[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()

[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>

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

[Paolo Abeni]

On 11/2/26 23:46, Alexei Starovoitov wrote:
> 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.

FTR the nipa sashiko instance dismissed the above concerns autonomously:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261101194433.2637113-1-alexei.starovoitov@gmail.com

you can reasonably safely refer only to such instance when submitting
patches to net/net-next.

/P

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

[Alexei Starovoitov]

On Wed Nov 4, 2026 at 8:41 AM UTC, Paolo Abeni wrote:
> On 11/2/26 23:46, Alexei Starovoitov wrote:
>> 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.
>
> FTR the nipa sashiko instance dismissed the above concerns autonomously:
>
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261101194433.2637113-1-alexei.starovoitov@gmail.com
>
> you can reasonably safely refer only to such instance when submitting
> patches to net/net-next.

Nice. I'll borrow your mechanism.

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

[Michael S. Tsirkin]

On Sun, Nov 01, 2026 at 07:44:33PM +0000, Alexei Starovoitov wrote:
> 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>

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

and cc stable, maybe.


> ---
>  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()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sun,  1 Nov 2026 19:44:33 +0000 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net] net: reject a negative TCP header offset in qdisc_pkt_len_segs_init()
    https://git.kernel.org/netdev/net/c/35d2d6e5ac0d

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