From patchwork Mon Oct 22 21:26:36 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jerry Chu X-Patchwork-Id: 193290 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 6CEAA2C00F3 for ; Tue, 23 Oct 2012 08:28:24 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756192Ab2JVV2T (ORCPT ); Mon, 22 Oct 2012 17:28:19 -0400 Received: from mail-yh0-f74.google.com ([209.85.213.74]:33704 "EHLO mail-yh0-f74.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755742Ab2JVV2S (ORCPT ); Mon, 22 Oct 2012 17:28:18 -0400 Received: by mail-yh0-f74.google.com with SMTP id 10so398254yhl.1 for ; Mon, 22 Oct 2012 14:28:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer; bh=uYCBMDa9gBrgHqs9Mubd286u3yiYjfcahGqzAJzSJ/Y=; b=e9V3eExpl3ihRTBV4LkGikwEKA99lqWo1jnq+tKznZ3mARE+9FD4cetLENArsLMPlW LemxELAOyYF8sEk1mPpues/+lqE6rCr5w7mAwHIhLQECDD8hyTXvnEw4kTGszOApsIvd viMCyOSwZ9QC2J7PwKn9UkRF+W5yAafmudPJ1bG8IXaNdvs3T2ZyLLjTGVdleEXyei62 yUQg7L3BXv8Tip7/Gl/0qA0jrMlVV3FtZRgKfYBsJl2U1C3RhNST3jjyRnBXYlN/00Sr EB5AYZi36JO1ar7JUrdn+/J6vDpWiftvNlL2cf3K9oUvfbvPDKLwmkZ2VG8RuNy6m3yN jd7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:x-gm-message-state; bh=uYCBMDa9gBrgHqs9Mubd286u3yiYjfcahGqzAJzSJ/Y=; b=F7gon9gq7K6gSjXAEnPZcRaApeeLfXDvOTdCVDfLEuMUqaZFWusDJi0SSFrixTNf8a BngMMfjwNOgu35JU9GZXupUWEooRyYSXKvzD8/t3TcM5wkomQCrbxYoeii9t+1QnW6WE T0uzwA8YejKiockgaNyZVjcKGtoSfgS520XbIzFq43Q16iyv1Mgz0gk2YRQbvlP1il0i Hi2D5P5ymh/C5ZMXSHBKs1Cm0CW5Em0Nua5qbqNXh6OaAOVSoLejg1BtTJTHQ6iRwCk/ G2wrnmh/FHEPZm+2ZUFmDHl6O3e46FrhKHZXSwUFMSM1nmBminKNCnywvVdDBt4ejyU6 CKew== Received: by 10.236.89.114 with SMTP id b78mr7284222yhf.16.1350941297801; Mon, 22 Oct 2012 14:28:17 -0700 (PDT) Received: from wpzn4.hot.corp.google.com (216-239-44-65.google.com [216.239.44.65]) by gmr-mx.google.com with ESMTPS id l20si908677yhi.2.2012.10.22.14.28.17 (version=TLSv1/SSLv3 cipher=AES128-SHA); Mon, 22 Oct 2012 14:28:17 -0700 (PDT) Received: from hkchu.mtv.corp.google.com (hkchu.mtv.corp.google.com [172.17.130.255]) by wpzn4.hot.corp.google.com (Postfix) with ESMTP id AF7501E0043; Mon, 22 Oct 2012 14:28:17 -0700 (PDT) Received: by hkchu.mtv.corp.google.com (Postfix, from userid 19823) id 59E2FD87AC; Mon, 22 Oct 2012 14:28:17 -0700 (PDT) From: "H.K. Jerry Chu" To: davem@davemloft.net Cc: netdev@vger.kernel.org, ncardwell@google.com, ycheng@google.com, Jerry Chu Subject: [PATCH] tcp: Reject invalid ack_seq to Fast Open sockets Date: Mon, 22 Oct 2012 14:26:36 -0700 Message-Id: <1350941196-31224-1-git-send-email-hkchu@google.com> X-Mailer: git-send-email 1.7.7.3 X-Gm-Message-State: ALoCoQmYM2FaXXBbRPdpUU4YVZ4iJnvPfKcanEVOoZcbmjIlQR7xkZHLzgiEvkgGX+BnhdCI3qsXm6orT3nrynDGzwqfA25aK1d0xmGtFm4VqMe6KvGUjsnBW2mXLmxMA+vxj5OAxduHD5xF0Xbd7xrtqULpdQJUU6fp8WH/mNtX0UMeyokyMMXM17yY5JNgLVFvLQcAH8W0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Jerry Chu A packet with an invalid ack_seq may cause a TCP Fast Open socket to switch to the unexpected TCP_CLOSING state, triggering a BUG_ON kernel panic. When a FIN packet with an invalid ack_seq# arrives at a socket in the TCP_FIN_WAIT1 state, rather than discarding the packet, the current code will accept the FIN, causing state transition to TCP_CLOSING. This may be a small deviation from RFC793, which seems to say that the packet should be dropped. Unfortunately I did not expect this case for Fast Open hence it will trigger a BUG_ON panic. It turns out there is really nothing bad about a TFO socket going into TCP_CLOSING state so I could just remove the BUG_ON statements. But after some thought I think it's better to treat this case like TCP_SYN_RECV and return a RST to the confused peer who caused the unacceptable ack_seq to be generated in the first place. Signed-off-by: H.K. Jerry Chu Cc: Neal Cardwell Cc: Yuchung Cheng Acked-by: Yuchung Cheng Acked-by: Eric Dumazet Acked-by: Neal Cardwell --- net/ipv4/tcp_input.c | 12 ++++++++++-- net/ipv4/tcp_timer.c | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 60cf836..ce3e3c7 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -5970,7 +5970,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb, req = tp->fastopen_rsk; if (req != NULL) { - BUG_ON(sk->sk_state != TCP_SYN_RECV && + WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV && sk->sk_state != TCP_FIN_WAIT1); if (tcp_check_req(sk, skb, req, NULL, true) == NULL) @@ -6059,7 +6059,15 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb, * ACK we have received, this would have acknowledged * our SYNACK so stop the SYNACK timer. */ - if (acceptable && req != NULL) { + if (req != NULL) { + /* Return RST if ack_seq is invalid. + * Note that RFC793 only says to generate a + * DUPACK for it but for TCP Fast Open it seems + * better to treat this case like TCP_SYN_RECV + * above. + */ + if (!acceptable) + return 1; /* We no longer need the request sock. */ reqsk_fastopen_remove(sk, req, false); tcp_rearm_rto(sk); diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c index fc04711..a24f4a4 100644 --- a/net/ipv4/tcp_timer.c +++ b/net/ipv4/tcp_timer.c @@ -347,7 +347,7 @@ void tcp_retransmit_timer(struct sock *sk) return; } if (tp->fastopen_rsk) { - BUG_ON(sk->sk_state != TCP_SYN_RECV && + WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV && sk->sk_state != TCP_FIN_WAIT1); tcp_fastopen_synack_timer(sk); /* Before we receive ACK to our SYN-ACK don't retransmit