From patchwork Tue Sep 18 23:35:51 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jerry Chu X-Patchwork-Id: 184873 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 117ED2C0086 for ; Wed, 19 Sep 2012 09:36:10 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751542Ab2IRXgI (ORCPT ); Tue, 18 Sep 2012 19:36:08 -0400 Received: from mail-gg0-f202.google.com ([209.85.161.202]:64150 "EHLO mail-gg0-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751044Ab2IRXgF (ORCPT ); Tue, 18 Sep 2012 19:36:05 -0400 Received: by ggnu1 with SMTP id u1so63659ggn.1 for ; Tue, 18 Sep 2012 16:36:04 -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=OYn2bC2JBUOEVvw1gtFTRMxx6S7JFjVjJ5o3yOfecY0=; b=Yt9sfvsnDAYvYSmrs/EHVDxNfN+HRTH9Gj4rQBTpdA46jCoX8O0JxUvbd7Rz+npPzz X7wN2nnPmZx+9RMkH6L1JS4JleEUIoN6NaBSVafX2zumzIlqgxblFHnJcsO3GJI6TmG9 QqnlEXRVrYoe2g+JccXCqHIjjjFRyOFfDprcYGWSnySW0pMXTXaKj+LQzknRAVqGdmrw RSH7UzNdkjUa2QyWO52xfI4PY+rLXAjF1bMDCOAW58C1t3+lh6hzn+kgqM0k/XJZVZfy kjCQSyLhf+rdHzcjxe1nJWdTkLcEZcEEE0gw07v55iA47tfPT+bVzmebPmcFw3K53YG8 DkhA== 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=OYn2bC2JBUOEVvw1gtFTRMxx6S7JFjVjJ5o3yOfecY0=; b=C1rVv7CcPqKzvTuSv7GwHBhHErCBI1sz3+kaAAZyVPJtiQLNZ38inXH0n7BFaDLU97 bhPxil/BOwd85mVmxmk4aGUeVDrEbJ85e/baD4kodqx9ZeklgAayuIjnD9kB8lEQQvWy LjYDPUHo8KmGCZEdeXK/vrEywKnp4eqmV6Vb3B2gBMvDnJlYVyvp2ny2k+0A5FtagZP0 UtRCEx40dqBzkoIrmbLVNBjwpqMX4ZukVGpRXU97mBbHsP95QQc6z6CHLb5mJYSefUo6 /ELM8WtqlQVStro3eBuR5XY3OlcAN4adZ3qVn1eyTspsnyrvyz4eFTlQN7I1J8EJx8e/ nNTw== Received: by 10.236.200.194 with SMTP id z42mr1055553yhn.13.1348011363990; Tue, 18 Sep 2012 16:36:03 -0700 (PDT) Received: by 10.236.200.194 with SMTP id z42mr1055548yhn.13.1348011363944; Tue, 18 Sep 2012 16:36:03 -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 l23si216810yhk.6.2012.09.18.16.36.03 (version=TLSv1/SSLv3 cipher=AES128-SHA); Tue, 18 Sep 2012 16:36:03 -0700 (PDT) Received: from hkchu.mtv.corp.google.com (hkchu.mtv.corp.google.com [172.18.96.139]) by wpzn4.hot.corp.google.com (Postfix) with ESMTP id B0D9F1E0048; Tue, 18 Sep 2012 16:36:03 -0700 (PDT) Received: by hkchu.mtv.corp.google.com (Postfix, from userid 19823) id 4EDA3D86DA; Tue, 18 Sep 2012 16:36:03 -0700 (PDT) From: "H.K. Jerry Chu" To: davem@davemloft.net, netdev@vger.kernel.org Cc: ncardwell@google.com, edumazet@google.com, Jerry Chu Subject: [PATCH] tcp: Fixed a TFO server bug that crashed kernel by raw sockets Date: Tue, 18 Sep 2012 16:35:51 -0700 Message-Id: <1348011351-13882-1-git-send-email-hkchu@google.com> X-Mailer: git-send-email 1.7.7.3 X-Gm-Message-State: ALoCoQlwvmUCBaBsHaeKwZnUGp7Yd+eDHOdrLnC9JiQ2ssxJlsyvxjsO6lYuArmgnExpHOeiSwN0T3VOJsExj//lD7VN5t7HUHuyVADThgAPfcFXhmnOthoLKVo7zam9EyFqbbwsOVq/RcWLf3ETKKojWglBWNQMdYqnVhSp5kAkiuflvGFJoCOoOpZgnNbgY422kmVwM0QF Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Jerry Chu Crash dump msg looks like this: <1>[34468.419809] BUG: unable to handle kernel paging request at ffffeb57000dc058 <1>[34468.426770] IP: [] kfree+0x4c/0x2d0 ... <4>[34468.603362] Call Trace: <4>[34468.605802] [] inet_sock_destruct+0x174/0x1f0 <4>[34468.611786] [] __sk_free+0x23/0x170 <4>[34468.616907] [] sk_free+0x25/0x30 <4>[34468.621762] [] sk_common_release+0x7a/0x80 <4>[34468.627481] [] raw_close+0x22/0x30 <4>[34468.632515] [] inet_release+0x58/0x90 <4>[34468.637802] [] sock_release+0x28/0x90 <4>[34468.643087] [] sock_close+0x17/0x30 <4>[34468.648203] [] fput+0xda/0x210 <4>[34468.652894] [] filp_close+0x66/0x90 <4>[34468.658016] [] put_files_struct+0x9d/0x120 <4>[34468.663743] [] exit_files+0x4a/0x60 <4>[34468.668857] [] do_exit+0x1a4/0x8c0 <4>[34468.673886] [] ? do_munmap+0x2ab/0x390 <4>[34468.679256] [] do_group_exit+0x44/0xa0 <4>[34468.684632] [] sys_exit_group+0x3f/0x50 <4>[34468.690094] [] sysenter_dispatch+0x7/0x1a This bug was introduced as part of patch 7ab4551f3b391818e29263279031dca1e26417c6 It turns out the call "socket(PF_INET/PF_INET6, SOCK_RAW, IPPROTO_TCP)" will cause a raw socket to be created with protocol == IPPROTO_TCP so checking against the protocol field in sk alone is not sufficient to guarantee a TCP socket. One must also check type == SOCK_STREAM. Signed-off-by: H.K. Jerry Chu Cc: Neal Cardwell Cc: Eric Dumazet --- net/ipv4/af_inet.c | 28 +++++++++++++++++----------- 1 files changed, 17 insertions(+), 11 deletions(-) diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 845372b..c9f0ea8 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -140,17 +140,22 @@ void inet_sock_destruct(struct sock *sk) sk_mem_reclaim(sk); - if (sk->sk_type == SOCK_STREAM && sk->sk_state != TCP_CLOSE) { - pr_err("Attempt to release TCP socket in state %d %p\n", - sk->sk_state, sk); - return; - } if (!sock_flag(sk, SOCK_DEAD)) { pr_err("Attempt to release alive inet socket %p\n", sk); return; } - if (sk->sk_protocol == IPPROTO_TCP) - kfree(inet_csk(sk)->icsk_accept_queue.fastopenq); + if (sk->sk_type == SOCK_STREAM) { + if (sk->sk_state != TCP_CLOSE) { + pr_err("Attempt to release TCP socket in state %d %p\n", + sk->sk_state, sk); + return; + } + /* Only TCP sockets (either v4 or v6) may have a valid + * icsk_accept_queue.fastopenq that must be freed. + */ + if (sk->sk_protocol == IPPROTO_TCP) + kfree(inet_csk(sk)->icsk_accept_queue.fastopenq); + } WARN_ON(atomic_read(&sk->sk_rmem_alloc)); WARN_ON(atomic_read(&sk->sk_wmem_alloc)); @@ -216,10 +221,11 @@ int inet_listen(struct socket *sock, int backlog) if (old_state != TCP_LISTEN) { /* Check special setups for testing purpose to enable TFO w/o * requiring TCP_FASTOPEN sockopt. - * Note that only TCP sockets (SOCK_STREAM) will reach here. - * Also fastopenq may already been allocated because this - * socket was in TCP_LISTEN state previously but was - * shutdown() (rather than close()). + * + * Note that only TCP sockets will reach here. Also fastopenq + * may already been allocated because this socket was in + * TCP_LISTEN state previously but was shutdown() (rather + * than close()). */ if ((sysctl_tcp_fastopen & TFO_SERVER_ENABLE) != 0 && inet_csk(sk)->icsk_accept_queue.fastopenq == NULL) {