[dpdk-dev] [PATCH v1 4/7] devargs: fix buffer data memory leak
From: Xueming Li <hidden>
Date: 2021-01-08 14:55:54
Subsystem:
library code, networking drivers, the rest · Maintainers:
Andrew Morton, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
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 <redacted>
---
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