All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Wei Yongjun <weiyongjun1@huawei.com>,
	Rohit Maheshwari <rohitm@chelsio.com>
Cc: Ayush Sawal <ayush.sawal@chelsio.com>,
	Vinay Kumar Yadav <vinay.yadav@chelsio.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	linux-crypto@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Hulk Robot <hulkci@huawei.com>
Subject: [PATCH] cxgb4/chcr: Fix a leak in chcr_ktls_dev_add()
Date: Tue, 12 May 2020 08:04:36 +0000	[thread overview]
Message-ID: <20200512080436.GA247819@mwanda> (raw)
In-Reply-To: <20200509080548.118667-1-weiyongjun1@huawei.com>

We need to free "tx_info->l2te" if chcr_setup_connection() fails.  My
other concern was that if we free "tx_info" then "tx_ctx->chcr_info"
points to a freed variable.  I don't think this causes a problem but
it's cleaner to reset it back to NULL.  Also I renamed the labels to
say what the gotos do instead of using numbered labels.

Fixes: 34aba2c45024 ("cxgb4/chcr : Register to tls add and del callback")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Applies on top of Wei Yongjun's patch.

 drivers/crypto/chelsio/chcr_ktls.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/chelsio/chcr_ktls.c b/drivers/crypto/chelsio/chcr_ktls.c
index baaea8ce40806..3173ac3099bc6 100644
--- a/drivers/crypto/chelsio/chcr_ktls.c
+++ b/drivers/crypto/chelsio/chcr_ktls.c
@@ -478,7 +478,7 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 
 	tx_info->rx_qid = chcr_get_first_rx_qid(adap);
 	if (unlikely(tx_info->rx_qid < 0))
-		goto out2;
+		goto free_tx_info;
 
 	tx_info->prev_seq = start_offload_tcp_sn;
 	tx_info->tcp_start_seq_number = start_offload_tcp_sn;
@@ -486,7 +486,7 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	/* save crypto keys */
 	ret = chcr_ktls_save_keys(tx_info, crypto_info, direction);
 	if (ret < 0)
