* [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
@ 2022-11-22 4:49 Zhang Qilong
2022-11-22 9:44 ` Sagi Grimberg
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Zhang Qilong @ 2022-11-22 4:49 UTC (permalink / raw)
To: sagi, hare, kbusch, hch, axboe; +Cc: linux-nvme
If dhchap_secret is not consistent with options or
nvme_auth_generate_key() fails, we should free the
memory of dhchap_secret to avoid memleak.
We fix it by changing the check strcmp to strncmp
directly against buf and moving allocating inside
to the clause.
Fixes: f50fff73d620 ("nvme: implement In-Band authentication")
Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
---
v2:
- changing checking function and allocating location.
---
drivers/nvme/host/core.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index da55ce45ac70..bfec2a63d0d8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3730,7 +3730,6 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
{
struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
struct nvmf_ctrl_options *opts = ctrl->opts;
- char *dhchap_secret;
if (!ctrl->opts->dhchap_secret)
return -EINVAL;
@@ -3739,17 +3738,19 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
if (memcmp(buf, "DHHC-1:", 7))
return -EINVAL;
- dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
- if (!dhchap_secret)
- return -ENOMEM;
- memcpy(dhchap_secret, buf, count);
nvme_auth_stop(ctrl);
- if (strcmp(dhchap_secret, opts->dhchap_secret)) {
+ if (strncmp(buf, opts->dhchap_secret, count)) {
int ret;
+ char *dhchap_secret;
+ dhchap_secret = kmemdup(buf);
+ if (!dhchap_secret)
+ return -ENOMEM;
ret = nvme_auth_generate_key(dhchap_secret, &ctrl->host_key);
- if (ret)
+ if (ret) {
+ kfree(dhchap_secret);
return ret;
+ }
kfree(opts->dhchap_secret);
opts->dhchap_secret = dhchap_secret;
/* Key has changed; re-authentication with new key */
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
2022-11-22 4:49 [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store() Zhang Qilong
@ 2022-11-22 9:44 ` Sagi Grimberg
2022-11-29 4:29 ` Chaitanya Kulkarni
2022-11-29 8:47 ` Christoph Hellwig
2 siblings, 0 replies; 9+ messages in thread
From: Sagi Grimberg @ 2022-11-22 9:44 UTC (permalink / raw)
To: Zhang Qilong, hare, kbusch, hch, axboe; +Cc: linux-nvme
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
2022-11-22 4:49 [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store() Zhang Qilong
2022-11-22 9:44 ` Sagi Grimberg
@ 2022-11-29 4:29 ` Chaitanya Kulkarni
2022-11-29 8:47 ` Christoph Hellwig
2 siblings, 0 replies; 9+ messages in thread
From: Chaitanya Kulkarni @ 2022-11-29 4:29 UTC (permalink / raw)
To: Zhang Qilong, sagi, hare, kbusch, hch, axboe; +Cc: linux-nvme
On 11/21/22 20:49, Zhang Qilong wrote:
> If dhchap_secret is not consistent with options or
> nvme_auth_generate_key() fails, we should free the
> memory of dhchap_secret to avoid memleak.
>
> We fix it by changing the check strcmp to strncmp
> directly against buf and moving allocating inside
> to the clause.
>
> Fixes: f50fff73d620 ("nvme: implement In-Band authentication")
> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
> ---
> v2:
> - changing checking function and allocating location.
> ---
>
Looks good.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
2022-11-22 4:49 [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store() Zhang Qilong
2022-11-22 9:44 ` Sagi Grimberg
2022-11-29 4:29 ` Chaitanya Kulkarni
@ 2022-11-29 8:47 ` Christoph Hellwig
2022-11-29 13:40 ` Christoph Hellwig
2 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2022-11-29 8:47 UTC (permalink / raw)
To: Zhang Qilong; +Cc: sagi, hare, kbusch, hch, axboe, linux-nvme
Thanks,
applied to nvme-6.1.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
2022-11-29 8:47 ` Christoph Hellwig
@ 2022-11-29 13:40 ` Christoph Hellwig
2022-11-30 3:49 ` 答复: " zhangqilong
2022-11-30 8:37 ` Sagi Grimberg
0 siblings, 2 replies; 9+ messages in thread
From: Christoph Hellwig @ 2022-11-29 13:40 UTC (permalink / raw)
To: Zhang Qilong; +Cc: sagi, hare, kbusch, hch, axboe, linux-nvme
On Tue, Nov 29, 2022 at 09:47:25AM +0100, Christoph Hellwig wrote:
> Thanks,
>
> applied to nvme-6.1.
And I had to fix it to actually pass all the correct paramters to
kmemdup, so it looks like this code wasn't even compile test.
Btw, how can nvme_ctrl_dhchap_secret_store work without any kind
of locking?
^ permalink raw reply [flat|nested] 9+ messages in thread
* 答复: [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
2022-11-29 13:40 ` Christoph Hellwig
@ 2022-11-30 3:49 ` zhangqilong
2022-11-30 8:37 ` Sagi Grimberg
1 sibling, 0 replies; 9+ messages in thread
From: zhangqilong @ 2022-11-30 3:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sagi, hare, kbusch, axboe, linux-nvme
>
> On Tue, Nov 29, 2022 at 09:47:25AM +0100, Christoph Hellwig wrote:
> > Thanks,
> >
> > applied to nvme-6.1.
>
> And I had to fix it to actually pass all the correct paramters to kmemdup, so it
> looks like this code wasn't even compile test.
>
Oh,
It seems that I forget to do it for V2 version. I am sorry to it, It is so nice that you fix it.
> Btw, how can nvme_ctrl_dhchap_secret_store work without any kind of
> locking?
You means it should need a lock to avoid race condition?
Thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
2022-11-29 13:40 ` Christoph Hellwig
2022-11-30 3:49 ` 答复: " zhangqilong
@ 2022-11-30 8:37 ` Sagi Grimberg
2022-11-30 13:37 ` Christoph Hellwig
1 sibling, 1 reply; 9+ messages in thread
From: Sagi Grimberg @ 2022-11-30 8:37 UTC (permalink / raw)
To: Christoph Hellwig, Zhang Qilong; +Cc: hare, kbusch, axboe, linux-nvme
>> Thanks,
>>
>> applied to nvme-6.1.
>
> And I had to fix it to actually pass all the correct paramters to
> kmemdup, so it looks like this code wasn't even compile test.
>
> Btw, how can nvme_ctrl_dhchap_secret_store work without any kind
> of locking?
This is fixed in 6.2 with some of my patches.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
2022-11-30 8:37 ` Sagi Grimberg
@ 2022-11-30 13:37 ` Christoph Hellwig
2022-12-01 1:57 ` 答复: " zhangqilong
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2022-11-30 13:37 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Christoph Hellwig, Zhang Qilong, hare, kbusch, axboe, linux-nvme
On Wed, Nov 30, 2022 at 10:37:45AM +0200, Sagi Grimberg wrote:
> This is fixed in 6.2 with some of my patches.
And this version conflicts with that.
Zhang, can you please resend a fully tested version against the
nvme-6.2 branch? Thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
* 答复: [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store()
2022-11-30 13:37 ` Christoph Hellwig
@ 2022-12-01 1:57 ` zhangqilong
0 siblings, 0 replies; 9+ messages in thread
From: zhangqilong @ 2022-12-01 1:57 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg; +Cc: hare, kbusch, axboe, linux-nvme
> On Wed, Nov 30, 2022 at 10:37:45AM +0200, Sagi Grimberg wrote:
> > This is fixed in 6.2 with some of my patches.
>
> And this version conflicts with that.
>
> Zhang, can you please resend a fully tested version against the
> nvme-6.2 branch? Thanks!
It is my pleasure to do that.
I will send a tested version to nvme-6.2 branch!
Thanks,
Zhang
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-12-01 1:57 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-22 4:49 [PATCH v2] nvme: fix memleak in nvme_ctrl_dhchap_secret_store() Zhang Qilong
2022-11-22 9:44 ` Sagi Grimberg
2022-11-29 4:29 ` Chaitanya Kulkarni
2022-11-29 8:47 ` Christoph Hellwig
2022-11-29 13:40 ` Christoph Hellwig
2022-11-30 3:49 ` 答复: " zhangqilong
2022-11-30 8:37 ` Sagi Grimberg
2022-11-30 13:37 ` Christoph Hellwig
2022-12-01 1:57 ` 答复: " zhangqilong
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.