[PATCH 1/2] nvme-fabrics: Fix memory leak when parsing host ID option

[PATCH 1/2] nvme-fabrics: Fix memory leak when parsing host ID option

[Roland Dreier]

From: Roland Dreier <roland@purestorage.com>

We use match_strdup() to get a copy of the option string for host ID string, but
we just pass it to uuid_parse() and don't store the string pointer, so we need to
kfree() the string after parsing it.

Signed-off-by: Roland Dreier <roland at purestorage.com>
---
 drivers/nvme/host/fabrics.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 76b4fe6816a0..2ba52f9f8185 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -738,7 +738,9 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 				ret = -ENOMEM;
 				goto out;
 			}
-			if (uuid_parse(p, &hostid)) {
+			ret = uuid_parse(p, &hostid);
+			kfree(p);
+			if (ret) {
 				pr_err("Invalid hostid %s\n", p);
 				ret = -EINVAL;
 				goto out;
-- 
2.14.1

[PATCH 1/2] nvme-fabrics: Fix memory leak when parsing host ID option

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>

Do you mind fixing the same pattern on:
- NVMF_OPT_TRANSPORT
- NVMF_OPT_NQN
- NVMF_OPT_TRADDR
- NVMF_OPT_TRSVCID
- NVMF_OPT_HOST_TRADDR
as well?

Thanks,
	Johannes
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 1/2] nvme-fabrics: Fix memory leak when parsing host ID option

[Roland Dreier]

On Fri, Jan 12, 2018@2:16 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> Looks good,
> Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
>
> Do you mind fixing the same pattern on:
> - NVMF_OPT_TRANSPORT
> - NVMF_OPT_NQN
> - NVMF_OPT_TRADDR
> - NVMF_OPT_TRSVCID
> - NVMF_OPT_HOST_TRADDR
> as well?

Thanks for the review.  I don't think the same pattern is there for
the other options - a pointer to the string is
stored in opts and freed in nvmf_free_options(). NVMF_OPT_HOST_ID is
unique in that we don't stash the
strdup'ed pointer anywhere.

 - R.

[PATCH 1/2] nvme-fabrics: Fix memory leak when parsing host ID option

[Sagi Grimberg]

> Thanks for the review.  I don't think the same pattern is there for
> the other options - a pointer to the string is
> stored in opts and freed in nvmf_free_options(). NVMF_OPT_HOST_ID is
> unique in that we don't stash the
> strdup'ed pointer anywhere.

Agreed, thanks Roland,

Reviewed-by: Sagi Grimberg <sagi at grimberg.me>

[PATCH 1/2] nvme-fabrics: Fix memory leak when parsing host ID option

[Johannes Thumshirn]

On Fri, Jan 12, 2018@12:00:32PM -0800, Roland Dreier wrote:
> On Fri, Jan 12, 2018@2:16 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> > Looks good,
> > Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
> >
> > Do you mind fixing the same pattern on:
> > - NVMF_OPT_TRANSPORT
> > - NVMF_OPT_NQN
> > - NVMF_OPT_TRADDR
> > - NVMF_OPT_TRSVCID
> > - NVMF_OPT_HOST_TRADDR
> > as well?
> 
> Thanks for the review.  I don't think the same pattern is there for
> the other options - a pointer to the string is
> stored in opts and freed in nvmf_free_options(). NVMF_OPT_HOST_ID is
> unique in that we don't stash the
> strdup'ed pointer anywhere.

Correct. I'm sorry for the false alarm.

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 1/2] nvme-fabrics: Fix memory leak when parsing host ID option

[Christoph Hellwig]

Thanks,

applied to nvme-4.16.