[dpdk-stable] [RFC 1/9] devargs: fix data buffer storage type

[dpdk-stable] [RFC 1/9] devargs: fix data buffer storage type

[Xueming Li]

The data field fo struct devargs is used as data scratch buffer, not a
const, remove.

Also fixes references to data field of struct devargs.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Fixes: c99a2d4c6b7f ("eal: implement device iteration initialization")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 examples/vdpa/main.c                   | 6 ++++--
 lib/librte_eal/common/eal_common_dev.c | 3 +--
 lib/librte_eal/include/rte_dev.h       | 2 +-
 lib/librte_eal/include/rte_devargs.h   | 2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/examples/vdpa/main.c b/examples/vdpa/main.c
index 97e967b9a2..88e9d8780a 100644
--- a/examples/vdpa/main.c
+++ b/examples/vdpa/main.c
@@ -294,9 +294,10 @@ static void cmd_list_vdpa_devices_parsed(
 	struct rte_vdpa_device *vdev;
 	struct rte_device *dev;
 	struct rte_dev_iterator dev_iter;
+	char args[16];
 
 	cmdline_printf(cl, "device name\tqueue num\tsupported features\n");
-	RTE_DEV_FOREACH(dev, "class=vdpa", &dev_iter) {
+	RTE_DEV_FOREACH(dev, strcpy(args, "class=vdpa"), &dev_iter) {
 		vdev = rte_vdpa_find_device_by_name(dev->name);
 		if (!vdev)
 			continue;
@@ -528,6 +529,7 @@ main(int argc, char *argv[])
 	struct rte_vdpa_device *vdev;
 	struct rte_device *dev;
 	struct rte_dev_iterator dev_iter;
+	char args[16];
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
@@ -549,7 +551,7 @@ main(int argc, char *argv[])
 		cmdline_interact(cl);
 		cmdline_stdin_exit(cl);
 	} else {
-		RTE_DEV_FOREACH(dev, "class=vdpa", &dev_iter) {
+		RTE_DEV_FOREACH(dev, strcpy(args, "class=vdpa"), &dev_iter) {
 			vdev = rte_vdpa_find_device_by_name(dev->name);
 			if (vdev == NULL) {
 				rte_panic("Failed to find vDPA dev for %s\n",
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 8a3bd3100a..793fbdf24b 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -572,8 +572,7 @@ rte_dev_event_callback_process(const char *device_name,
 }
 
 int
-rte_dev_iterator_init(struct rte_dev_iterator *it,
-		      const char *dev_str)
+rte_dev_iterator_init(struct rte_dev_iterator *it, char *dev_str)
 {
 	struct rte_devargs devargs;
 	struct rte_class *cls = NULL;
diff --git a/lib/librte_eal/include/rte_dev.h b/lib/librte_eal/include/rte_dev.h
index 6dd72c11a1..b320e98637 100644
--- a/lib/librte_eal/include/rte_dev.h
+++ b/lib/librte_eal/include/rte_dev.h
@@ -299,7 +299,7 @@ typedef void *(*rte_dev_iterate_t)(const void *start,
  */
 __rte_experimental
 int
-rte_dev_iterator_init(struct rte_dev_iterator *it, const char *str);
+rte_dev_iterator_init(struct rte_dev_iterator *it, char *str);
 
 /**
  * Iterates on a device iterator.
diff --git a/lib/librte_eal/include/rte_devargs.h b/lib/librte_eal/include/rte_devargs.h
index 296f19324f..8a5ffa2af2 100644
--- a/lib/librte_eal/include/rte_devargs.h
+++ b/lib/librte_eal/include/rte_devargs.h
@@ -69,7 +69,7 @@ struct rte_devargs {
 	struct rte_class *cls; /**< class handle. */
 	const char *bus_str; /**< bus-related part of device string. */
 	const char *cls_str; /**< class-related part of device string. */
-	const char *data; /**< Device string storage. */
+	char *data; /**< Device string storage. */
 };
 
 /**
-- 
2.25.1

[dpdk-stable] [RFC 2/9] devargs: fix memory leak on parsing error

[Xueming Li]

This patch fixes memory leak in parsing error handling.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 lib/librte_eal/common/eal_common_devargs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index fcf3d9a3cc..f36f71fbce 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -163,6 +163,11 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 		if (layers[i].kvlist)
 			rte_kvargs_free(layers[i].kvlist);
 	}
+	if (ret && devargs->data && devargs->data != devstr) {
+		/* Free duplicated data. */
+		free(devargs->data);
+		devargs->data = NULL;
+	}
 	if (ret != 0)
 		rte_errno = -ret;
 	return ret;
-- 
2.25.1

[dpdk-stable] [RFC 3/9] devargs: fix memory leak in legacy parser

[Xueming Li]

Data field was designed as parser buffer, will be released once in
releasing struct memory. The duplicated device arguments was not saved
to data and this caused memory leak.

This patch fixes this leak by saving to new allocated memory to data
field.

Fixes: 4969f5914c9e ("devargs: introduce new parsing helper")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 lib/librte_eal/common/eal_common_devargs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index f36f71fbce..3c4774c88a 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -224,13 +224,14 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev)
 	da->bus = bus;
 	/* Parse eventual device arguments */
 	if (devname[i] == ',')
-		da->args = strdup(&devname[i + 1]);
+		da->data = strdup(&devname[i + 1]);
 	else
-		da->args = strdup("");
-	if (da->args == NULL) {
+		da->data = strdup("");
+	if (da->data == NULL) {
 		RTE_LOG(ERR, EAL, "not enough memory to parse arguments\n");
 		return -ENOMEM;
 	}
+	da->args = da->data;
 	return 0;
 }
 
-- 
2.25.1

[dpdk-stable] [RFC 4/9] devargs: fix buffer data memory leak

[Xueming Li]

Struct rte_devargs data buffer was changed from args to data field, not
all references were changed accordingly, memory leak happened when
releasing devargs.

Free data field of devargs struct.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org
Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 app/test-pmd/config.c                        | 4 ++--
 app/test-pmd/testpmd.c                       | 4 ++--
 drivers/bus/vdev/vdev.c                      | 5 +++--
 drivers/net/failsafe/failsafe_args.c         | 3 ++-
 drivers/net/failsafe/failsafe_eal.c          | 2 +-
 examples/multi_process/hotplug_mp/commands.c | 8 ++++----
 lib/librte_eal/common/eal_common_dev.c       | 4 ++--
 lib/librte_eal/common/eal_common_devargs.c   | 7 ++++---
 lib/librte_eal/common/hotplug_mp.c           | 5 ++---
 lib/librte_ethdev/rte_ethdev.c               | 5 +++--
 10 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b51de59e1e..e7f456692b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -593,8 +593,8 @@ device_infos_display(const char *identifier)
 
 	if (rte_devargs_parsef(&da, "%s", identifier)) {
 		printf("cannot parse identifier\n");
-		if (da.args)
-			free(da.args);
+		if (da.data)
+			free(da.data);
 		return;
 	}
 
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 33fc0fddf5..66f3ff9320 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3056,8 +3056,8 @@ detach_devargs(char *identifier)
 	memset(&da, 0, sizeof(da));
 	if (rte_devargs_parsef(&da, "%s", identifier)) {
 		printf("cannot parse identifier\n");
-		if (da.args)
-			free(da.args);
+		if (da.data)
+			free(da.data);
 		return;
 	}
 
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index acfd78828f..43375bb334 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -236,9 +236,10 @@ alloc_devargs(const char *name, const char *args)
 
 	devargs->bus = &rte_vdev_bus;
 	if (args)
-		devargs->args = strdup(args);
+		devargs->data = strdup(args);
 	else
-		devargs->args = strdup("");
+		devargs->data = strdup("");
+	devargs->args = devargs->data;
 
 	ret = strlcpy(devargs->name, name, sizeof(devargs->name));
 	if (ret < 0 || ret >= (int)sizeof(devargs->name)) {
diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index 707490b94c..5e507bffbc 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -451,7 +451,8 @@ failsafe_args_free(struct rte_eth_dev *dev)
 		sdev->cmdline = NULL;
 		free(sdev->fd_str);
 		sdev->fd_str = NULL;
-		free(sdev->devargs.args);
+		free(sdev->devargs.data);
+		sdev->devargs.data = NULL;
 		sdev->devargs.args = NULL;
 	}
 }
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index b9fc508673..f066c053f3 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -79,7 +79,7 @@ fs_bus_init(struct rte_eth_dev *dev)
 					rte_eth_devices[pid].device->devargs;
 
 			/* Take control of probed device. */
-			free(da->args);
+			free(da->data);
 			memset(da, 0, sizeof(*da));
 			if (probed_da != NULL)
 				snprintf(devstr, sizeof(devstr), "%s,%s",
diff --git a/examples/multi_process/hotplug_mp/commands.c b/examples/multi_process/hotplug_mp/commands.c
index a8a39d07f7..e77585e5b4 100644
--- a/examples/multi_process/hotplug_mp/commands.c
+++ b/examples/multi_process/hotplug_mp/commands.c
@@ -121,8 +121,8 @@ static void cmd_dev_attach_parsed(void *parsed_result,
 
 	if (rte_devargs_parsef(&da, "%s", res->devargs)) {
 		cmdline_printf(cl, "cannot parse devargs\n");
-		if (da.args)
-			free(da.args);
+		if (da.data)
+			free(da.data);
 		return;
 	}
 
@@ -168,8 +168,8 @@ static void cmd_dev_detach_parsed(void *parsed_result,
 
 	if (rte_devargs_parsef(&da, "%s", res->devargs)) {
 		cmdline_printf(cl, "cannot parse devargs\n");
-		if (da.args)
-			free(da.args);
+		if (da.data)
+			free(da.data);
 		return;
 	}
 
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 793fbdf24b..f65a9594cc 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -186,7 +186,7 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
 
 err_devarg:
 	if (rte_devargs_remove(da) != 0) {
-		free(da->args);
+		free(da->data);
 		free(da);
 	}
 	return ret;
@@ -585,7 +585,7 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, char *dev_str)
 	it->bus_str = NULL;
 	it->cls_str = NULL;
 
-	devargs.data = dev_str;
+	devargs.data = NULL;
 	if (rte_devargs_layers_parse(&devargs, dev_str))
 		goto get_out;
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 3c4774c88a..e1a3cd7367 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -284,7 +284,8 @@ rte_devargs_insert(struct rte_devargs **da)
 			/* device already in devargs list, must be updated */
 			listed_da->type = (*da)->type;
 			listed_da->policy = (*da)->policy;
-			free(listed_da->args);
+			if (listed_da->data)
+				free(listed_da->data);
 			listed_da->args = (*da)->args;
 			listed_da->bus = (*da)->bus;
 			listed_da->cls = (*da)->cls;
@@ -332,7 +333,7 @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 
 fail:
 	if (devargs) {
-		free(devargs->args);
+		free(devargs->data);
 		free(devargs);
 	}
 
@@ -352,7 +353,7 @@ rte_devargs_remove(struct rte_devargs *devargs)
 		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
 		    strcmp(d->name, devargs->name) == 0) {
 			TAILQ_REMOVE(&devargs_list, d, next);
-			free(d->args);
+			free(d->data);
 			free(d);
 			return 0;
 		}
diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c
index ee791903b3..f0f7c61048 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -118,8 +118,7 @@ __handle_secondary_request(void *param)
 		ret = rte_devargs_parse(&da, req->devargs);
 		if (ret != 0)
 			goto finish;
-		free(da.args); /* we don't need those */
-		da.args = NULL;
+		free(da.data); /* we don't need those */
 
 		ret = eal_dev_hotplug_request_to_secondary(&tmp_req);
 		if (ret != 0) {
@@ -283,7 +282,7 @@ static void __handle_primary_request(void *param)
 
 		ret = local_dev_remove(dev);
 quit:
-		free(da->args);
+		free(da->data);
 		free(da);
 		break;
 	default:
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 17ddacc78d..4976961d13 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -244,7 +244,8 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 		goto error;
 	}
 	iter->cls_str = cls_str;
-	free(devargs.args); /* allocated by rte_devargs_parse() */
+	free(devargs.data); /* allocated by rte_devargs_parse() */
+	devargs.data = NULL;
 	devargs.args = NULL;
 
 	iter->bus = devargs.bus;
@@ -284,7 +285,7 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 	if (ret == -ENOTSUP)
 		RTE_ETHDEV_LOG(ERR, "Bus %s does not support iterating.\n",
 				iter->bus->name);
-	free(devargs.args);
+	free(devargs.data);
 	free(bus_str);
 	free(cls_str);
 	return ret;
-- 
2.25.1

[dpdk-stable] [PATCH v1 1/7] devargs: fix data buffer storage type

[Xueming Li]

The data field fo struct devargs is used as data scratch buffer, not a
const, remove.

Also fixes references to data field of struct devargs.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Fixes: c99a2d4c6b7f ("eal: implement device iteration initialization")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 examples/vdpa/main.c                   | 6 ++++--
 lib/librte_eal/common/eal_common_dev.c | 3 +--
 lib/librte_eal/include/rte_dev.h       | 2 +-
 lib/librte_eal/include/rte_devargs.h   | 2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/examples/vdpa/main.c b/examples/vdpa/main.c
index 97e967b9a2..88e9d8780a 100644
--- a/examples/vdpa/main.c
+++ b/examples/vdpa/main.c
@@ -294,9 +294,10 @@ static void cmd_list_vdpa_devices_parsed(
 	struct rte_vdpa_device *vdev;
 	struct rte_device *dev;
 	struct rte_dev_iterator dev_iter;
+	char args[16];
 
 	cmdline_printf(cl, "device name\tqueue num\tsupported features\n");
-	RTE_DEV_FOREACH(dev, "class=vdpa", &dev_iter) {
+	RTE_DEV_FOREACH(dev, strcpy(args, "class=vdpa"), &dev_iter) {
 		vdev = rte_vdpa_find_device_by_name(dev->name);
 		if (!vdev)
 			continue;
@@ -528,6 +529,7 @@ main(int argc, char *argv[])
 	struct rte_vdpa_device *vdev;
 	struct rte_device *dev;
 	struct rte_dev_iterator dev_iter;
+	char args[16];
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
@@ -549,7 +551,7 @@ main(int argc, char *argv[])
 		cmdline_interact(cl);
 		cmdline_stdin_exit(cl);
 	} else {
-		RTE_DEV_FOREACH(dev, "class=vdpa", &dev_iter) {
+		RTE_DEV_FOREACH(dev, strcpy(args, "class=vdpa"), &dev_iter) {
 			vdev = rte_vdpa_find_device_by_name(dev->name);
 			if (vdev == NULL) {
 				rte_panic("Failed to find vDPA dev for %s\n",
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 8a3bd3100a..793fbdf24b 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -572,8 +572,7 @@ rte_dev_event_callback_process(const char *device_name,
 }
 
 int
-rte_dev_iterator_init(struct rte_dev_iterator *it,
-		      const char *dev_str)
+rte_dev_iterator_init(struct rte_dev_iterator *it, char *dev_str)
 {
 	struct rte_devargs devargs;
 	struct rte_class *cls = NULL;
diff --git a/lib/librte_eal/include/rte_dev.h b/lib/librte_eal/include/rte_dev.h
index 6dd72c11a1..b320e98637 100644
--- a/lib/librte_eal/include/rte_dev.h
+++ b/lib/librte_eal/include/rte_dev.h
@@ -299,7 +299,7 @@ typedef void *(*rte_dev_iterate_t)(const void *start,
  */
 __rte_experimental
 int
-rte_dev_iterator_init(struct rte_dev_iterator *it, const char *str);
+rte_dev_iterator_init(struct rte_dev_iterator *it, char *str);
 
 /**
  * Iterates on a device iterator.
diff --git a/lib/librte_eal/include/rte_devargs.h b/lib/librte_eal/include/rte_devargs.h
index 296f19324f..8a5ffa2af2 100644
--- a/lib/librte_eal/include/rte_devargs.h
+++ b/lib/librte_eal/include/rte_devargs.h
@@ -69,7 +69,7 @@ struct rte_devargs {
 	struct rte_class *cls; /**< class handle. */
 	const char *bus_str; /**< bus-related part of device string. */
 	const char *cls_str; /**< class-related part of device string. */
-	const char *data; /**< Device string storage. */
+	char *data; /**< Device string storage. */
 };
 
 /**
-- 
2.25.1

[dpdk-stable] [PATCH v1 2/7] devargs: fix memory leak on parsing error

[Xueming Li]

This patch fixes memory leak in parsing error handling.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 lib/librte_eal/common/eal_common_devargs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index fcf3d9a3cc..f36f71fbce 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -163,6 +163,11 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 		if (layers[i].kvlist)
 			rte_kvargs_free(layers[i].kvlist);
 	}
+	if (ret && devargs->data && devargs->data != devstr) {
+		/* Free duplicated data. */
+		free(devargs->data);
+		devargs->data = NULL;
+	}
 	if (ret != 0)
 		rte_errno = -ret;
 	return ret;
-- 
2.25.1

[dpdk-stable] [PATCH v1 3/7] devargs: fix memory leak in legacy parser

[Xueming Li]

Data field was designed as parser buffer, will be released once in
releasing struct memory. The duplicated device arguments was not saved
to data and this caused memory leak.

This patch fixes this leak by saving to new allocated memory to data
field.

Fixes: 4969f5914c9e ("devargs: introduce new parsing helper")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 lib/librte_eal/common/eal_common_devargs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index f36f71fbce..3c4774c88a 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -224,13 +224,14 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev)
 	da->bus = bus;
 	/* Parse eventual device arguments */
 	if (devname[i] == ',')
-		da->args = strdup(&devname[i + 1]);
+		da->data = strdup(&devname[i + 1]);
 	else
-		da->args = strdup("");
-	if (da->args == NULL) {
+		da->data = strdup("");
+	if (da->data == NULL) {
 		RTE_LOG(ERR, EAL, "not enough memory to parse arguments\n");
 		return -ENOMEM;
 	}
+	da->args = da->data;
 	return 0;
 }
 
-- 
2.25.1

[dpdk-stable] [PATCH v1 4/7] devargs: fix buffer data memory leak

[Xueming Li]

Struct rte_devargs data buffer was changed from args to data field, not
all references were changed accordingly, memory leak happened when
releasing devargs.

Free data field of devargs struct.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org
Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 app/test-pmd/config.c                        | 4 ++--
 app/test-pmd/testpmd.c                       | 4 ++--
 drivers/bus/vdev/vdev.c                      | 5 +++--
 drivers/net/failsafe/failsafe_args.c         | 3 ++-
 drivers/net/failsafe/failsafe_eal.c          | 2 +-
 examples/multi_process/hotplug_mp/commands.c | 8 ++++----
 lib/librte_eal/common/eal_common_dev.c       | 4 ++--
 lib/librte_eal/common/eal_common_devargs.c   | 7 ++++---
 lib/librte_eal/common/hotplug_mp.c           | 5 ++---
 lib/librte_ethdev/rte_ethdev.c               | 5 +++--
 10 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b51de59e1e..e7f456692b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -593,8 +593,8 @@ device_infos_display(const char *identifier)
 
 	if (rte_devargs_parsef(&da, "%s", identifier)) {
 		printf("cannot parse identifier\n");
-		if (da.args)
-			free(da.args);
+		if (da.data)
+			free(da.data);
 		return;
 	}
 
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 33fc0fddf5..66f3ff9320 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3056,8 +3056,8 @@ detach_devargs(char *identifier)
 	memset(&da, 0, sizeof(da));
 	if (rte_devargs_parsef(&da, "%s", identifier)) {
 		printf("cannot parse identifier\n");
-		if (da.args)
-			free(da.args);
+		if (da.data)
+			free(da.data);
 		return;
 	}
 
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index acfd78828f..43375bb334 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -236,9 +236,10 @@ alloc_devargs(const char *name, const char *args)
 
 	devargs->bus = &rte_vdev_bus;
 	if (args)
-		devargs->args = strdup(args);
+		devargs->data = strdup(args);
 	else
-		devargs->args = strdup("");
+		devargs->data = strdup("");
+	devargs->args = devargs->data;
 
 	ret = strlcpy(devargs->name, name, sizeof(devargs->name));
 	if (ret < 0 || ret >= (int)sizeof(devargs->name)) {
diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index 707490b94c..5e507bffbc 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -451,7 +451,8 @@ failsafe_args_free(struct rte_eth_dev *dev)
 		sdev->cmdline = NULL;
 		free(sdev->fd_str);
 		sdev->fd_str = NULL;
-		free(sdev->devargs.args);
+		free(sdev->devargs.data);
+		sdev->devargs.data = NULL;
 		sdev->devargs.args = NULL;
 	}
 }
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index b9fc508673..f066c053f3 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -79,7 +79,7 @@ fs_bus_init(struct rte_eth_dev *dev)
 					rte_eth_devices[pid].device->devargs;
 
 			/* Take control of probed device. */
-			free(da->args);
+			free(da->data);
 			memset(da, 0, sizeof(*da));
 			if (probed_da != NULL)
 				snprintf(devstr, sizeof(devstr), "%s,%s",
diff --git a/examples/multi_process/hotplug_mp/commands.c b/examples/multi_process/hotplug_mp/commands.c
index a8a39d07f7..e77585e5b4 100644
--- a/examples/multi_process/hotplug_mp/commands.c
+++ b/examples/multi_process/hotplug_mp/commands.c
@@ -121,8 +121,8 @@ static void cmd_dev_attach_parsed(void *parsed_result,
 
 	if (rte_devargs_parsef(&da, "%s", res->devargs)) {
 		cmdline_printf(cl, "cannot parse devargs\n");
-		if (da.args)
-			free(da.args);
+		if (da.data)
+			free(da.data);
 		return;
 	}
 
@@ -168,8 +168,8 @@ static void cmd_dev_detach_parsed(void *parsed_result,
 
 	if (rte_devargs_parsef(&da, "%s", res->devargs)) {
 		cmdline_printf(cl, "cannot parse devargs\n");
-		if (da.args)
-			free(da.args);
+		if (da.data)
+			free(da.data);
 		return;
 	}
 
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 793fbdf24b..f65a9594cc 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -186,7 +186,7 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
 
 err_devarg:
 	if (rte_devargs_remove(da) != 0) {
-		free(da->args);
+		free(da->data);
 		free(da);
 	}
 	return ret;
@@ -585,7 +585,7 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, char *dev_str)
 	it->bus_str = NULL;
 	it->cls_str = NULL;
 
-	devargs.data = dev_str;
+	devargs.data = NULL;
 	if (rte_devargs_layers_parse(&devargs, dev_str))
 		goto get_out;
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 3c4774c88a..e1a3cd7367 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -284,7 +284,8 @@ rte_devargs_insert(struct rte_devargs **da)
 			/* device already in devargs list, must be updated */
 			listed_da->type = (*da)->type;
 			listed_da->policy = (*da)->policy;
-			free(listed_da->args);
+			if (listed_da->data)
+				free(listed_da->data);
 			listed_da->args = (*da)->args;
 			listed_da->bus = (*da)->bus;
 			listed_da->cls = (*da)->cls;
@@ -332,7 +333,7 @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 
 fail:
 	if (devargs) {
-		free(devargs->args);
+		free(devargs->data);
 		free(devargs);
 	}
 
@@ -352,7 +353,7 @@ rte_devargs_remove(struct rte_devargs *devargs)
 		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
 		    strcmp(d->name, devargs->name) == 0) {
 			TAILQ_REMOVE(&devargs_list, d, next);
-			free(d->args);
+			free(d->data);
 			free(d);
 			return 0;
 		}
diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c
index ee791903b3..f0f7c61048 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -118,8 +118,7 @@ __handle_secondary_request(void *param)
 		ret = rte_devargs_parse(&da, req->devargs);
 		if (ret != 0)
 			goto finish;
-		free(da.args); /* we don't need those */
-		da.args = NULL;
+		free(da.data); /* we don't need those */
 
 		ret = eal_dev_hotplug_request_to_secondary(&tmp_req);
 		if (ret != 0) {
@@ -283,7 +282,7 @@ static void __handle_primary_request(void *param)
 
 		ret = local_dev_remove(dev);
 quit:
-		free(da->args);
+		free(da->data);
 		free(da);
 		break;
 	default:
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 17ddacc78d..4976961d13 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -244,7 +244,8 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 		goto error;
 	}
 	iter->cls_str = cls_str;
-	free(devargs.args); /* allocated by rte_devargs_parse() */
+	free(devargs.data); /* allocated by rte_devargs_parse() */
+	devargs.data = NULL;
 	devargs.args = NULL;
 
 	iter->bus = devargs.bus;
@@ -284,7 +285,7 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 	if (ret == -ENOTSUP)
 		RTE_ETHDEV_LOG(ERR, "Bus %s does not support iterating.\n",
 				iter->bus->name);
-	free(devargs.args);
+	free(devargs.data);
 	free(bus_str);
 	free(cls_str);
 	return ret;
-- 
2.25.1

[dpdk-stable] [PATCH v2 1/5] devargs: fix memory leak on parsing error

[Xueming Li]

This patch fixes memory leak in parsing error handling.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 lib/librte_eal/common/eal_common_devargs.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index fcf3d9a3cc..c3969ff158 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -163,8 +163,14 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 		if (layers[i].kvlist)
 			rte_kvargs_free(layers[i].kvlist);
 	}
-	if (ret != 0)
+	if (ret != 0) {
+		if (devargs->data && devargs->data != devstr) {
+			/* Free duplicated data. */
+			free(devargs->data);
+			devargs->data = NULL;
+		}
 		rte_errno = -ret;
+	}
 	return ret;
 }
 
-- 
2.25.1

Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/5] devargs: fix memory leak on parsing error

[Thomas Monjalon]

18/01/2021 16:16, Xueming Li:
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> +	if (ret != 0) {
> +		if (devargs->data && devargs->data != devstr) {

Better to make comparison explicit:
	if (devargs->data != NULL

> +			/* Free duplicated data. */
> +			free(devargs->data);

Before patch 2, devargs->data is const,
so we cannot free (compilation error).

[dpdk-stable] [PATCH v3 2/5] devargs: fix memory leak on parsing error

[Xueming Li]

This patch fixes memory leak in parsing error handling.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 lib/librte_eal/common/eal_common_devargs.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 48f85ee9c0..e40b91ea66 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -60,6 +60,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 	size_t nblayer;
 	size_t i = 0;
 	int ret = 0;
+	bool allocated_data = false;
 
 	/* Split each sub-lists. */
 	nblayer = devargs_layer_count(devstr);
@@ -81,6 +82,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 			ret = -ENOMEM;
 			goto get_out;
 		}
+		allocated_data = true;
 		s = devargs->data;
 	}
 
@@ -163,8 +165,14 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 		if (layers[i].kvlist)
 			rte_kvargs_free(layers[i].kvlist);
 	}
-	if (ret != 0)
+	if (ret != 0) {
+		if (allocated_data) {
+			/* Free duplicated data. */
+			free(devargs->data);
+			devargs->data = NULL;
+		}
 		rte_errno = -ret;
+	}
 	return ret;
 }
 
-- 
2.25.1

[dpdk-stable] [PATCH v4 2/5] devargs: fix memory leak on parsing error

[Xueming Li]

This patch fixes memory leak in parsing error handling.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Reviewed-by: Gaetan Rivet <grive@u256.net>
---
 lib/librte_eal/common/eal_common_devargs.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 48f85ee9c0..e40b91ea66 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -60,6 +60,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 	size_t nblayer;
 	size_t i = 0;
 	int ret = 0;
+	bool allocated_data = false;
 
 	/* Split each sub-lists. */
 	nblayer = devargs_layer_count(devstr);
@@ -81,6 +82,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 			ret = -ENOMEM;
 			goto get_out;
 		}
+		allocated_data = true;
 		s = devargs->data;
 	}
 
@@ -163,8 +165,14 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 		if (layers[i].kvlist)
 			rte_kvargs_free(layers[i].kvlist);
 	}
-	if (ret != 0)
+	if (ret != 0) {
+		if (allocated_data) {
+			/* Free duplicated data. */
+			free(devargs->data);
+			devargs->data = NULL;
+		}
 		rte_errno = -ret;
+	}
 	return ret;
 }
 
-- 
2.25.1

[dpdk-stable] [PATCH v5 2/5] devargs: fix memory leak on parsing error

[Xueming Li]

This patch fixes memory leak in parsing error handling.

Fixes: 338327d731e6 ("devargs: add function to parse device layers")
Cc: gaetan.rivet@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Reviewed-by: Gaetan Rivet <grive@u256.net>
---
 lib/librte_eal/common/eal_common_devargs.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 48f85ee9c0..e40b91ea66 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -60,6 +60,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 	size_t nblayer;
 	size_t i = 0;
 	int ret = 0;
+	bool allocated_data = false;
 
 	/* Split each sub-lists. */
 	nblayer = devargs_layer_count(devstr);
@@ -81,6 +82,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 			ret = -ENOMEM;
 			goto get_out;
 		}
+		allocated_data = true;
 		s = devargs->data;
 	}
 
@@ -163,8 +165,14 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 		if (layers[i].kvlist)
 			rte_kvargs_free(layers[i].kvlist);
 	}
-	if (ret != 0)
+	if (ret != 0) {
+		if (allocated_data) {
+			/* Free duplicated data. */
+			free(devargs->data);
+			devargs->data = NULL;
+		}
 		rte_errno = -ret;
+	}
 	return ret;
 }
 
-- 
2.25.1