netdev - Re: [PATCH net] tcp: fix undo spurious SYNACK in passive Fast Open

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: <20190609.200432.128713001997279882.davem@davemloft.net>
Date:   Sun, 09 Jun 2019 20:04:32 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     ycheng@...gle.com
Cc:     netdev@...r.kernel.org, ncardwell@...gle.com, edumazet@...gle.com
Subject: Re: [PATCH net] tcp: fix undo spurious SYNACK in passive Fast Open

From: Yuchung Cheng <ycheng@...gle.com>
Date: Fri,  7 Jun 2019 18:26:33 -0700

> Commit 794200d66273 ("tcp: undo cwnd on Fast Open spurious SYNACK
> retransmit") may cause tcp_fastretrans_alert() to warn about pending
> retransmission in Open state. This is triggered when the Fast Open
> server both sends data and has spurious SYNACK retransmission during
> the handshake, and the data packets were lost or reordered.
> 
> The root cause is a bit complicated:
> 
> (1) Upon receiving SYN-data: a full socket is created with
>     snd_una = ISN + 1 by tcp_create_openreq_child()
> 
> (2) On SYNACK timeout the server/sender enters CA_Loss state.
> 
> (3) Upon receiving the final ACK to complete the handshake, sender
>     does not mark FLAG_SND_UNA_ADVANCED since (1)
> 
>     Sender then calls tcp_process_loss since state is CA_loss by (2)
> 
> (4) tcp_process_loss() does not invoke undo operations but instead
>     mark REXMIT_LOST to force retransmission
> 
> (5) tcp_rcv_synrecv_state_fastopen() calls tcp_try_undo_loss(). It
>     changes state to CA_Open but has positive tp->retrans_out
> 
> (6) Next ACK triggers the WARN_ON in tcp_fastretrans_alert()
> 
> The step that goes wrong is (4) where the undo operation should
> have been invoked because the ACK successfully acknowledged the
> SYN sequence. This fixes that by specifically checking undo
> when the SYN-ACK sequence is acknowledged. Then after
> tcp_process_loss() the state would be further adjusted based
> in tcp_fastretrans_alert() to avoid triggering the warning in (6).
> 
> Fixes: 794200d66273 ("tcp: undo cwnd on Fast Open spurious SYNACK retransmit")
> Signed-off-by: Yuchung Cheng <ycheng@...gle.com>
> Signed-off-by: Neal Cardwell <ncardwell@...gle.com>

Applied, thank you.