2018-01-25 08:11:26

by Johannes Thumshirn

Subject: [PATCH] nvme: don't free uuid pointer before printing it

Commit df351ef73789 ("nvme-fabrics: fix memory leak when parsing host ID
option") fixed the leak of 'p' but in case uuid_parse() fails the memory
is freed before the error print that is using it.

Free it after printing eventual errors.

Signed-off-by: Johannes Thumshirn <[email protected]>
Fixes: df351ef73789 ("nvme-fabrics: fix memory leak when parsing host ID option")
Reported-by: Dan Carpenter <[email protected]>
Cc: Roland Dreier <[email protected]>
---
drivers/nvme/host/fabrics.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index eb46967bb0d5..9cee72a80472 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -739,12 +739,13 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
goto out;
}
ret = uuid_parse(p, &hostid);
- kfree(p);
if (ret) {
pr_err("Invalid hostid %s\n", p);
ret = -EINVAL;
+ kfree(p);
goto out;
}
+ kfree(p);
break;
case NVMF_OPT_DUP_CONNECT:
opts->duplicate_connect = true;
--
2.13.6

2018-01-25 09:39:25

by Max Gurtovoy

Subject: Re: [PATCH] nvme: don't free uuid pointer before printing it

On 1/25/2018 10:09 AM, Johannes Thumshirn wrote:
> Commit df351ef73789 ("nvme-fabrics: fix memory leak when parsing host ID
> option") fixed the leak of 'p' but in case uuid_parse() fails the memory
> is freed before the error print that is using it.
>
> Free it after printing eventual errors.
>
> Signed-off-by: Johannes Thumshirn <[email protected]>
> Fixes: df351ef73789 ("nvme-fabrics: fix memory leak when parsing host ID option")
> Reported-by: Dan Carpenter <[email protected]>
> Cc: Roland Dreier <[email protected]>
> ---
> drivers/nvme/host/fabrics.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index eb46967bb0d5..9cee72a80472 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -739,12 +739,13 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
> goto out;
> }
> ret = uuid_parse(p, &hostid);
> - kfree(p);
> if (ret) {
> pr_err("Invalid hostid %s\n", p);
> ret = -EINVAL;
> + kfree(p);
> goto out;
> }
> + kfree(p);
> break;
> case NVMF_OPT_DUP_CONNECT:
> opts->duplicate_connect = true;
>


Looks good,

Reviewed-by: Max Gurtovoy <[email protected]>

2018-01-25 15:21:47

by Christoph Hellwig

Subject: Re: [PATCH] nvme: don't free uuid pointer before printing it

On Thu, Jan 25, 2018 at 09:09:25AM +0100, Johannes Thumshirn wrote:
> Commit df351ef73789 ("nvme-fabrics: fix memory leak when parsing host ID
> option") fixed the leak of 'p' but in case uuid_parse() fails the memory
> is freed before the error print that is using it.
>
> Free it after printing eventual errors.

Thanks,

applied to nvme-4.16.