stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Neal Cardwell <ncardwell@google.com>,
	Yuchung Cheng <ycheng@google.com>,
	Eric Dumazet <edumazet@google.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 5.4 33/36] tcp: fix TFO SYNACK undo to avoid double-timestamp-undo
Date: Tue,  7 Apr 2020 12:22:06 +0200	[thread overview]
Message-ID: <20200407101458.459005973@linuxfoundation.org> (raw)
In-Reply-To: <20200407101454.281052964@linuxfoundation.org>

From: Neal Cardwell <ncardwell@google.com>

commit dad8cea7add96a353fa1898b5ccefbb72da66f29 upstream.

In a rare corner case the new logic for undo of SYNACK RTO could
result in triggering the warning in tcp_fastretrans_alert() that says:
        WARN_ON(tp->retrans_out != 0);

The warning looked like:

WARNING: CPU: 1 PID: 1 at net/ipv4/tcp_input.c:2818 tcp_ack+0x13e0/0x3270

The sequence that tickles this bug is:
 - Fast Open server receives TFO SYN with data, sends SYNACK
 - (client receives SYNACK and sends ACK, but ACK is lost)
 - server app sends some data packets
 - (N of the first data packets are lost)
 - server receives client ACK that has a TS ECR matching first SYNACK,
   and also SACKs suggesting the first N data packets were lost
    - server performs TS undo of SYNACK RTO, then immediately
      enters recovery
    - buggy behavior then performed a *second* undo that caused
      the connection to be in CA_Open with retrans_out != 0

Basically, the incoming ACK packet with SACK blocks causes us to first
undo the cwnd reduction from the SYNACK RTO, but then immediately
enters fast recovery, which then makes us eligible for undo again. And
then tcp_rcv_synrecv_state_fastopen() accidentally performs an undo
using a "mash-up" of state from two different loss recovery phases: it
uses the timestamp info from the ACK of the original SYNACK, and the
undo_marker from the fast recovery.

This fix refines the logic to only invoke the tcp_try_undo_loss()
inside tcp_rcv_synrecv_state_fastopen() if the connection is still in
CA_Loss.  If peer SACKs triggered fast recovery, then
tcp_rcv_synrecv_state_fastopen() can't safely undo.

Fixes: 794200d66273 ("tcp: undo cwnd on Fast Open spurious SYNACK retransmit")
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 net/ipv4/tcp_input.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6096,7 +6096,11 @@ static void tcp_rcv_synrecv_state_fastop
 {
 	struct request_sock *req;
 
-	tcp_try_undo_loss(sk, false);
+	/* If we are still handling the SYNACK RTO, see if timestamp ECR allows
+	 * undo. If peer SACKs triggered fast recovery, we can't undo here.
+	 */
+	if (inet_csk(sk)->icsk_ca_state == TCP_CA_Loss)
+		tcp_try_undo_loss(sk, false);
 
 	/* Reset rtx states to prevent spurious retransmits_timed_out() */
 	tcp_sk(sk)->retrans_stamp = 0;



  parent reply	other threads:[~2020-04-07 10:29 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-07 10:21 [PATCH 5.4 00/36] 5.4.31-rc1 review Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 01/36] nvme-rdma: Avoid double freeing of async event data Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 02/36] kconfig: introduce m32-flag and m64-flag Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 03/36] drm/amd/display: Add link_rate quirk for Apple 15" MBP 2017 Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 04/36] drm/bochs: downgrade pci_request_region failure from error to warning Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 05/36] initramfs: restore default compression behavior Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 06/36] drm/amdgpu: fix typo for vcn1 idle check Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 07/36] tools/power turbostat: Fix gcc build warnings Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 08/36] tools/power turbostat: Fix missing SYS_LPI counter on some Chromebooks Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 09/36] tools/power turbostat: Fix 32-bit capabilities warning Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 10/36] bpf: Fix tnum constraints for 32-bit comparisons Greg Kroah-Hartman
2020-04-07 10:45   ` Daniel Borkmann
2020-04-07 14:42     ` Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 11/36] net/mlx5e: kTLS, Fix TCP seq off-by-1 issue in TX resync flow Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 12/36] XArray: Fix xa_find_next for large multi-index entries Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 13/36] padata: fix uninitialized return value in padata_replace() Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 14/36] brcmfmac: abort and release host after error Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 15/36] misc: rtsx: set correct pcr_ops for rts522A Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 16/36] misc: pci_endpoint_test: Fix to support > 10 pci-endpoint-test devices Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 17/36] misc: pci_endpoint_test: Avoid using module parameter to determine irqtype Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 18/36] PCI: sysfs: Revert "rescan" file renames Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 19/36] coresight: do not use the BIT() macro in the UAPI header Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 20/36] mei: me: add cedar fork device ids Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 21/36] nvmem: check for NULL reg_read and reg_write before dereferencing Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 22/36] extcon: axp288: Add wakeup support Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 23/36] power: supply: axp288_charger: Add special handling for HP Pavilion x2 10 Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 24/36] Revert "dm: always call blk_queue_split() in dm_process_bio()" Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 25/36] ALSA: hda/ca0132 - Add Recon3Di quirk to handle integrated sound on EVGA X99 Classified motherboard Greg Kroah-Hartman
2020-04-07 10:21 ` [PATCH 5.4 26/36] soc: mediatek: knows_txdone needs to be set in Mediatek CMDQ helper Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.4 27/36] net/mlx5e: kTLS, Fix wrong value in record tracker enum Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.4 28/36] iwlwifi: consider HE capability when setting LDPC Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.4 29/36] iwlwifi: yoyo: dont add TLV offset when reading FIFOs Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.4 30/36] iwlwifi: dbg: dont abort if sending DBGC_SUSPEND_RESUME fails Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.4 31/36] rxrpc: Fix sendmsg(MSG_WAITALL) handling Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.4 32/36] IB/hfi1: Ensure pq is not left on waitlist Greg Kroah-Hartman
2020-04-07 10:22 ` Greg Kroah-Hartman [this message]
2020-04-07 10:22 ` [PATCH 5.4 34/36] i2c: i801: Do not add ICH_RES_IO_SMI for the iTCO_wdt device Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.4 35/36] net: Fix Tx hash bound checking Greg Kroah-Hartman
2020-04-07 10:22 ` [PATCH 5.4 36/36] padata: always acquire cpu_hotplug_lock before pinst->lock Greg Kroah-Hartman
2020-04-07 12:37 ` [PATCH 5.4 00/36] 5.4.31-rc1 review Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200407101458.459005973@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ncardwell@google.com \
    --cc=stable@vger.kernel.org \
    --cc=ycheng@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).