From patchwork Tue Apr 7 10:22:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 228140 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=3.0 tests=DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 660BCC2BB1D for ; Tue, 7 Apr 2020 10:29:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3E25D2074F for ; Tue, 7 Apr 2020 10:29:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1586255368; bh=zHf6rioeVhtooweuQYliiB150kH9YFprMHPlKJ3+gm4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=t4H4cDL8LJNuApzjj2If0Y7g/RaZocDbjDp+BdeJ65LEhcrghG0KVYVjhvnLB742j FWhTRKXjOPyOO2tLyMsjv3xMaaM65grQYUsHktAbMdqkI7fIBZn2mjOJefG+jb+leI AA9klhxoERDenQicb7L4ro3kjlKNj2vlccx0bOG0= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728529AbgDGKXj (ORCPT ); Tue, 7 Apr 2020 06:23:39 -0400 Received: from mail.kernel.org ([198.145.29.99]:33516 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728525AbgDGKXj (ORCPT ); Tue, 7 Apr 2020 06:23:39 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 2798F2074B; Tue, 7 Apr 2020 10:23:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1586255018; bh=zHf6rioeVhtooweuQYliiB150kH9YFprMHPlKJ3+gm4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Qq9tt5IKmi+hVerW1xc+brPvlbld+eauMVCD9j4ZrOYfg18MyzU7HBqO+0pPTNtzX UB4G8omBRy46ylHyf3eWnRQRB5km5U0yGxnzU9mqaZG+vl56Uus+QiMj3U88tW7FFu LeMqgnEu9PSVL2GoYNQnM6niUaHxJFpA+SdF7ifM= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Neal Cardwell , Yuchung Cheng , Eric Dumazet , "David S. Miller" 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 Message-Id: <20200407101458.459005973@linuxfoundation.org> X-Mailer: git-send-email 2.26.0 In-Reply-To: <20200407101454.281052964@linuxfoundation.org> References: <20200407101454.281052964@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Neal Cardwell 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 Signed-off-by: Yuchung Cheng Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- 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;