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
next prev parent 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.