Skip to contentSkip to conversation

nvme-fabrics: Fix memory leak when parsing host ID option

PATCH New Roland Dreier 4 participants 6 messages Jan 11, 2018 – Jan 15, 2018 NVMe

lore
Share & tools
lore

Roland Dreier

author
#0 lore raw

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

From: Roland Dreier <[email protected]>

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(-)
drivers/nvme/host/fabrics.c +3 -1
index 76b4fe6816a0..2ba52f9f8185 100644
@@ -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;
Signature · 1 line
2.14.1
#1 lore raw

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

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

Signature · 6 lines
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
author
#2 lore raw

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

Quoted 10 lines · 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 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.

maintainer
#3 lore raw

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

Quoted 6 lines

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>
#4 lore raw

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

In reply to Roland Dreier
Quoted 12 lines · Roland Dreier
Quoted 10 lines · 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?

Quoted 5 lines

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.

Signature · 6 lines
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