netdev - Re: [PATCH net] tcp: fix tcp_packet_delayed() for tcp_is_non_sack_preventing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89iLFNDh1gbAXgS5XVUhjRz8NX9Gsrymcnv7A-SB1s2wpqg@mail.gmail.com>
Date: Fri, 13 Jun 2025 23:17:51 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Neal Cardwell <ncardwell.sw@...il.com>
Cc: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, 
	Neal Cardwell <ncardwell@...gle.com>, Eric Wheeler <netdev@...ts.ewheeler.net>, 
	Yuchung Cheng <ycheng@...gle.com>
Subject: Re: [PATCH net] tcp: fix tcp_packet_delayed() for tcp_is_non_sack_preventing_reopen()
 behavior

On Fri, Jun 13, 2025 at 12:31 PM Neal Cardwell <ncardwell.sw@...il.com> wrote:
>
> From: Neal Cardwell <ncardwell@...gle.com>
>
> After the following commit from 2024:
>
> commit e37ab7373696 ("tcp: fix to allow timestamp undo if no retransmits were sent")
>
> ...there was buggy behavior where TCP connections without SACK support
> could easily see erroneous undo events at the end of fast recovery or
> RTO recovery episodes. The erroneous undo events could cause those
> connections to suffer repeated loss recovery episodes and high
> retransmit rates.
>
> The problem was an interaction between the non-SACK behavior on these
> connections and the undo logic. The problem is that, for non-SACK
> connections at the end of a loss recovery episode, if snd_una ==
> high_seq, then tcp_is_non_sack_preventing_reopen() holds steady in
> CA_Recovery or CA_Loss, but clears tp->retrans_stamp to 0. Then upon
> the next ACK the "tcp: fix to allow timestamp undo if no retransmits
> were sent" logic saw the tp->retrans_stamp at 0 and erroneously
> concluded that no data was retransmitted, and erroneously performed an
> undo of the cwnd reduction, restoring cwnd immediately to the value it
> had before loss recovery.  This caused an immediate burst of traffic
> and build-up of queues and likely another immediate loss recovery
> episode.
>
> This commit fixes tcp_packet_delayed() to ignore zero retrans_stamp
> values for non-SACK connections when snd_una is at or above high_seq,
> because tcp_is_non_sack_preventing_reopen() clears retrans_stamp in
> this case, so it's not a valid signal that we can undo.
>
> Note that the commit named in the Fixes footer restored long-present
> behavior from roughly 2005-2019, so apparently this bug was present
> for a while during that era, and this was simply not caught.
>
> Fixes: e37ab7373696 ("tcp: fix to allow timestamp undo if no retransmits were sent")
> Reported-by: Eric Wheeler <netdev@...ts.ewheeler.net>
> Closes: https://lore.kernel.org/netdev/64ea9333-e7f9-0df-b0f2-8d566143acab@ewheeler.net/
> Signed-off-by: Neal Cardwell <ncardwell@...gle.com>
> Co-developed-by: Yuchung Cheng <ycheng@...gle.com>
> Signed-off-by: Yuchung Cheng <ycheng@...gle.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@...gle.com>