From patchwork Tue Jan 24 02:01:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Kerr X-Patchwork-Id: 13113284 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E6C75C54E94 for ; Tue, 24 Jan 2023 02:01:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231610AbjAXCBf (ORCPT ); Mon, 23 Jan 2023 21:01:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37062 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230062AbjAXCBa (ORCPT ); Mon, 23 Jan 2023 21:01:30 -0500 Received: from codeconstruct.com.au (pi.codeconstruct.com.au [203.29.241.158]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 708A99037 for ; Mon, 23 Jan 2023 18:01:29 -0800 (PST) Received: by codeconstruct.com.au (Postfix, from userid 10000) id 9856B2022A; Tue, 24 Jan 2023 10:01:24 +0800 (AWST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1674525684; bh=uYvpFQPZVj0rJFyVTs23uHbSWmVWuED+fLvuR/nFxUg=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=Ivmqiq/P/juXITmGP2s/x5yPOmuFfCMzvkHDDa3ZsqRpiXv5xDb/8QRecs4REWZAg hQjH1tculeb1QvEvrAloNtVpCkzxtqcLKeIaZHl5vShdZQo3FjXdRe1P8P9WSn4H4H 4eTjDCDykX9yA237o7Iwvaz7zFW/iWfAmgyNGhhpzK4+na3EmpGaJrf3+mJ8ExHP09 ySK4uWeriWCRTTLcTUi8xBxoX1pR92MSeNJl2TgajOAiMchOD7CNOC0SYzzGo0YqrN KHgNOqnQRSL9vvDDmBWAks3Yx020w5bB2sfhuohbvqu/LZi3gdrP1hpjZ4VtXVh428 a+fT9tXZY1yKQ== From: Jeremy Kerr To: netdev@vger.kernel.org Cc: Matt Johnston , Paolo Abeni , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Noam Rathaus Subject: [PATCH net 1/4] net: mctp: add an explicit reference from a mctp_sk_key to sock Date: Tue, 24 Jan 2023 10:01:03 +0800 Message-Id: <20230124020106.743966-2-jk@codeconstruct.com.au> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20230124020106.743966-1-jk@codeconstruct.com.au> References: <20230124020106.743966-1-jk@codeconstruct.com.au> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Currently, we correlate the mctp_sk_key lifetime to the sock lifetime through the sock hash/unhash operations, but this is pretty tenuous, and there are cases where we may have a temporary reference to an unhashed sk. This change makes the reference more explicit, by adding a hold on the sock when it's associated with a mctp_sk_key, released on final key unref. Fixes: 73c618456dc5 ("mctp: locking, lifetime and validity changes for sk_keys") Signed-off-by: Jeremy Kerr --- net/mctp/route.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/net/mctp/route.c b/net/mctp/route.c index f9a80b82dc51..ce10ba7ae839 100644 --- a/net/mctp/route.c +++ b/net/mctp/route.c @@ -147,6 +147,7 @@ static struct mctp_sk_key *mctp_key_alloc(struct mctp_sock *msk, key->valid = true; spin_lock_init(&key->lock); refcount_set(&key->refs, 1); + sock_hold(key->sk); return key; } @@ -165,6 +166,7 @@ void mctp_key_unref(struct mctp_sk_key *key) mctp_dev_release_key(key->dev, key); spin_unlock_irqrestore(&key->lock, flags); + sock_put(key->sk); kfree(key); } @@ -419,14 +421,14 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb) * this function. */ rc = mctp_key_add(key, msk); - if (rc) { - kfree(key); - } else { + if (!rc) trace_mctp_key_acquire(key); - /* we don't need to release key->lock on exit */ - mctp_key_unref(key); - } + /* we don't need to release key->lock on exit, so + * clean up here and suppress the unlock via + * setting to NULL + */ + mctp_key_unref(key); key = NULL; } else { From patchwork Tue Jan 24 02:01:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Kerr X-Patchwork-Id: 13113286 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5AA10C05027 for ; Tue, 24 Jan 2023 02:01:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231584AbjAXCBh (ORCPT ); Mon, 23 Jan 2023 21:01:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37066 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230181AbjAXCBb (ORCPT ); Mon, 23 Jan 2023 21:01:31 -0500 Received: from codeconstruct.com.au (pi.codeconstruct.com.au [203.29.241.158]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 722B8CDD6 for ; Mon, 23 Jan 2023 18:01:29 -0800 (PST) Received: by codeconstruct.com.au (Postfix, from userid 10000) id 0703C20261; Tue, 24 Jan 2023 10:01:25 +0800 (AWST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1674525685; bh=MmgVl6bENYLnjTNlm5rADiTwduNNn5ZNakeEDeKLv4k=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=DE4c1lAGtYbqPtc8yZdv4kyZR+nl3YahPG1/vlMDeIij4VbIYCrBcEZwkgtOEuIfE VCjhlOAgLnxcgYIHTm09RhYG1HfW+dIDruhrt1OAqKW5EzP0ZQVAE9nVRPln3QiXKe j/5bDDE2UP8JrYCkfexGLlOyNv3/Oisrc+m13KUtG0RYHP+2nawtrqVX8CWkvsvgA3 4VkLykpqTVRd0I/LS6p0RDAaeC+5DU92n1TCkPu4e+Hug69xsNyQgv1zx08rE8kyP5 KHmwEWAQRqDKRf+c82ZCZyFYsz/fHh0WuSSuRAugqhOpiZVln502Zq6UWZW3cXx45C 07AFHGxG2kACg== From: Jeremy Kerr To: netdev@vger.kernel.org Cc: Matt Johnston , Paolo Abeni , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Noam Rathaus Subject: [PATCH net 2/4] net: mctp: move expiry timer delete to unhash Date: Tue, 24 Jan 2023 10:01:04 +0800 Message-Id: <20230124020106.743966-3-jk@codeconstruct.com.au> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20230124020106.743966-1-jk@codeconstruct.com.au> References: <20230124020106.743966-1-jk@codeconstruct.com.au> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Currently, we delete the key expiry timer (in sk->close) before unhashing the sk. This means that another thread may find the sk through its presence on the key list, and re-queue the timer. This change moves the timer deletion to the unhash, after we have made the key no longer observable, so the timer cannot be re-queued. Fixes: 7b14e15ae6f4 ("mctp: Implement a timeout for tags") Signed-off-by: Jeremy Kerr --- net/mctp/af_mctp.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c index fc9e728b6333..fb6ae3110528 100644 --- a/net/mctp/af_mctp.c +++ b/net/mctp/af_mctp.c @@ -544,9 +544,6 @@ static int mctp_sk_init(struct sock *sk) static void mctp_sk_close(struct sock *sk, long timeout) { - struct mctp_sock *msk = container_of(sk, struct mctp_sock, sk); - - del_timer_sync(&msk->key_expiry); sk_common_release(sk); } @@ -581,6 +578,12 @@ static void mctp_sk_unhash(struct sock *sk) __mctp_key_remove(key, net, fl2, MCTP_TRACE_KEY_CLOSED); } spin_unlock_irqrestore(&net->mctp.keys_lock, flags); + + /* Since there are no more tag allocations (we have removed all of the + * keys), stop any pending expiry events. the timer cannot be re-queued + * as the sk is no longer observable + */ + del_timer_sync(&msk->key_expiry); } static struct proto mctp_proto = { From patchwork Tue Jan 24 02:01:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Kerr X-Patchwork-Id: 13113287 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B43A8C54EB4 for ; Tue, 24 Jan 2023 02:01:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230181AbjAXCBi (ORCPT ); Mon, 23 Jan 2023 21:01:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37070 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231511AbjAXCBd (ORCPT ); Mon, 23 Jan 2023 21:01:33 -0500 Received: from codeconstruct.com.au (pi.codeconstruct.com.au [203.29.241.158]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 726CC11161 for ; Mon, 23 Jan 2023 18:01:29 -0800 (PST) Received: by codeconstruct.com.au (Postfix, from userid 10000) id 68D5C2036E; Tue, 24 Jan 2023 10:01:25 +0800 (AWST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1674525685; bh=AyLhRsZxXzbiDViZIVo9hcCIXcOxqV0TYtP/V49oCPM=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=CtN7QNx86XJNkQOLVTbALOUMx8U6GSpQY4ohi/1ApUmiMkPAopeUDQTREgGbgsAsM 5ID383t49TEyNYcvh1X4QWJ2sNImWyuQsW6rr8TA+q6oC4Abd4nWPwE6XA0Ysi7qDb fRMFFJpNNY0Pplf8jrGtpn/lfn8eLcW+ylh+R4wfXaJxecJnyjEVHxAoNqVz7TwqRE PfESLSD+1jMHX7jsCzcIkRZ9THJSU3SYUBfJNNNURjy9qYOh9cy7WLnTnNr2kQr4Aj ZsyWUlx5taIHW9QbIP9inUBSTG2tSsjQ3TxbQU1FdF3VsYx43R+8WUdgoUgporbmLx oJxuhlo7ntBNw== From: Jeremy Kerr To: netdev@vger.kernel.org Cc: Matt Johnston , Paolo Abeni , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Noam Rathaus Subject: [PATCH net 3/4] net: mctp: hold key reference when looking up a general key Date: Tue, 24 Jan 2023 10:01:05 +0800 Message-Id: <20230124020106.743966-4-jk@codeconstruct.com.au> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20230124020106.743966-1-jk@codeconstruct.com.au> References: <20230124020106.743966-1-jk@codeconstruct.com.au> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org From: Paolo Abeni Currently, we have a race where we look up a sock through a "general" (ie, not directly associated with the (src,dest,tag) tuple) key, then drop the key reference while still holding the key's sock. This change expands the key reference until we've finished using the sock, and hence the sock reference too. Commit message changes from Jeremy Kerr . Reported-by: Noam Rathaus Fixes: 73c618456dc5 ("mctp: locking, lifetime and validity changes for sk_keys") Signed-off-by: Paolo Abeni Signed-off-by: Jeremy Kerr --- net/mctp/route.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/net/mctp/route.c b/net/mctp/route.c index ce10ba7ae839..06c0de21984d 100644 --- a/net/mctp/route.c +++ b/net/mctp/route.c @@ -317,8 +317,8 @@ static int mctp_frag_queue(struct mctp_sk_key *key, struct sk_buff *skb) static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb) { + struct mctp_sk_key *key, *any_key = NULL; struct net *net = dev_net(skb->dev); - struct mctp_sk_key *key; struct mctp_sock *msk; struct mctp_hdr *mh; unsigned long f; @@ -363,13 +363,11 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb) * key for reassembly - we'll create a more specific * one for future packets if required (ie, !EOM). */ - key = mctp_lookup_key(net, skb, MCTP_ADDR_ANY, &f); - if (key) { - msk = container_of(key->sk, + any_key = mctp_lookup_key(net, skb, MCTP_ADDR_ANY, &f); + if (any_key) { + msk = container_of(any_key->sk, struct mctp_sock, sk); - spin_unlock_irqrestore(&key->lock, f); - mctp_key_unref(key); - key = NULL; + spin_unlock_irqrestore(&any_key->lock, f); } } @@ -475,6 +473,8 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb) spin_unlock_irqrestore(&key->lock, f); mctp_key_unref(key); } + if (any_key) + mctp_key_unref(any_key); out: if (rc) kfree_skb(skb); From patchwork Tue Jan 24 02:01:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Kerr X-Patchwork-Id: 13113285 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C10F0C25B50 for ; Tue, 24 Jan 2023 02:01:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231618AbjAXCBg (ORCPT ); Mon, 23 Jan 2023 21:01:36 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37068 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231228AbjAXCBb (ORCPT ); Mon, 23 Jan 2023 21:01:31 -0500 Received: from codeconstruct.com.au (pi.codeconstruct.com.au [203.29.241.158]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 725BB10AA0 for ; Mon, 23 Jan 2023 18:01:29 -0800 (PST) Received: by codeconstruct.com.au (Postfix, from userid 10000) id BFBA521688; Tue, 24 Jan 2023 10:01:25 +0800 (AWST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1674525685; bh=DCmSZpGbUwHzMdBsEUb1P7R+nEn7Pf7UaTqxtDsphAw=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=Tf7tU13tcgWNKLjjStI+PiFiAfGd+mI/1oHXDyw6ruQlinuCZA7nP3/5qmAAqpv+k LMOHdx3w3Sm+9vTFwWkNzGf7vKeLVOtgDrE10lrzaEcWNYgMOBlIFZE29eBDkPAguo J2K8d1AL4Fu/q8fpIji4M6tG6PQeyR11imhQ88TKU/gt1U0ZCvHdMrxsRMFB3VuNpd sM5nVkbKmwhWczFiZRSBnnm74R8Lyzsecd4KIZhxo15iusB0qdqBtS9QG7KeF/g2vG fKBL6pMCZOl7v5daf76FALrTqfBUQ/KOA4+2NxKtvliu1eTQgUoWMKCo4HTwuHJ/hu IP/n4Oss4042A== From: Jeremy Kerr To: netdev@vger.kernel.org Cc: Matt Johnston , Paolo Abeni , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Noam Rathaus Subject: [PATCH net 4/4] net: mctp: mark socks as dead on unhash, prevent re-add Date: Tue, 24 Jan 2023 10:01:06 +0800 Message-Id: <20230124020106.743966-5-jk@codeconstruct.com.au> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20230124020106.743966-1-jk@codeconstruct.com.au> References: <20230124020106.743966-1-jk@codeconstruct.com.au> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Once a socket has been unhashed, we want to prevent it from being re-used in a sk_key entry as part of a routing operation. This change marks the sk as SOCK_DEAD on unhash, which prevents addition into the net's key list. We need to do this during the key add path, rather than key lookup, as we release the net keys_lock between those operations. Fixes: 4a992bbd3650 ("mctp: Implement message fragmentation & reassembly") Signed-off-by: Jeremy Kerr --- net/mctp/af_mctp.c | 1 + net/mctp/route.c | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c index fb6ae3110528..45bbe3e54cc2 100644 --- a/net/mctp/af_mctp.c +++ b/net/mctp/af_mctp.c @@ -577,6 +577,7 @@ static void mctp_sk_unhash(struct sock *sk) spin_lock_irqsave(&key->lock, fl2); __mctp_key_remove(key, net, fl2, MCTP_TRACE_KEY_CLOSED); } + sock_set_flag(sk, SOCK_DEAD); spin_unlock_irqrestore(&net->mctp.keys_lock, flags); /* Since there are no more tag allocations (we have removed all of the diff --git a/net/mctp/route.c b/net/mctp/route.c index 06c0de21984d..f51a05ec7162 100644 --- a/net/mctp/route.c +++ b/net/mctp/route.c @@ -179,6 +179,11 @@ static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk) spin_lock_irqsave(&net->mctp.keys_lock, flags); + if (sock_flag(&msk->sk, SOCK_DEAD)) { + rc = -EINVAL; + goto out_unlock; + } + hlist_for_each_entry(tmp, &net->mctp.keys, hlist) { if (mctp_key_match(tmp, key->local_addr, key->peer_addr, key->tag)) { @@ -200,6 +205,7 @@ static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk) hlist_add_head(&key->sklist, &msk->keys); } +out_unlock: spin_unlock_irqrestore(&net->mctp.keys_lock, flags); return rc;