-		goto out2;
+		goto free_tx_info;
 
 	/* get peer ip */
 	if (sk->sk_family = AF_INET ||
@@ -502,14 +502,14 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	if (!dst) {
 		pr_err("DST entry not found\n");
 		ret = -ENOENT;
-		goto out2;
+		goto free_tx_info;
 	}
 	n = dst_neigh_lookup(dst, daaddr);
 	if (!n || !n->dev) {
 		pr_err("neighbour not found\n");
 		dst_release(dst);
 		ret = -ENOENT;
-		goto out2;
+		goto free_tx_info;
 	}
 	tx_info->l2te  = cxgb4_l2t_get(adap->l2t, n, n->dev, 0);
 
@@ -519,7 +519,7 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	if (!tx_info->l2te) {
 		pr_err("l2t entry not found\n");
 		ret = -ENOENT;
-		goto out2;
+		goto free_tx_info;
 	}
 
 	tx_ctx->chcr_info = tx_info;
@@ -529,12 +529,16 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	 */
 	ret = chcr_setup_connection(sk, tx_info);
 	if (ret)
-		goto out2;
+		goto free_l2te;
 
 	atomic64_inc(&adap->chcr_stats.ktls_tx_connection_open);
 	return 0;
-out2:
+
+free_l2te:
+	cxgb4_l2t_release(tx_info->l2te);
+free_tx_info:
 	kvfree(tx_info);
+	tx_ctx->chcr_info = NULL;
 out:
 	atomic64_inc(&adap->chcr_stats.ktls_tx_connection_fail);
 	return ret;
-- 
2.26.2

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Wei Yongjun <weiyongjun1@huawei.com>,
	Rohit Maheshwari <rohitm@chelsio.com>
Cc: Ayush Sawal <ayush.sawal@chelsio.com>,
	Vinay Kumar Yadav <vinay.yadav@chelsio.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Wei Yongjun <weiyongjun1@huawei.com>,
	linux-crypto@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Hulk Robot <hulkci@huawei.com>
Subject: [PATCH] cxgb4/chcr: Fix a leak in chcr_ktls_dev_add()
Date: Tue, 12 May 2020 11:04:36 +0300	[thread overview]
Message-ID: <20200512080436.GA247819@mwanda> (raw)
In-Reply-To: <20200509080548.118667-1-weiyongjun1@huawei.com>

We need to free "tx_info->l2te" if chcr_setup_connection() fails.  My
other concern was that if we free "tx_info" then "tx_ctx->chcr_info"
points to a freed variable.  I don't think this causes a problem but
it's cleaner to reset it back to NULL.  Also I renamed the labels to
say what the gotos do instead of using numbered labels.

Fixes: 34aba2c45024 ("cxgb4/chcr : Register to tls add and del callback")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Applies on top of Wei Yongjun's patch.

 drivers/crypto/chelsio/chcr_ktls.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/chelsio/chcr_ktls.c b/drivers/crypto/chelsio/chcr_ktls.c
index baaea8ce40806..3173ac3099bc6 100644
--- a/drivers/crypto/chelsio/chcr_ktls.c
+++ b/drivers/crypto/chelsio/chcr_ktls.c
@@ -478,7 +478,7 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 
 	tx_info->rx_qid = chcr_get_first_rx_qid(adap);
 	if (unlikely(tx_info->rx_qid < 0))
-		goto out2;
+		goto free_tx_info;
 
 	tx_info->prev_seq = start_offload_tcp_sn;
 	tx_info->tcp_start_seq_number = start_offload_tcp_sn;
@@ -486,7 +486,7 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	/* save crypto keys */
 	ret = chcr_ktls_save_keys(tx_info, crypto_info, direction);
 	if (ret < 0)
-		goto out2;
+		goto free_tx_info;
 
 	/* get peer ip */
 	if (sk->sk_family == AF_INET ||
@@ -502,14 +502,14 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	if (!dst) {
 		pr_err("DST entry not found\n");
 		ret = -ENOENT;
-		goto out2;
+		goto free_tx_info;
 	}
 	n = dst_neigh_lookup(dst, daaddr);
 	if (!n || !n->dev) {
 		pr_err("neighbour not found\n");
 		dst_release(dst);
 		ret = -ENOENT;
-		goto out2;
+		goto free_tx_info;
 	}
 	tx_info->l2te  = cxgb4_l2t_get(adap->l2t, n, n->dev, 0);
 
@@ -519,7 +519,7 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	if (!tx_info->l2te) {
 		pr_err("l2t entry not found\n");
 		ret = -ENOENT;
-		goto out2;
+		goto free_tx_info;
 	}
 
 	tx_ctx->chcr_info = tx_info;
@@ -529,12 +529,16 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	 */
 	ret = chcr_setup_connection(sk, tx_info);
 	if (ret)
-		goto out2;
+		goto free_l2te;
 
 	atomic64_inc(&adap->chcr_stats.ktls_tx_connection_open);
 	return 0;
-out2:
+
+free_l2te:
+	cxgb4_l2t_release(tx_info->l2te);
+free_tx_info:
 	kvfree(tx_info);
+	tx_ctx->chcr_info = NULL;
 out:
 	atomic64_inc(&adap->chcr_stats.ktls_tx_connection_fail);
 	return ret;
-- 
2.26.2


  reply	other threads:[~2020-05-12  8:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-09  8:05 [PATCH -next] cxgb4/chcr: Fix error return code in chcr_ktls_dev_add() Wei Yongjun
2020-05-09  8:05 ` Wei Yongjun
2020-05-12  8:04 ` Dan Carpenter [this message]
2020-05-12  8:04   ` [PATCH] cxgb4/chcr: Fix a leak " Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200512080436.GA247819@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=ayush.sawal@chelsio.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=hulkci@huawei.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=rohitm@chelsio.com \
    --cc=vinay.yadav@chelsio.com \
    --cc=weiyongjun1@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.