diff mbox series

[v2,net] tcp: fix retrans timestamp on passive Fast Open

Message ID 20190513173205.212181-1-ycheng@google.com
State Accepted
Delegated to: David Miller
Headers show
Series [v2,net] tcp: fix retrans timestamp on passive Fast Open | expand

Commit Message

Yuchung Cheng May 13, 2019, 5:32 p.m. UTC
Commit c7d13c8faa74 ("tcp: properly track retry time on
passive Fast Open") sets the start of SYNACK retransmission
time on passive Fast Open in "retrans_stamp". However the
timestamp is not reset upon the handshake has completed. As a
result, future data packet retransmission may not update it in
tcp_retransmit_skb(). This may lead to socket aborting earlier
unexpectedly by retransmits_timed_out() since retrans_stamp remains
the SYNACK rtx time.

This bug only manifests on passive TFO sender that a) suffered
SYNACK timeout and then b) stalls on very first loss recovery. Any
successful loss recovery would reset the timestamp to avoid this
issue.

Fixes: c7d13c8faa74 ("tcp: properly track retry time on passive Fast Open")
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv4/tcp_input.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Eric Dumazet May 13, 2019, 7:43 p.m. UTC | #1
On 5/13/19 10:32 AM, Yuchung Cheng wrote:
> Commit c7d13c8faa74 ("tcp: properly track retry time on
> passive Fast Open") sets the start of SYNACK retransmission
> time on passive Fast Open in "retrans_stamp". However the
> timestamp is not reset upon the handshake has completed. As a
> result, future data packet retransmission may not update it in
> tcp_retransmit_skb(). This may lead to socket aborting earlier
> unexpectedly by retransmits_timed_out() since retrans_stamp remains
> the SYNACK rtx time.
> 
> This bug only manifests on passive TFO sender that a) suffered
> SYNACK timeout and then b) stalls on very first loss recovery. Any
> successful loss recovery would reset the timestamp to avoid this
> issue.
> 
> Fixes: c7d13c8faa74 ("tcp: properly track retry time on passive Fast Open")
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> ---

Signed-off-by: Eric Dumazet <edumazet@google.com>
David Miller May 14, 2019, 10:19 p.m. UTC | #2
From: Yuchung Cheng <ycheng@google.com>
Date: Mon, 13 May 2019 10:32:05 -0700

> Commit c7d13c8faa74 ("tcp: properly track retry time on
> passive Fast Open") sets the start of SYNACK retransmission
> time on passive Fast Open in "retrans_stamp". However the
> timestamp is not reset upon the handshake has completed. As a
> result, future data packet retransmission may not update it in
> tcp_retransmit_skb(). This may lead to socket aborting earlier
> unexpectedly by retransmits_timed_out() since retrans_stamp remains
> the SYNACK rtx time.
> 
> This bug only manifests on passive TFO sender that a) suffered
> SYNACK timeout and then b) stalls on very first loss recovery. Any
> successful loss recovery would reset the timestamp to avoid this
> issue.
> 
> Fixes: c7d13c8faa74 ("tcp: properly track retry time on passive Fast Open")
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>

Applied and queued up for -stable, thanks.
diff mbox series

Patch

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 20f6fac5882e..cf69f50855ea 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6024,6 +6024,9 @@  static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 static void tcp_rcv_synrecv_state_fastopen(struct sock *sk)
 {
 	tcp_try_undo_loss(sk, false);
+
+	/* Reset rtx states to prevent spurious retransmits_timed_out() */
+	tcp_sk(sk)->retrans_stamp = 0;
 	inet_csk(sk)->icsk_retransmits = 0;
 
 	/* Once we leave TCP_SYN_RECV or TCP_FIN_WAIT_1,