From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 365B4A09FD; Fri, 18 Dec 2020 16:17:48 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BD239CB19; Fri, 18 Dec 2020 16:17:15 +0100 (CET) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by dpdk.org (Postfix) with ESMTP id 38858CAE2 for ; Fri, 18 Dec 2020 16:17:10 +0100 (CET) Received: from Internal Mail-Server by MTLPINE1 (envelope-from xuemingl@nvidia.com) with SMTP; 18 Dec 2020 17:17:05 +0200 Received: from nvidia.com (pegasus05.mtr.labs.mlnx [10.210.16.100]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 0BIFH4qc010856; Fri, 18 Dec 2020 17:17:04 +0200 From: Xueming Li To: Viacheslav Ovsiienko , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Olivier Matz , Matan Azrad Cc: dev@dpdk.org, xuemingl@nvidia.com, Asaf Penso , gaetan.rivet@6wind.com, stable@dpdk.org Date: Fri, 18 Dec 2020 15:16:49 +0000 Message-Id: <1608304614-13908-5-git-send-email-xuemingl@nvidia.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1608304614-13908-1-git-send-email-xuemingl@nvidia.com> References: <1608304614-13908-1-git-send-email-xuemingl@nvidia.com> Subject: [dpdk-dev] [RFC 4/9] devargs: fix buffer data memory leak X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 --- 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