From patchwork Fri May 29 15:43:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Abeni X-Patchwork-Id: 218198 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, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS autolearn=ham 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 C5F6AC433E0 for ; Fri, 29 May 2020 15:44:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A5D022077D for ; Fri, 29 May 2020 15:44:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="JlN2OsWV" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728048AbgE2PoQ (ORCPT ); Fri, 29 May 2020 11:44:16 -0400 Received: from us-smtp-2.mimecast.com ([205.139.110.61]:40189 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727999AbgE2PoO (ORCPT ); Fri, 29 May 2020 11:44:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1590767053; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EJzoQCGf3gj3i2w4aCrFG+qmTAW4YNGEnvSifCYpi9A=; b=JlN2OsWV9BbtyQ3ti89dv+4SH52JuBfkqGUg9P624RGi7K0dqS/Co1jLXuUAjyMpgu5iFJ UqbHTKst8L3ae5YpUEFgHv0mCk4pV8nNycd2XUzri/6QZ1llfVJewQeJBIVLuBn2eofcJN edMZoaQ39Bn6EDy+KxvhNGwMVXG7Bpo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-318-RYRNEa_SOIS3pfRMCVgb2w-1; Fri, 29 May 2020 11:44:08 -0400 X-MC-Unique: RYRNEa_SOIS3pfRMCVgb2w-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3BD5C835B40; Fri, 29 May 2020 15:44:07 +0000 (UTC) Received: from linux.fritz.box.com (ovpn-114-94.ams2.redhat.com [10.36.114.94]) by smtp.corp.redhat.com (Postfix) with ESMTP id 071117A8C0; Fri, 29 May 2020 15:44:05 +0000 (UTC) From: Paolo Abeni To: netdev@vger.kernel.org Cc: Mat Martineau , "David S. Miller" , Jakub Kicinski Subject: [PATCH net 2/3] mptcp: fix race between MP_JOIN and close Date: Fri, 29 May 2020 17:43:30 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org If a MP_JOIN subflow completes the 3whs while another CPU is closing the master msk, we can hit the following race: CPU1 CPU2 close() mptcp_close subflow_syn_recv_sock mptcp_token_get_sock mptcp_finish_join inet_sk_state_load mptcp_token_destroy inet_sk_state_store(TCP_CLOSE) __mptcp_flush_join_list() mptcp_sock_graft list_add_tail sk_common_release sock_orphan() The MP_JOIN socket will be leaked. Additionally we can hit UaF for the msk 'struct socket' referenced via the 'conn' field. This change try to address the issue introducing some synchronization between the MP_JOIN 3whs and mptcp_close via the join_list spinlock. If we detect the msk is closing the MP_JOIN socket is closed, too. Fixes: f296234c98a8 ("mptcp: Add handling of incoming MP_JOIN requests") Signed-off-by: Paolo Abeni --- net/mptcp/protocol.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 8959a74f707d..35bdfb4f3eae 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -1266,8 +1266,12 @@ static void mptcp_close(struct sock *sk, long timeout) mptcp_token_destroy(msk->token); inet_sk_state_store(sk, TCP_CLOSE); - __mptcp_flush_join_list(msk); - + /* be sure to always acquire the join list lock, to sync vs + * mptcp_finish_join(). + */ + spin_lock_bh(&msk->join_list_lock); + list_splice_tail_init(&msk->join_list, &msk->conn_list); + spin_unlock_bh(&msk->join_list_lock); list_splice_init(&msk->conn_list, &conn_list); data_fin_tx_seq = msk->write_seq; @@ -1623,22 +1627,30 @@ bool mptcp_finish_join(struct sock *sk) if (!msk->pm.server_side) return true; - /* passive connection, attach to msk socket */ + if (!mptcp_pm_allow_new_subflow(msk)) + return false; + + /* active connections are already on conn_list, and we can't acquire + * msk lock here. + * use the join list lock as synchronization point and double-check + * msk status to avoid racing with mptcp_close() + */ + spin_lock_bh(&msk->join_list_lock); + ret = inet_sk_state_load(parent) == TCP_ESTABLISHED; + if (ret && !WARN_ON_ONCE(!list_empty(&subflow->node))) + list_add_tail(&subflow->node, &msk->join_list); + spin_unlock_bh(&msk->join_list_lock); + if (!ret) + return false; + + /* attach to msk socket only after we are sure he will deal with us + * at close time + */ parent_sock = READ_ONCE(parent->sk_socket); if (parent_sock && !sk->sk_socket) mptcp_sock_graft(sk, parent_sock); - - ret = mptcp_pm_allow_new_subflow(msk); - if (ret) { - subflow->map_seq = msk->ack_seq; - - /* active connections are already on conn_list */ - spin_lock_bh(&msk->join_list_lock); - if (!WARN_ON_ONCE(!list_empty(&subflow->node))) - list_add_tail(&subflow->node, &msk->join_list); - spin_unlock_bh(&msk->join_list_lock); - } - return ret; + subflow->map_seq = msk->ack_seq; + return true; } bool mptcp_sk_is_subflow(const struct sock *sk) From patchwork Fri May 29 15:43:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Abeni X-Patchwork-Id: 218199 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, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS autolearn=ham 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 35140C433DF for ; Fri, 29 May 2020 15:44:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 07F7A207BC for ; Fri, 29 May 2020 15:44:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="D2t5xL1J" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728041AbgE2PoO (ORCPT ); Fri, 29 May 2020 11:44:14 -0400 Received: from us-smtp-2.mimecast.com ([205.139.110.61]:56409 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726940AbgE2PoM (ORCPT ); Fri, 29 May 2020 11:44:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1590767051; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5AD3HnP3A68DFj5y3YNpEEwK2K02DYm/1aEunRukBJ4=; b=D2t5xL1J6bRPTpG4Pe+pYnwr84qDRNYCMAIIAZRRNvGQA4rWr1vvSQjKZmDLPi6jzrQQe7 ljd7Hj6fNRVmy6++8ZP7WNnVFskKhb8zeFFiM1dLO2nbZvs/aUmYbAbuI6xb9l4ZuUPWWR hkf1QQ6abK3mEHPOpiSbr3phwDVrqLA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-168-sxFHK8I_MwWO1rF_wr2qTw-1; Fri, 29 May 2020 11:44:09 -0400 X-MC-Unique: sxFHK8I_MwWO1rF_wr2qTw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C9A3F19057A9; Fri, 29 May 2020 15:44:08 +0000 (UTC) Received: from linux.fritz.box.com (ovpn-114-94.ams2.redhat.com [10.36.114.94]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9836C7A8BA; Fri, 29 May 2020 15:44:07 +0000 (UTC) From: Paolo Abeni To: netdev@vger.kernel.org Cc: Mat Martineau , "David S. Miller" , Jakub Kicinski Subject: [PATCH net 3/3] mptcp: remove msk from the token container at destruction time. Date: Fri, 29 May 2020 17:43:31 +0200 Message-Id: <73105e38dc7e9153dc3b58a3c4ccc59de3a10947.1590766645.git.pabeni@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Currently we remote the msk from the token container only via mptcp_close(). The MPTCP master socket can be destroyed also via other paths (e.g. if not yet accepted, when shutting down the listener socket). When we hit the latter scenario, dangling msk references are left into the token container, leading to memory corruption and/or UaF. This change addresses the issue by moving the token removal into the msk destructor. Fixes: 79c0949e9a09 ("mptcp: Add key generation and token tree") Signed-off-by: Paolo Abeni --- net/mptcp/protocol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 35bdfb4f3eae..34dd0e278a82 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -1263,7 +1263,6 @@ static void mptcp_close(struct sock *sk, long timeout) lock_sock(sk); - mptcp_token_destroy(msk->token); inet_sk_state_store(sk, TCP_CLOSE); /* be sure to always acquire the join list lock, to sync vs @@ -1461,6 +1460,7 @@ static void mptcp_destroy(struct sock *sk) { struct mptcp_sock *msk = mptcp_sk(sk); + mptcp_token_destroy(msk->token); if (msk->cached_ext) __skb_ext_put(msk->cached_ext);