Message ID | 20230725183646.5668-1-pchelkin@ispras.ru |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9010:0:b0:3e4:2afc:c1 with SMTP id l16csp2681389vqg; Tue, 25 Jul 2023 12:16:20 -0700 (PDT) X-Google-Smtp-Source: APBJJlFxpKvVyATfAWz8BJWP6lQTya4UILkFZ3TxsxokOeS1o4/hK0J87f1+aSGavlHvqprOFdn8 X-Received: by 2002:aa7:dcda:0:b0:51e:421e:d209 with SMTP id w26-20020aa7dcda000000b0051e421ed209mr156561edu.13.1690312580259; Tue, 25 Jul 2023 12:16:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690312580; cv=none; d=google.com; s=arc-20160816; b=U5z8DmLVXNLG++eL770QvaJpvbmH9MOyfvUv6UlRhTi+mwhtTvMu0dEmVoxYUWwmxp 54ndqjlZZ62Bg8rFUVteLa6kvN4FJjYUVgFgfhLrH2gu49i92fgPMRnymuG+rm90eHuB 0+UljdGsGNaKvTKs3lR0kJF8i/CQUn9Fcv/nONmsAy698GsSN0BFkJA/LrvA2n410Wrt ukN6UVa0ODVvQo650YsvvFlspu7S10QhcQMHUXbrn+p4FEtDNlz6+AE7Yi7ya/T5M7kK PTccz61H7LhUCHqnj17id4QbPlAPOY7ht0f1UexiwyM/+t3ShhFzf0YVmJsKwhFwc7AX 4AMg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature:dkim-filter; bh=Fab9GIRqttp0ydbdVKfgoFyEoFMbYfoPhmXpg6WhCHs=; fh=XbsocgPLcvDfwU+Z0mwjleoObAujf/3zPtX8MXqfgy8=; b=Q37JeoCYY2ItZY9tCQ2u8xWzS6qPIApeXHsefPm+oZ3JpHhXZ1XUpTdfdbTtQSsr/7 0dtRvYT4XcHYut/riMbf9xELe8Rw6aIdBzIHGnvH60y/sNzAHKT6wO45jjmW2rab1BfU 5paZdJY+dv2fTIS8Ju2YqC9HMF+XvA1D+m5vVrQNyhAfHNRhODXfeUALnDQh5nU/URuW NnRjkA1gheNbFR8M2HMinoiDnwqFHfZndZuENAy1l2DAcGy2NWYJ4jwX5YZtXiRoiyyC 6euOpedzx8DYVqLLcLOl+KWlt2d5bOg4i0tFB1vaxtP5MalZ1wOAtFqZJXroetRD4MEI HlLA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ispras.ru header.s=default header.b="ddcaE4I/"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=ispras.ru Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l4-20020a056402124400b005222bdf55e9si3509435edw.205.2023.07.25.12.15.53; Tue, 25 Jul 2023 12:16:20 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@ispras.ru header.s=default header.b="ddcaE4I/"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=ispras.ru Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231489AbjGYShq (ORCPT <rfc822;kautuk.consul.80@gmail.com> + 99 others); Tue, 25 Jul 2023 14:37:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43348 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230104AbjGYShp (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 25 Jul 2023 14:37:45 -0400 Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.84]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 92142A3; Tue, 25 Jul 2023 11:37:42 -0700 (PDT) Received: from localhost.localdomain (unknown [46.242.14.200]) by mail.ispras.ru (Postfix) with ESMTPSA id 2441A40737C9; Tue, 25 Jul 2023 18:37:40 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru 2441A40737C9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ispras.ru; s=default; t=1690310260; bh=Fab9GIRqttp0ydbdVKfgoFyEoFMbYfoPhmXpg6WhCHs=; h=From:To:Cc:Subject:Date:From; b=ddcaE4I/VzTy3aKJgfWeYWGmrOmkYBy5F4v+Tl+RBOJ4vPinRUv3me4AIANvpFpp6 qDUn/b4SzwGhpEM8P/5uR1E5bbFFMp/VVoRtPBnyF6iZQunyUm/M4jZ54Zx17SFmrI uQSU26BsLa6BiIW0ScfzrLMgyhKKX8SY13BqBhTI= From: Fedor Pchelkin <pchelkin@ispras.ru> To: Jon Maloy <jmaloy@redhat.com> Cc: Fedor Pchelkin <pchelkin@ispras.ru>, Ying Xue <ying.xue@windriver.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Xin Long <lucien.xin@gmail.com>, netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net, linux-kernel@vger.kernel.org, Alexey Khoroshilov <khoroshilov@ispras.ru>, lvc-project@linuxtesting.org Subject: [PATCH] tipc: stop tipc crypto on failure in tipc_node_create Date: Tue, 25 Jul 2023 21:36:44 +0300 Message-ID: <20230725183646.5668-1-pchelkin@ispras.ru> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772421204503593693 X-GMAIL-MSGID: 1772421204503593693 |
Series |
tipc: stop tipc crypto on failure in tipc_node_create
|
|
Commit Message
Fedor Pchelkin
July 25, 2023, 6:36 p.m. UTC
If tipc_link_bc_create() fails inside tipc_node_create() for a newly
allocated tipc node then we should stop its tipc crypto and free the
resources allocated with a call to tipc_crypto_start().
Call tipc_crypto_stop() in that case. Also extract the similar error exit
paths into a goto statement.
Found by Linux Verification Center (linuxtesting.org).
Fixes: cb8092d70a6f ("tipc: move bc link creation back to tipc_node_create")
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
net/tipc/node.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
Comments
On Tue, Jul 25, 2023 at 2:37 PM Fedor Pchelkin <pchelkin@ispras.ru> wrote: > > If tipc_link_bc_create() fails inside tipc_node_create() for a newly > allocated tipc node then we should stop its tipc crypto and free the > resources allocated with a call to tipc_crypto_start(). > > Call tipc_crypto_stop() in that case. Also extract the similar error exit > paths into a goto statement. > > Found by Linux Verification Center (linuxtesting.org). > > Fixes: cb8092d70a6f ("tipc: move bc link creation back to tipc_node_create") > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> > --- > net/tipc/node.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/net/tipc/node.c b/net/tipc/node.c > index 5e000fde8067..0d64005a803b 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -546,9 +546,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > #ifdef CONFIG_TIPC_CRYPTO > if (unlikely(tipc_crypto_start(&n->crypto_rx, net, n))) { > pr_warn("Failed to start crypto RX(%s)!\n", n->peer_id_string); > - kfree(n); > - n = NULL; > - goto exit; > + goto free_node; > } > #endif > n->addr = addr; > @@ -583,9 +581,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > n->capabilities, &n->bc_entry.inputq1, > &n->bc_entry.namedq, snd_l, &n->bc_entry.link)) { > pr_warn("Broadcast rcv link creation failed, no memory\n"); > - kfree(n); > - n = NULL; > - goto exit; > + goto stop_crypto; > } > tipc_node_get(n); Can you please try moving up tipc_node_get(n) ahead tipc_link_bc_create() and use tipc_node_put(n) to replace kfree(n) to avoid the extra tipc_crypto_stop() call below? Thanks. > timer_setup(&n->timer, tipc_node_timeout, 0); > @@ -610,6 +606,15 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > exit: > spin_unlock_bh(&tn->node_list_lock); > return n; > +stop_crypto: > + > +#ifdef CONFIG_TIPC_CRYPTO > + tipc_crypto_stop(&n->crypto_rx); > +free_node: > +#endif > + kfree(n); > + spin_unlock_bh(&tn->node_list_lock); > + return NULL; > } > > static void tipc_node_calculate_timer(struct tipc_node *n, struct tipc_link *l) > -- > 2.41.0 >
On 23/07/25 03:46PM, Xin Long wrote: > On Tue, Jul 25, 2023 at 2:37 PM Fedor Pchelkin <pchelkin@ispras.ru> wrote: > > > > If tipc_link_bc_create() fails inside tipc_node_create() for a newly > > allocated tipc node then we should stop its tipc crypto and free the > > resources allocated with a call to tipc_crypto_start(). > > > > Call tipc_crypto_stop() in that case. Also extract the similar error exit > > paths into a goto statement. > > > > Found by Linux Verification Center (linuxtesting.org). > > > > Fixes: cb8092d70a6f ("tipc: move bc link creation back to tipc_node_create") > > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> > > --- > > net/tipc/node.c | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/net/tipc/node.c b/net/tipc/node.c > > index 5e000fde8067..0d64005a803b 100644 > > --- a/net/tipc/node.c > > +++ b/net/tipc/node.c > > @@ -546,9 +546,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > > #ifdef CONFIG_TIPC_CRYPTO > > if (unlikely(tipc_crypto_start(&n->crypto_rx, net, n))) { > > pr_warn("Failed to start crypto RX(%s)!\n", n->peer_id_string); > > - kfree(n); > > - n = NULL; > > - goto exit; > > + goto free_node; > > } > > #endif > > n->addr = addr; > > @@ -583,9 +581,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > > n->capabilities, &n->bc_entry.inputq1, > > &n->bc_entry.namedq, snd_l, &n->bc_entry.link)) { > > pr_warn("Broadcast rcv link creation failed, no memory\n"); > > - kfree(n); > > - n = NULL; > > - goto exit; > > + goto stop_crypto; > > } > > tipc_node_get(n); > Can you please try moving up tipc_node_get(n) ahead tipc_link_bc_create() > and use tipc_node_put(n) to replace kfree(n) to avoid the extra > tipc_crypto_stop() call below? > > Thanks. > Guess moving tipc_node_get() before tipc_link_bc_create() would not solve the problem as ref is already initialized to 1 at that point. So just replacing direct kfree() with tipc_node_put() will fix it. Thank you for advice! I'll resend the v2 shortly. > > timer_setup(&n->timer, tipc_node_timeout, 0); > > @@ -610,6 +606,15 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > > exit: > > spin_unlock_bh(&tn->node_list_lock); > > return n; > > +stop_crypto: > > + > > +#ifdef CONFIG_TIPC_CRYPTO > > + tipc_crypto_stop(&n->crypto_rx); > > +free_node: > > +#endif > > + kfree(n); > > + spin_unlock_bh(&tn->node_list_lock); > > + return NULL; > > } > > > > static void tipc_node_calculate_timer(struct tipc_node *n, struct tipc_link *l) > > -- > > 2.41.0 > >
On Tue, Jul 25, 2023 at 5:46 PM Fedor Pchelkin <pchelkin@ispras.ru> wrote: > > If tipc_link_bc_create() fails inside tipc_node_create() for a newly > allocated tipc node then we should stop its tipc crypto and free the > resources allocated with a call to tipc_crypto_start(). > > As the node ref is initialized to one to that point, just put the ref on > tipc_link_bc_create() error case that would lead to tipc_node_free() be > eventually executed and properly clean the node and its crypto resources. > > Found by Linux Verification Center (linuxtesting.org). > > Fixes: cb8092d70a6f ("tipc: move bc link creation back to tipc_node_create") > Suggested-by: Xin Long <lucien.xin@gmail.com> > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> > --- > v1->v2: simplify the patch per Xin Long's advice: putting the ref on error > case would solve the problem more conveniently; update the patch > description accordingly. > > net/tipc/node.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/tipc/node.c b/net/tipc/node.c > index 5e000fde8067..a9c5b6594889 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -583,7 +583,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > n->capabilities, &n->bc_entry.inputq1, > &n->bc_entry.namedq, snd_l, &n->bc_entry.link)) { > pr_warn("Broadcast rcv link creation failed, no memory\n"); > - kfree(n); > + tipc_node_put(n); > n = NULL; > goto exit; > } > -- > 2.41.0 > Reviewed-by: Xin Long <lucien.xin@gmail.com>
diff --git a/net/tipc/node.c b/net/tipc/node.c index 5e000fde8067..0d64005a803b 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -546,9 +546,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, #ifdef CONFIG_TIPC_CRYPTO if (unlikely(tipc_crypto_start(&n->crypto_rx, net, n))) { pr_warn("Failed to start crypto RX(%s)!\n", n->peer_id_string); - kfree(n); - n = NULL; - goto exit; + goto free_node; } #endif n->addr = addr; @@ -583,9 +581,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, n->capabilities, &n->bc_entry.inputq1, &n->bc_entry.namedq, snd_l, &n->bc_entry.link)) { pr_warn("Broadcast rcv link creation failed, no memory\n"); - kfree(n); - n = NULL; - goto exit; + goto stop_crypto; } tipc_node_get(n); timer_setup(&n->timer, tipc_node_timeout, 0); @@ -610,6 +606,15 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, exit: spin_unlock_bh(&tn->node_list_lock); return n; +stop_crypto: + +#ifdef CONFIG_TIPC_CRYPTO + tipc_crypto_stop(&n->crypto_rx); +free_node: +#endif + kfree(n); + spin_unlock_bh(&tn->node_list_lock); + return NULL; } static void tipc_node_calculate_timer(struct tipc_node *n, struct tipc_link *l)