netdev - [PATCH net] tcp: tcp_acceptable_seq select SND.UNA when SND.WND is 0

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-next>] [day] [month] [year] [list]

Message-ID: <20250410175140.10805-3-luizcmpc@gmail.com>
Date: Thu, 10 Apr 2025 17:50:34 +0000
From: Luiz Carvalho <luizcmpc@...il.com>
To: netdev@...r.kernel.org
Cc: Luiz Carvalho <luizcmpc@...il.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Neal Cardwell <ncardwell@...gle.com>
Subject: [PATCH net] tcp: tcp_acceptable_seq select SND.UNA when SND.WND is 0

The current tcp_acceptable_seq() returns SND.NXT when the available
window shrinks to less then one scaling factor. This works fine for most
cases, and seemed to not be a problem until a slight behavior change to
how tcp_select_window() handles ZeroWindow cases.

Before commit 8c670bdfa58e ("tcp: correct handling of extreme memory squeeze"),
a zero window would only be announced when data failed to be consumed,
and following packets would have non-zero windows despite the receiver
still not having any available space. After the commit, however, the
zero window is stored in the socket and the advertised window will be
zero until the receiver frees up space.

For tcp_acceptable_seq(), a zero window case will result in SND.NXT
being sent, but the problem now arises when the receptor validates the
sequence number in tcp_sequence():

static enum skb_drop_reason tcp_sequence(const struct tcp_sock *tp,
					 u32 seq, u32 end_seq)
{
	// ...
	if (after(seq, tp->rcv_nxt + tcp_receive_window(tp)))
		return SKB_DROP_REASON_TCP_INVALID_SEQUENCE;
	// ...
}

Because RCV.WND is now stored in the socket as zero, using SND.NXT will fail
the INVALID_SEQUENCE check: SEG.SEQ <= RCV.NXT + RCV.WND. A valid ACK is
dropped by the receiver, correctly, as RFC793 mentions:

	There are four cases for the acceptability test for an incoming
        segment:

	Segment Receive  Test
        Length  Window
        ------- -------  -------------------------------------------

           0       0     SEG.SEQ = RCV.NXT

The ACK will be ignored until tcp_write_wakeup() sends SND.UNA again,
and the connection continues. If the receptor announces ZeroWindow
again, the stall could be very long, as was in my case. Found this out
while giving a shot at bug #213827.

Could the precision loss lead to a zero window? If so, then this
approach probably won't work.

Fixes: a4ecb15a2432 ("tcp: accommodate sequence number to a peer's shrunk receive window caused by precision loss in window scaling")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=213827
Signed-off-by: Luiz Carvalho <luizcmpc@...il.com>
---
 net/ipv4/tcp_output.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index bc95d2a5924f..4d05083372a0 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -88,7 +88,7 @@ static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
 }

 /* SND.NXT, if window was not shrunk or the amount of shrunk was less than one
- * window scaling factor due to loss of precision.
+ * window scaling factor due to loss of precision, but window is not zero.
  * If window has been shrunk, what should we make? It is not clear at all.
  * Using SND.UNA we will fail to open window, SND.NXT is out of window. :-(
  * Anything in between SND.UNA...SND.UNA+SND.WND also can be already
@@ -99,7 +99,7 @@ static inline __u32 tcp_acceptable_seq(const struct sock *sk)
 	const struct tcp_sock *tp = tcp_sk(sk);

 	if (!before(tcp_wnd_end(tp), tp->snd_nxt) ||
-	    (tp->rx_opt.wscale_ok &&
+	    (tp->snd_wnd && tp->rx_opt.wscale_ok &&
 	     ((tp->snd_nxt - tcp_wnd_end(tp)) < (1 << tp->rx_opt.rcv_wscale))))
 		return tp->snd_nxt;
 	else
-- 
2.43.0