* [PATCH] eventdev/eth_rx: fix memory leak when token parsing finished
@ 2022-02-02 7:13 Weiguo Li
2022-02-02 7:40 ` [PATCH v2] " Weiguo Li
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Weiguo Li @ 2022-02-02 7:13 UTC (permalink / raw)
To: jay.jayatheerthan; +Cc: ganapati.kundapura, s.v.naga.harish.k, dev
The memory get from strdup should be freed when parameter parsing
finished, and also should be freed when error occurs.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
lib/eventdev/rte_event_eth_rx_adapter.c | 39 +++++++++++++++++--------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index ae1e260c08..44775575a2 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -3337,21 +3337,24 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3359,6 +3362,8 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &queue_conf)) {
@@ -3401,21 +3406,24 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3423,6 +3431,8 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &q_stats)) {
@@ -3463,21 +3473,24 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3485,6 +3498,8 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
eth_dev_id,
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 7:13 [PATCH] eventdev/eth_rx: fix memory leak when token parsing finished Weiguo Li
@ 2022-02-02 7:40 ` Weiguo Li
2022-02-02 7:48 ` Kundapura, Ganapati
` (2 more replies)
2022-02-03 6:02 ` [PATCH] " Stephen Hemminger
2022-02-03 7:18 ` [PATCH v3] " Weiguo Li
2 siblings, 3 replies; 21+ messages in thread
From: Weiguo Li @ 2022-02-02 7:40 UTC (permalink / raw)
To: jay.jayatheerthan; +Cc: ganapati.kundapura, s.v.naga.harish.k, dev
The memory get from strdup should be freed when parameter parsing
finished, and also should be freed when error occurs.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
v2:
* add memory check after strdup
---
lib/eventdev/rte_event_eth_rx_adapter.c | 45 ++++++++++++++++++-------
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index ae1e260c08..8519c98b19 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -3332,26 +3332,31 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
rx_adapter_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3359,6 +3364,8 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &queue_conf)) {
@@ -3396,26 +3403,31 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
rx_adapter_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3423,6 +3435,8 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &q_stats)) {
@@ -3458,26 +3472,31 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
rx_adapter_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
+ free(l_params);
return -1;
-
+ }
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3485,6 +3504,8 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
eth_dev_id,
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 7:40 ` [PATCH v2] " Weiguo Li
@ 2022-02-02 7:48 ` Kundapura, Ganapati
2022-02-02 8:25 ` Jayatheerthan, Jay
2022-02-02 14:54 ` Kundapura, Ganapati
2022-02-03 3:38 ` Weiguo Li
2022-02-03 8:49 ` [PATCH v4] " Weiguo Li
2 siblings, 2 replies; 21+ messages in thread
From: Kundapura, Ganapati @ 2022-02-02 7:48 UTC (permalink / raw)
To: Weiguo Li, Jayatheerthan, Jay; +Cc: Naga Harish K, S V, dev
Acked-by: Ganapati Kundapura<ganapati.kundapura@intel.com>
> -----Original Message-----
> From: Weiguo Li <liwg06@foxmail.com>
> Sent: 02 February 2022 13:11
> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S
> V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> Subject: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing
> finished
>
> The memory get from strdup should be freed when parameter parsing
> finished, and also should be freed when error occurs.
>
> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> ---
> v2:
> * add memory check after strdup
> ---
> lib/eventdev/rte_event_eth_rx_adapter.c | 45 ++++++++++++++++++------
> -
> 1 file changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> b/lib/eventdev/rte_event_eth_rx_adapter.c
> index ae1e260c08..8519c98b19 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -3332,26 +3332,31 @@ handle_rxa_get_queue_conf(const char *cmd
> __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> rx_adapter_id = strtoul(token, NULL, 10);
>
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> _id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> + free(l_params);
> return -1;
> -
> + }
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> + free(l_params);
> return -1;
> -
> + }
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> >nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> rx_queue_id);
> + free(l_params);
> return -EINVAL;
> }
>
> @@ -3359,6 +3364,8 @@ handle_rxa_get_queue_conf(const char *cmd
> __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id,
> eth_dev_id,
> rx_queue_id,
> &queue_conf)) {
> @@ -3396,26 +3403,31 @@ handle_rxa_get_queue_stats(const char *cmd
> __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> rx_adapter_id = strtoul(token, NULL, 10);
>
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> _id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> + free(l_params);
> return -1;
> -
> + }
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> + free(l_params);
> return -1;
> -
> + }
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> >nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> rx_queue_id);
> + free(l_params);
> return -EINVAL;
> }
>
> @@ -3423,6 +3435,8 @@ handle_rxa_get_queue_stats(const char *cmd
> __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id,
> eth_dev_id,
> rx_queue_id, &q_stats)) {
> @@ -3458,26 +3472,31 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> rx_adapter_id = strtoul(token, NULL, 10);
>
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> _id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> + free(l_params);
> return -1;
> -
> + }
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> + free(l_params);
> return -1;
> -
> + }
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> >nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> rx_queue_id);
> + free(l_params);
> return -EINVAL;
> }
>
> @@ -3485,6 +3504,8 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> eth_dev_id,
> --
> 2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 7:48 ` Kundapura, Ganapati
@ 2022-02-02 8:25 ` Jayatheerthan, Jay
2022-02-02 14:54 ` Kundapura, Ganapati
1 sibling, 0 replies; 21+ messages in thread
From: Jayatheerthan, Jay @ 2022-02-02 8:25 UTC (permalink / raw)
To: Kundapura, Ganapati, Weiguo Li; +Cc: Naga Harish K, S V, dev
Looks good. Thanks Weiguo for posting the patch.
Acked-by: Jay Jayatheerthan <jay.jayatheerthan@intel.com>
-Jay
> -----Original Message-----
> From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> Sent: Wednesday, February 2, 2022 1:18 PM
> To: Weiguo Li <liwg06@foxmail.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
>
> Acked-by: Ganapati Kundapura<ganapati.kundapura@intel.com>
>
> > -----Original Message-----
> > From: Weiguo Li <liwg06@foxmail.com>
> > Sent: 02 February 2022 13:11
> > To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S
> > V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> > Subject: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing
> > finished
> >
> > The memory get from strdup should be freed when parameter parsing
> > finished, and also should be freed when error occurs.
> >
> > Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> > Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
> >
> > Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> > ---
> > v2:
> > * add memory check after strdup
> > ---
> > lib/eventdev/rte_event_eth_rx_adapter.c | 45 ++++++++++++++++++------
> > -
> > 1 file changed, 33 insertions(+), 12 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > index ae1e260c08..8519c98b19 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > @@ -3332,26 +3332,31 @@ handle_rxa_get_queue_conf(const char *cmd
> > __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > rx_adapter_id = strtoul(token, NULL, 10);
> >
> > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > _id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > >nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> > rx_queue_id);
> > + free(l_params);
> > return -EINVAL;
> > }
> >
> > @@ -3359,6 +3364,8 @@ handle_rxa_get_queue_conf(const char *cmd
> > __rte_unused,
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id,
> > eth_dev_id,
> > rx_queue_id,
> > &queue_conf)) {
> > @@ -3396,26 +3403,31 @@ handle_rxa_get_queue_stats(const char *cmd
> > __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > rx_adapter_id = strtoul(token, NULL, 10);
> >
> > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > _id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > >nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> > rx_queue_id);
> > + free(l_params);
> > return -EINVAL;
> > }
> >
> > @@ -3423,6 +3435,8 @@ handle_rxa_get_queue_stats(const char *cmd
> > __rte_unused,
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id,
> > eth_dev_id,
> > rx_queue_id, &q_stats)) {
> > @@ -3458,26 +3472,31 @@ handle_rxa_queue_stats_reset(const char *cmd
> > __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > rx_adapter_id = strtoul(token, NULL, 10);
> >
> > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > _id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > >nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> > rx_queue_id);
> > + free(l_params);
> > return -EINVAL;
> > }
> >
> > @@ -3485,6 +3504,8 @@ handle_rxa_queue_stats_reset(const char *cmd
> > __rte_unused,
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> > eth_dev_id,
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 7:48 ` Kundapura, Ganapati
2022-02-02 8:25 ` Jayatheerthan, Jay
@ 2022-02-02 14:54 ` Kundapura, Ganapati
2022-02-02 16:47 ` Kundapura, Ganapati
1 sibling, 1 reply; 21+ messages in thread
From: Kundapura, Ganapati @ 2022-02-02 14:54 UTC (permalink / raw)
To: Kundapura, Ganapati, Weiguo Li, Jayatheerthan, Jay
Cc: Naga Harish K, S V, dev
Hi Weigo,
Could you please address the below issues also?
Thanks,
Ganapati
> -----Original Message-----
> From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> Sent: 02 February 2022 13:18
> To: Weiguo Li <liwg06@foxmail.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>
> Cc: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token
> parsing finished
>
> Acked-by: Ganapati Kundapura<ganapati.kundapura@intel.com>
>
> > -----Original Message-----
> > From: Weiguo Li <liwg06@foxmail.com>
> > Sent: 02 February 2022 13:11
> > To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K,
> > S V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> > Subject: [PATCH v2] eventdev/eth_rx: fix memory leak when token
> > parsing finished
> >
> > The memory get from strdup should be freed when parameter parsing
> > finished, and also should be freed when error occurs.
> >
> > Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> > Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
> >
> > Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> > ---
> > v2:
> > * add memory check after strdup
> > ---
> > lib/eventdev/rte_event_eth_rx_adapter.c | 45 ++++++++++++++++++----
> --
> > -
> > 1 file changed, 33 insertions(+), 12 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > index ae1e260c08..8519c98b19 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > @@ -3332,26 +3332,31 @@ handle_rxa_get_queue_conf(const char *cmd
> > __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
Validate token here
> > rx_adapter_id = strtoul(token, NULL, 10);
> >
> > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > _id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
Use RTE_ETH_VALID_PORTID_OR_ERR_RET to validate eth_dev_id
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > >nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> rx_queue_id);
> > + free(l_params);
> > return -EINVAL;
> > }
> >
> > @@ -3359,6 +3364,8 @@ handle_rxa_get_queue_conf(const char *cmd
> > __rte_unused,
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id,
> > eth_dev_id,
> > rx_queue_id,
> > &queue_conf)) {
> > @@ -3396,26 +3403,31 @@ handle_rxa_get_queue_stats(const char *cmd
> > __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
Validate token here
> > rx_adapter_id = strtoul(token, NULL, 10);
> >
> > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > _id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
Use RTE_ETH_VALID_PORTID_OR_ERR_RET to validate eth_dev_id
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > >nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> rx_queue_id);
> > + free(l_params);
> > return -EINVAL;
> > }
> >
> > @@ -3423,6 +3435,8 @@ handle_rxa_get_queue_stats(const char *cmd
> > __rte_unused,
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id,
> > eth_dev_id,
> > rx_queue_id, &q_stats)) {
> > @@ -3458,26 +3472,31 @@ handle_rxa_queue_stats_reset(const char
> *cmd
> > __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
Validate token
> > rx_adapter_id = strtoul(token, NULL, 10);
> >
> > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > _id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
Use RTE_ETH_VALID_PORTID_OR_ERR_RET to validate eth_dev_id
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > + free(l_params);
> > return -1;
> > -
> > + }
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > >nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> rx_queue_id);
> > + free(l_params);
> > return -EINVAL;
> > }
> >
> > @@ -3485,6 +3504,8 @@ handle_rxa_queue_stats_reset(const char *cmd
> > __rte_unused,
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> > eth_dev_id,
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 14:54 ` Kundapura, Ganapati
@ 2022-02-02 16:47 ` Kundapura, Ganapati
0 siblings, 0 replies; 21+ messages in thread
From: Kundapura, Ganapati @ 2022-02-02 16:47 UTC (permalink / raw)
To: Weiguo Li, Jayatheerthan, Jay; +Cc: Naga Harish K, S V, dev
Hi Weiguo,
Also noticed that usage of macros like RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET and
RTE_ETH_VALID_PORTID_OR_ERR_RET to validate rx_adapter_id and eth_dev_id returns error
bypassing the freeing of memory get from strdup() in error case.
Thanks,
Ganapati
> -----Original Message-----
> From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> Sent: 02 February 2022 20:24
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Weiguo Li
> <liwg06@foxmail.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token
> parsing finished
>
> Hi Weigo,
> Could you please address the below issues also?
>
> Thanks,
> Ganapati
> > -----Original Message-----
> > From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> > Sent: 02 February 2022 13:18
> > To: Weiguo Li <liwg06@foxmail.com>; Jayatheerthan, Jay
> > <jay.jayatheerthan@intel.com>
> > Cc: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> > Subject: RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token
> > parsing finished
> >
> > Acked-by: Ganapati Kundapura<ganapati.kundapura@intel.com>
> >
> > > -----Original Message-----
> > > From: Weiguo Li <liwg06@foxmail.com>
> > > Sent: 02 February 2022 13:11
> > > To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > > Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish
> > > K, S V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> > > Subject: [PATCH v2] eventdev/eth_rx: fix memory leak when token
> > > parsing finished
> > >
> > > The memory get from strdup should be freed when parameter parsing
> > > finished, and also should be freed when error occurs.
> > >
> > > Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> > > Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
> > >
> > > Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> > > ---
> > > v2:
> > > * add memory check after strdup
> > > ---
> > > lib/eventdev/rte_event_eth_rx_adapter.c | 45 ++++++++++++++++++-
> ---
> > --
> > > -
> > > 1 file changed, 33 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > index ae1e260c08..8519c98b19 100644
> > > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > @@ -3332,26 +3332,31 @@ handle_rxa_get_queue_conf(const char
> *cmd
> > > __rte_unused,
> > >
> > > /* Get Rx adapter ID from parameter string */
> > > l_params = strdup(params);
> > > + if (l_params == NULL)
> > > + return -ENOMEM;
> > > token = strtok(l_params, ",");
> Validate token here
> > > rx_adapter_id = strtoul(token, NULL, 10);
> > >
> > > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > > _id, -EINVAL);
> > >
> > > token = strtok(NULL, ",");
> > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > > + free(l_params);
> > > return -1;
> > > -
> > > + }
> > > /* Get device ID from parameter string */
> > > eth_dev_id = strtoul(token, NULL, 10);
> > > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> Use RTE_ETH_VALID_PORTID_OR_ERR_RET to validate eth_dev_id
> > >
> > > token = strtok(NULL, ",");
> > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > > + free(l_params);
> > > return -1;
> > > -
> > > + }
> > > /* Get Rx queue ID from parameter string */
> > > rx_queue_id = strtoul(token, NULL, 10);
> > > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > > >nb_rx_queues) {
> > > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> > rx_queue_id);
> > > + free(l_params);
> > > return -EINVAL;
> > > }
> > >
> > > @@ -3359,6 +3364,8 @@ handle_rxa_get_queue_conf(const char *cmd
> > > __rte_unused,
> > > if (token != NULL)
> > > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > > " telemetry command, ignoring");
> > > + /* Parsing parameter finished */
> > > + free(l_params);
> > >
> > > if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id,
> > > eth_dev_id,
> > > rx_queue_id,
> > > &queue_conf)) {
> > > @@ -3396,26 +3403,31 @@ handle_rxa_get_queue_stats(const char
> *cmd
> > > __rte_unused,
> > >
> > > /* Get Rx adapter ID from parameter string */
> > > l_params = strdup(params);
> > > + if (l_params == NULL)
> > > + return -ENOMEM;
> > > token = strtok(l_params, ",");
> Validate token here
> > > rx_adapter_id = strtoul(token, NULL, 10);
> > >
> > > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > > _id, -EINVAL);
> > >
> > > token = strtok(NULL, ",");
> > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > > + free(l_params);
> > > return -1;
> > > -
> > > + }
> > > /* Get device ID from parameter string */
> > > eth_dev_id = strtoul(token, NULL, 10);
> > > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> Use RTE_ETH_VALID_PORTID_OR_ERR_RET to validate eth_dev_id
> > >
> > > token = strtok(NULL, ",");
> > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > > + free(l_params);
> > > return -1;
> > > -
> > > + }
> > > /* Get Rx queue ID from parameter string */
> > > rx_queue_id = strtoul(token, NULL, 10);
> > > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > > >nb_rx_queues) {
> > > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> > rx_queue_id);
> > > + free(l_params);
> > > return -EINVAL;
> > > }
> > >
> > > @@ -3423,6 +3435,8 @@ handle_rxa_get_queue_stats(const char *cmd
> > > __rte_unused,
> > > if (token != NULL)
> > > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > > " telemetry command, ignoring");
> > > + /* Parsing parameter finished */
> > > + free(l_params);
> > >
> > > if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id,
> > > eth_dev_id,
> > > rx_queue_id, &q_stats)) {
> @@ -3458,26 +3472,31 @@
> > > handle_rxa_queue_stats_reset(const char
> > *cmd
> > > __rte_unused,
> > >
> > > /* Get Rx adapter ID from parameter string */
> > > l_params = strdup(params);
> > > + if (l_params == NULL)
> > > + return -ENOMEM;
> > > token = strtok(l_params, ",");
> Validate token
> > > rx_adapter_id = strtoul(token, NULL, 10);
> > >
> > > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> > > _id, -EINVAL);
> > >
> > > token = strtok(NULL, ",");
> > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > > + free(l_params);
> > > return -1;
> > > -
> > > + }
> > > /* Get device ID from parameter string */
> > > eth_dev_id = strtoul(token, NULL, 10);
> > > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> Use RTE_ETH_VALID_PORTID_OR_ERR_RET to validate eth_dev_id
> > >
> > > token = strtok(NULL, ",");
> > > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > > + if (token == NULL || strlen(token) == 0 || !isdigit(*token)) {
> > > + free(l_params);
> > > return -1;
> > > -
> > > + }
> > > /* Get Rx queue ID from parameter string */
> > > rx_queue_id = strtoul(token, NULL, 10);
> > > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data-
> > > >nb_rx_queues) {
> > > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u",
> > rx_queue_id);
> > > + free(l_params);
> > > return -EINVAL;
> > > }
> > >
> > > @@ -3485,6 +3504,8 @@ handle_rxa_queue_stats_reset(const char
> *cmd
> > > __rte_unused,
> > > if (token != NULL)
> > > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > > " telemetry command, ignoring");
> > > + /* Parsing parameter finished */
> > > + free(l_params);
> > >
> > > if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> > > eth_dev_id,
> > > --
> > > 2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE:[PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 7:40 ` [PATCH v2] " Weiguo Li
2022-02-02 7:48 ` Kundapura, Ganapati
@ 2022-02-03 3:38 ` Weiguo Li
2022-02-03 5:44 ` Kundapura, Ganapati
2022-02-03 8:49 ` [PATCH v4] " Weiguo Li
2 siblings, 1 reply; 21+ messages in thread
From: Weiguo Li @ 2022-02-03 3:38 UTC (permalink / raw)
To: ganapati.kundapura; +Cc: jay.jayatheerthan, s.v.naga.harish.k, dev
Hi Ganapati,
Thanks for your reminding.
I'm a bit confused by "eth_dev_id validate" you metiond below.
I'll post a new patch for your review if this make clear.
Thanks,
-Weiguo
> ------------------ Original ------------------
> From: "Kundapura, Ganapati" <ganapati.kundapura@intel.com>;
> Date: Thu, Feb 3, 2022 00:47 AM
> To: "Weiguo Li"<liwg06@foxmail.com>;"Jayatheerthan, Jay"<jay.jayatheerthan@intel.com>;
> Cc: "Naga Harish K, S V"<s.v.naga.harish.k@intel.com>;"dev@dpdk.org"<dev@dpdk.org>;
> Subject: RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
>
> Hi Weiguo,
> Also noticed that usage of macros like RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET and
> RTE_ETH_VALID_PORTID_OR_ERR_RET to validate rx_adapter_id and eth_dev_id returns error
> bypassing the freeing of memory get from strdup() in error case.
>
> Thanks,
> Ganapati
> > > > token = strtok(l_params, ",");
> > Validate token
>
> > > > /* Get device ID from parameter string */
> > > > eth_dev_id = strtoul(token, NULL, 10);
> > > > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> > Use RTE_ETH_VALID_PORTID_OR_ERR_RET to validate eth_dev_id
I looked up the macro defines:
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET -- use rte_event_pmd_is_valid_dev()
RTE_ETH_VALID_PORTID_OR_ERR_RET -- use rte_eth_dev_is_valid_port()
If you mean the validate of "eth_dev_id" here be replaced by rte_eth_dev_is_valid_port() ?
or both rte_event_pmd_is_valid_dev() and rte_eth_dev_is_valid_port() used
to do the validation?
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: RE:[PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-03 3:38 ` Weiguo Li
@ 2022-02-03 5:44 ` Kundapura, Ganapati
0 siblings, 0 replies; 21+ messages in thread
From: Kundapura, Ganapati @ 2022-02-03 5:44 UTC (permalink / raw)
To: Weiguo Li; +Cc: Jayatheerthan, Jay, Naga Harish K, S V, dev
Hi Weighuo,
> -----Original Message-----
> From: Weiguo Li <liwg06@foxmail.com>
> Sent: 03 February 2022 09:08
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Naga Harish K, S V
> <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> Subject: RE:[PATCH v2] eventdev/eth_rx: fix memory leak when token
> parsing finished
>
> Hi Ganapati,
>
> Thanks for your reminding.
>
> I'm a bit confused by "eth_dev_id validate" you metiond below.
> I'll post a new patch for your review if this make clear.
>
> Thanks,
> -Weiguo
>
> > ------------------ Original ------------------
> > From: "Kundapura, Ganapati" <ganapati.kundapura@intel.com>;
> > Date: Thu, Feb 3, 2022 00:47 AM
> > To: "Weiguo Li"<liwg06@foxmail.com>;"Jayatheerthan,
> > Jay"<jay.jayatheerthan@intel.com>;
> > Cc: "Naga Harish K, S
> > V"<s.v.naga.harish.k@intel.com>;"dev@dpdk.org"<dev@dpdk.org>;
> > Subject: RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token
> > parsing finished
> >
> > Hi Weiguo,
> > Also noticed that usage of macros like
> > RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET and
> > RTE_ETH_VALID_PORTID_OR_ERR_RET to validate rx_adapter_id and
> eth_dev_id returns error bypassing the freeing of memory get from strdup()
> in error case.
> >
> > Thanks,
> > Ganapati
> > > > > token = strtok(l_params, ",");
> > > Validate token
> >
> > > > > /* Get device ID from parameter string */
> > > > > eth_dev_id = strtoul(token, NULL, 10);
> > > > > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> > > Use RTE_ETH_VALID_PORTID_OR_ERR_RET to validate eth_dev_id
>
> I looked up the macro defines:
> RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET -- use
> rte_event_pmd_is_valid_dev()
> RTE_ETH_VALID_PORTID_OR_ERR_RET -- use rte_eth_dev_is_valid_port()
>
> If you mean the validate of "eth_dev_id" here be replaced by
> rte_eth_dev_is_valid_port() ?
> or both rte_event_pmd_is_valid_dev() and rte_eth_dev_is_valid_port() used
> to do the validation?
>
>
rxa_validate_id() for validating rx_adapter_id and rte_eth_dev_is_valid_port() for
validating eth_dev_id.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 7:13 [PATCH] eventdev/eth_rx: fix memory leak when token parsing finished Weiguo Li
2022-02-02 7:40 ` [PATCH v2] " Weiguo Li
@ 2022-02-03 6:02 ` Stephen Hemminger
2022-02-03 7:18 ` [PATCH v3] " Weiguo Li
2 siblings, 0 replies; 21+ messages in thread
From: Stephen Hemminger @ 2022-02-03 6:02 UTC (permalink / raw)
To: Weiguo Li; +Cc: jay.jayatheerthan, ganapati.kundapura, s.v.naga.harish.k, dev
On Wed, 2 Feb 2022 15:13:22 +0800
Weiguo Li <liwg06@foxmail.com> wrote:
> The memory get from strdup should be freed when parameter parsing
> finished, and also should be freed when error occurs.
>
> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
strdupa() is easier to use here instead of strdup().
Also, the code is not checking for strdup failing...
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 7:13 [PATCH] eventdev/eth_rx: fix memory leak when token parsing finished Weiguo Li
2022-02-02 7:40 ` [PATCH v2] " Weiguo Li
2022-02-03 6:02 ` [PATCH] " Stephen Hemminger
@ 2022-02-03 7:18 ` Weiguo Li
2 siblings, 0 replies; 21+ messages in thread
From: Weiguo Li @ 2022-02-03 7:18 UTC (permalink / raw)
To: jay.jayatheerthan; +Cc: ganapati.kundapura, s.v.naga.harish.k, stephen, dev
The memory get from strdup should be freed when parameter parsing
finished, and also should be freed when error occurs.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
v3:
* validate "eth_dev_id" by rte_eth_dev_is_valid_port()
as Ganapati's comment.
* validate "token" by macros to reduce code redundancy.
* use new macros to avoid bypassing the freeing of memory problem.
v2:
* add memory check after strdup
---
lib/eventdev/rte_event_eth_rx_adapter.c | 90 +++++++++++++++++++------
1 file changed, 69 insertions(+), 21 deletions(-)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index ae1e260c08..886256c28c 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
} \
} while (0)
+#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
+ if (!rxa_validate_id(id)) { \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
+ if (token == NULL || strlen(token) == 0 || !isdigit(*token)) \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
+ if (!rte_eth_dev_is_valid_port(port_id)) { \
+ RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
static inline int
rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
{
@@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_conf queue_conf;
@@ -3332,26 +3356,28 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3359,6 +3385,8 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &queue_conf)) {
@@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
RXA_ADD_DICT(queue_conf.ev, flow_id);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_stats q_stats;
@@ -3396,26 +3428,28 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3423,6 +3457,8 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &q_stats)) {
@@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
RXA_ADD_DICT(q_stats, rx_dropped);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
if (params == NULL || strlen(params) == 0 || !isdigit(*params))
@@ -3458,26 +3498,28 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3485,6 +3527,8 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
eth_dev_id,
@@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
}
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
RTE_INIT(rxa_init_telemetry)
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-02 7:40 ` [PATCH v2] " Weiguo Li
2022-02-02 7:48 ` Kundapura, Ganapati
2022-02-03 3:38 ` Weiguo Li
@ 2022-02-03 8:49 ` Weiguo Li
2022-02-03 13:09 ` Kundapura, Ganapati
` (3 more replies)
2 siblings, 4 replies; 21+ messages in thread
From: Weiguo Li @ 2022-02-03 8:49 UTC (permalink / raw)
To: jay.jayatheerthan; +Cc: ganapati.kundapura, s.v.naga.harish.k, dev
The memory get from strdup should be freed when parameter parsing
finished, and also should be freed when error occurs.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
v4:
* fix compilation issue
v3:
* validate "eth_dev_id" by rte_eth_dev_is_valid_port()
as Ganapati's comment.
* validate "token" by macros to reduce code redundancy.
* use new macros to avoid bypassing the freeing of memory problem.
v2:
* add memory check after strdup
---
lib/eventdev/rte_event_eth_rx_adapter.c | 90 +++++++++++++++++++------
1 file changed, 69 insertions(+), 21 deletions(-)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index ae1e260c08..de96dbf481 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
} \
} while (0)
+#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
+ if (!rxa_validate_id(id)) { \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
+ if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
+ if (!rte_eth_dev_is_valid_port(port_id)) { \
+ RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
static inline int
rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
{
@@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_conf queue_conf;
@@ -3332,26 +3356,28 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3359,6 +3385,8 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &queue_conf)) {
@@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
RXA_ADD_DICT(queue_conf.ev, flow_id);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_stats q_stats;
@@ -3396,26 +3428,28 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3423,6 +3457,8 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &q_stats)) {
@@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
RXA_ADD_DICT(q_stats, rx_dropped);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
if (params == NULL || strlen(params) == 0 || !isdigit(*params))
@@ -3458,26 +3498,28 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3485,6 +3527,8 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
eth_dev_id,
@@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
}
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
RTE_INIT(rxa_init_telemetry)
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH v4] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-03 8:49 ` [PATCH v4] " Weiguo Li
@ 2022-02-03 13:09 ` Kundapura, Ganapati
2022-02-04 6:54 ` Jayatheerthan, Jay
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Kundapura, Ganapati @ 2022-02-03 13:09 UTC (permalink / raw)
To: Weiguo Li, Jayatheerthan, Jay; +Cc: Naga Harish K, S V, dev
Looks good to me. Thank you for catching this.
Acked-by: Ganapati Kundapura<ganapati.kundapura@intel.com>
> -----Original Message-----
> From: Weiguo Li <liwg06@foxmail.com>
> Sent: 03 February 2022 14:20
> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S
> V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> Subject: [PATCH v4] eventdev/eth_rx: fix memory leak when token parsing
> finished
>
> The memory get from strdup should be freed when parameter parsing
> finished, and also should be freed when error occurs.
>
> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
>
> ---
> v4:
> * fix compilation issue
> v3:
> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> as Ganapati's comment.
> * validate "token" by macros to reduce code redundancy.
> * use new macros to avoid bypassing the freeing of memory problem.
> v2:
> * add memory check after strdup
> ---
> lib/eventdev/rte_event_eth_rx_adapter.c | 90 +++++++++++++++++++------
> 1 file changed, 69 insertions(+), 21 deletions(-)
>
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> b/lib/eventdev/rte_event_eth_rx_adapter.c
> index ae1e260c08..de96dbf481 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter
> *rx_adapter, uint16_t eth_dev_id,
> } \
> } while (0)
>
> +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id,
> retval) do { \
> + if (!rxa_validate_id(id)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> retval) do { \
> + if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do {
> \
> + if (!rte_eth_dev_is_valid_port(port_id)) { \
> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> static inline int
> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter) {
> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd
> __rte_unused, {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
>
> @@ -3332,26 +3356,28 @@ handle_rxa_get_queue_conf(const char *cmd
> __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> -
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> _id, -EINVAL);
> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_a
> dapter_id,
> +-EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -
> EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues)
> {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> + free(l_params);
> return -EINVAL;
> }
>
> @@ -3359,6 +3385,8 @@ handle_rxa_get_queue_conf(const char *cmd
> __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id,
> eth_dev_id,
> rx_queue_id,
> &queue_conf)) {
> @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd
> __rte_unused,
> RXA_ADD_DICT(queue_conf.ev, flow_id);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd
> __rte_unused, {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_stats q_stats;
>
> @@ -3396,26 +3428,28 @@ handle_rxa_get_queue_stats(const char *cmd
> __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> -
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> _id, -EINVAL);
> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_a
> dapter_id,
> +-EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -
> EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues)
> {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> + free(l_params);
> return -EINVAL;
> }
>
> @@ -3423,6 +3457,8 @@ handle_rxa_get_queue_stats(const char *cmd
> __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id,
> eth_dev_id,
> rx_queue_id, &q_stats)) {
> @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd
> __rte_unused,
> RXA_ADD_DICT(q_stats, rx_dropped);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused, {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
>
> if (params == NULL || strlen(params) == 0 || !isdigit(*params)) @@ -
> 3458,26 +3498,28 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> -
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter
> _id, -EINVAL);
> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_a
> dapter_id,
> +-EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -
> EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET
> (token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues)
> {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> + free(l_params);
> return -EINVAL;
> }
>
> @@ -3485,6 +3527,8 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> eth_dev_id,
> @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
> }
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> RTE_INIT(rxa_init_telemetry)
> --
> 2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-03 8:49 ` [PATCH v4] " Weiguo Li
2022-02-03 13:09 ` Kundapura, Ganapati
@ 2022-02-04 6:54 ` Jayatheerthan, Jay
2022-02-04 7:49 ` [PATCH v5] " Weiguo Li
2022-02-04 8:21 ` [PATCH v6] " Weiguo Li
3 siblings, 0 replies; 21+ messages in thread
From: Jayatheerthan, Jay @ 2022-02-04 6:54 UTC (permalink / raw)
To: Weiguo Li; +Cc: Kundapura, Ganapati, Naga Harish K, S V, dev
Hi Weiguo,
> -----Original Message-----
> From: Weiguo Li <liwg06@foxmail.com>
> Sent: Thursday, February 3, 2022 2:20 PM
> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> Subject: [PATCH v4] eventdev/eth_rx: fix memory leak when token parsing finished
>
> The memory get from strdup should be freed when parameter parsing
> finished, and also should be freed when error occurs.
>
> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
>
> ---
> v4:
> * fix compilation issue
> v3:
> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> as Ganapati's comment.
> * validate "token" by macros to reduce code redundancy.
> * use new macros to avoid bypassing the freeing of memory problem.
> v2:
> * add memory check after strdup
> ---
> lib/eventdev/rte_event_eth_rx_adapter.c | 90 +++++++++++++++++++------
> 1 file changed, 69 insertions(+), 21 deletions(-)
>
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> index ae1e260c08..de96dbf481 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
> } \
> } while (0)
>
> +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
> + if (!rxa_validate_id(id)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
> + if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
> + if (!rte_eth_dev_is_valid_port(port_id)) { \
> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> static inline int
> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
> {
> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
>
> @@ -3332,26 +3356,28 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> + free(l_params);
could set ret and goto error.
> return -EINVAL;
> }
>
> @@ -3359,6 +3385,8 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
> rx_queue_id, &queue_conf)) {
> @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> RXA_ADD_DICT(queue_conf.ev, flow_id);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_stats q_stats;
>
> @@ -3396,26 +3428,28 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> + free(l_params);
same as above.
> return -EINVAL;
> }
>
> @@ -3423,6 +3457,8 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
> rx_queue_id, &q_stats)) {
> @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> RXA_ADD_DICT(q_stats, rx_dropped);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
>
> if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> @@ -3458,26 +3498,28 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> + free(l_params);
same as above.
> return -EINVAL;
> }
>
> @@ -3485,6 +3527,8 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> eth_dev_id,
> @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> }
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> RTE_INIT(rxa_init_telemetry)
> --
> 2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-03 8:49 ` [PATCH v4] " Weiguo Li
2022-02-03 13:09 ` Kundapura, Ganapati
2022-02-04 6:54 ` Jayatheerthan, Jay
@ 2022-02-04 7:49 ` Weiguo Li
2022-02-04 7:57 ` Jayatheerthan, Jay
2022-02-04 8:21 ` [PATCH v6] " Weiguo Li
3 siblings, 1 reply; 21+ messages in thread
From: Weiguo Li @ 2022-02-04 7:49 UTC (permalink / raw)
To: jay.jayatheerthan; +Cc: ganapati.kundapura, s.v.naga.harish.k, stephen, dev
The memory get from strdup should be freed when parameter parsing
finished, and also should be freed when error occurs.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
Acked-by: Ganapati Kundapura<ganapati.kundapura@intel.com>
---
v5:
* fix according to Jay's comment.
* fix anothor bypassing the freeing of memory problem
v4:
* fix compilation issue
v3:
* validate "eth_dev_id" by rte_eth_dev_is_valid_port()
as Ganapati's comment.
* validate "token" by macros to reduce code redundancy.
* use new macros to avoid bypassing the freeing of memory problem.
v2:
* add memory check after strdup
---
lib/eventdev/rte_event_eth_rx_adapter.c | 105 ++++++++++++++++++------
1 file changed, 78 insertions(+), 27 deletions(-)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index ae1e260c08..719ef520cd 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
} \
} while (0)
+#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
+ if (!rxa_validate_id(id)) { \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
+ if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
+ if (!rte_eth_dev_is_valid_port(port_id)) { \
+ RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
static inline int
rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
{
@@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_conf queue_conf;
@@ -3332,38 +3356,43 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &queue_conf)) {
RTE_EDEV_LOG_ERR("Failed to get Rx adapter queue config");
- return -1;
+ ret = -1;
+ goto error;
}
rte_tel_data_start_dict(d);
@@ -3378,6 +3407,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
RXA_ADD_DICT(queue_conf.ev, flow_id);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3387,7 +3420,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_stats q_stats;
@@ -3396,38 +3429,43 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &q_stats)) {
RTE_EDEV_LOG_ERR("Failed to get Rx adapter queue stats");
- return -1;
+ ret = -1;
+ goto error;
}
rte_tel_data_start_dict(d);
@@ -3441,6 +3479,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
RXA_ADD_DICT(q_stats, rx_dropped);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3450,7 +3492,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
if (params == NULL || strlen(params) == 0 || !isdigit(*params))
@@ -3458,42 +3500,51 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
eth_dev_id,
rx_queue_id)) {
RTE_EDEV_LOG_ERR("Failed to reset Rx adapter queue stats");
- return -1;
+ ret = -1;
+ goto error;
}
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
RTE_INIT(rxa_init_telemetry)
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH v5] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-04 7:49 ` [PATCH v5] " Weiguo Li
@ 2022-02-04 7:57 ` Jayatheerthan, Jay
0 siblings, 0 replies; 21+ messages in thread
From: Jayatheerthan, Jay @ 2022-02-04 7:57 UTC (permalink / raw)
To: Weiguo Li; +Cc: Kundapura, Ganapati, Naga Harish K, S V, stephen, dev
Hi Weiguo,
> -----Original Message-----
> From: Weiguo Li <liwg06@foxmail.com>
> Sent: Friday, February 4, 2022 1:20 PM
> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S V <s.v.naga.harish.k@intel.com>;
> stephen@networkplumber.org; dev@dpdk.org
> Subject: [PATCH v5] eventdev/eth_rx: fix memory leak when token parsing finished
>
> The memory get from strdup should be freed when parameter parsing
> finished, and also should be freed when error occurs.
>
> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> Acked-by: Ganapati Kundapura<ganapati.kundapura@intel.com>
>
> ---
> v5:
> * fix according to Jay's comment.
> * fix anothor bypassing the freeing of memory problem
> v4:
> * fix compilation issue
> v3:
> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> as Ganapati's comment.
> * validate "token" by macros to reduce code redundancy.
> * use new macros to avoid bypassing the freeing of memory problem.
> v2:
> * add memory check after strdup
> ---
> lib/eventdev/rte_event_eth_rx_adapter.c | 105 ++++++++++++++++++------
> 1 file changed, 78 insertions(+), 27 deletions(-)
>
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> index ae1e260c08..719ef520cd 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
> } \
> } while (0)
>
> +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
> + if (!rxa_validate_id(id)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
> + if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
> + if (!rte_eth_dev_is_valid_port(port_id)) { \
> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> static inline int
> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
> {
> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
>
> @@ -3332,38 +3356,43 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
> rx_queue_id, &queue_conf)) {
> RTE_EDEV_LOG_ERR("Failed to get Rx adapter queue config");
> - return -1;
> + ret = -1;
> + goto error;
This causes double free of l_params. It is already freed above and then one more time in error section.
If you want you can more above free just before return 0;
> }
>
> rte_tel_data_start_dict(d);
> @@ -3378,6 +3407,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> RXA_ADD_DICT(queue_conf.ev, flow_id);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3387,7 +3420,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_stats q_stats;
>
> @@ -3396,38 +3429,43 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
> rx_queue_id, &q_stats)) {
> RTE_EDEV_LOG_ERR("Failed to get Rx adapter queue stats");
> - return -1;
> + ret = -1;
> + goto error;
same here.
> }
>
> rte_tel_data_start_dict(d);
> @@ -3441,6 +3479,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> RXA_ADD_DICT(q_stats, rx_dropped);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3450,7 +3492,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
>
> if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> @@ -3458,42 +3500,51 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> eth_dev_id,
> rx_queue_id)) {
> RTE_EDEV_LOG_ERR("Failed to reset Rx adapter queue stats");
> - return -1;
> + ret = -1;
> + goto error;
same here.
> }
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> RTE_INIT(rxa_init_telemetry)
> --
> 2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-03 8:49 ` [PATCH v4] " Weiguo Li
` (2 preceding siblings ...)
2022-02-04 7:49 ` [PATCH v5] " Weiguo Li
@ 2022-02-04 8:21 ` Weiguo Li
2022-02-04 9:47 ` Jayatheerthan, Jay
3 siblings, 1 reply; 21+ messages in thread
From: Weiguo Li @ 2022-02-04 8:21 UTC (permalink / raw)
To: jay.jayatheerthan; +Cc: ganapati.kundapura, s.v.naga.harish.k, stephen, dev
The memory get from strdup should be freed when parameter parsing
finished, and also should be freed when error occurs.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
---
v6:
* fix double free problem in v5 as Jay noted
v5:
* fix according to Jay's comment.
* fix anothor bypassing the freeing of memory problem
v4:
* fix compilation issue
v3:
* validate "eth_dev_id" by rte_eth_dev_is_valid_port()
as Ganapati's comment.
* validate "token" by macros to reduce code redundancy.
* use new macros to avoid bypassing the freeing of memory problem.
v2:
* add memory check after strdup
---
lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
1 file changed, 72 insertions(+), 24 deletions(-)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index ae1e260c08..4da6f1ff90 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
} \
} while (0)
+#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
+ if (!rxa_validate_id(id)) { \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
+ if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
+ if (!rte_eth_dev_is_valid_port(port_id)) { \
+ RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
static inline int
rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
{
@@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_conf queue_conf;
@@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &queue_conf)) {
@@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
RXA_ADD_DICT(queue_conf.ev, flow_id);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_stats q_stats;
@@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &q_stats)) {
@@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
RXA_ADD_DICT(q_stats, rx_dropped);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
if (params == NULL || strlen(params) == 0 || !isdigit(*params))
@@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
- RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
- RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
eth_dev_id,
@@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
}
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
RTE_INIT(rxa_init_telemetry)
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-04 8:21 ` [PATCH v6] " Weiguo Li
@ 2022-02-04 9:47 ` Jayatheerthan, Jay
2022-02-18 9:39 ` Jerin Jacob
0 siblings, 1 reply; 21+ messages in thread
From: Jayatheerthan, Jay @ 2022-02-04 9:47 UTC (permalink / raw)
To: Weiguo Li; +Cc: Kundapura, Ganapati, Naga Harish K, S V, stephen, dev
Looks good. Thanks Weiguo for posting this patch and addressing all the comments!
Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com>
-Jay
> -----Original Message-----
> From: Weiguo Li <liwg06@foxmail.com>
> Sent: Friday, February 4, 2022 1:51 PM
> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S V <s.v.naga.harish.k@intel.com>;
> stephen@networkplumber.org; dev@dpdk.org
> Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
>
> The memory get from strdup should be freed when parameter parsing
> finished, and also should be freed when error occurs.
>
> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
>
> ---
> v6:
> * fix double free problem in v5 as Jay noted
> v5:
> * fix according to Jay's comment.
> * fix anothor bypassing the freeing of memory problem
> v4:
> * fix compilation issue
> v3:
> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> as Ganapati's comment.
> * validate "token" by macros to reduce code redundancy.
> * use new macros to avoid bypassing the freeing of memory problem.
> v2:
> * add memory check after strdup
> ---
> lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
> 1 file changed, 72 insertions(+), 24 deletions(-)
>
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> index ae1e260c08..4da6f1ff90 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
> } \
> } while (0)
>
> +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
> + if (!rxa_validate_id(id)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
> + if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
> + if (!rte_eth_dev_is_valid_port(port_id)) { \
> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> static inline int
> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
> {
> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
>
> @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
> rx_queue_id, &queue_conf)) {
> @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> RXA_ADD_DICT(queue_conf.ev, flow_id);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_stats q_stats;
>
> @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
> rx_queue_id, &q_stats)) {
> @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> RXA_ADD_DICT(q_stats, rx_dropped);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
>
> if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> @@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> rx_adapter_id = strtoul(token, NULL, 10);
> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get device ID from parameter string */
> eth_dev_id = strtoul(token, NULL, 10);
> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* Get Rx queue ID from parameter string */
> rx_queue_id = strtoul(token, NULL, 10);
> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> eth_dev_id,
> @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> }
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> RTE_INIT(rxa_init_telemetry)
> --
> 2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-04 9:47 ` Jayatheerthan, Jay
@ 2022-02-18 9:39 ` Jerin Jacob
2022-02-22 10:08 ` Kevin Traynor
0 siblings, 1 reply; 21+ messages in thread
From: Jerin Jacob @ 2022-02-18 9:39 UTC (permalink / raw)
To: Jayatheerthan, Jay
Cc: Weiguo Li, Kundapura, Ganapati, Naga Harish K, S V, stephen, dev
On Fri, Feb 4, 2022 at 3:17 PM Jayatheerthan, Jay
<jay.jayatheerthan@intel.com> wrote:
>
> Looks good. Thanks Weiguo for posting this patch and addressing all the comments!
>
> Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com>
Applied to dpdk-next-net-eventdev/for-main. Thanks
>
> -Jay
>
>
>
> > -----Original Message-----
> > From: Weiguo Li <liwg06@foxmail.com>
> > Sent: Friday, February 4, 2022 1:51 PM
> > To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S V <s.v.naga.harish.k@intel.com>;
> > stephen@networkplumber.org; dev@dpdk.org
> > Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
> >
> > The memory get from strdup should be freed when parameter parsing
> > finished, and also should be freed when error occurs.
> >
> > Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> > Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
> >
> > Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> > Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> >
> > ---
> > v6:
> > * fix double free problem in v5 as Jay noted
> > v5:
> > * fix according to Jay's comment.
> > * fix anothor bypassing the freeing of memory problem
> > v4:
> > * fix compilation issue
> > v3:
> > * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> > as Ganapati's comment.
> > * validate "token" by macros to reduce code redundancy.
> > * use new macros to avoid bypassing the freeing of memory problem.
> > v2:
> > * add memory check after strdup
> > ---
> > lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
> > 1 file changed, 72 insertions(+), 24 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> > index ae1e260c08..4da6f1ff90 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
> > } \
> > } while (0)
> >
> > +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
> > + if (!rxa_validate_id(id)) { \
> > + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
> > + ret = retval; \
> > + goto error; \
> > + } \
> > +} while (0)
> > +
> > +#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
> > + if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
> > + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> > + ret = retval; \
> > + goto error; \
> > + } \
> > +} while (0)
> > +
> > +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
> > + if (!rte_eth_dev_is_valid_port(port_id)) { \
> > + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> > + ret = retval; \
> > + goto error; \
> > + } \
> > +} while (0)
> > +
> > static inline int
> > rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
> > {
> > @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> > {
> > uint8_t rx_adapter_id;
> > uint16_t rx_queue_id;
> > - int eth_dev_id;
> > + int eth_dev_id, ret = -1;
> > char *token, *l_params;
> > struct rte_event_eth_rx_adapter_queue_conf queue_conf;
> >
> > @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> > rx_adapter_id = strtoul(token, NULL, 10);
> > - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto error;
> > }
> >
> > token = strtok(NULL, "\0");
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
> > rx_queue_id, &queue_conf)) {
> > @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> > RXA_ADD_DICT(queue_conf.ev, flow_id);
> >
> > return 0;
> > +
> > +error:
> > + free(l_params);
> > + return ret;
> > }
> >
> > static int
> > @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> > {
> > uint8_t rx_adapter_id;
> > uint16_t rx_queue_id;
> > - int eth_dev_id;
> > + int eth_dev_id, ret = -1;
> > char *token, *l_params;
> > struct rte_event_eth_rx_adapter_queue_stats q_stats;
> >
> > @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> > rx_adapter_id = strtoul(token, NULL, 10);
> > - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto error;
> > }
> >
> > token = strtok(NULL, "\0");
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
> > rx_queue_id, &q_stats)) {
> > @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> > RXA_ADD_DICT(q_stats, rx_dropped);
> >
> > return 0;
> > +
> > +error:
> > + free(l_params);
> > + return ret;
> > }
> >
> > static int
> > @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> > {
> > uint8_t rx_adapter_id;
> > uint16_t rx_queue_id;
> > - int eth_dev_id;
> > + int eth_dev_id, ret = -1;
> > char *token, *l_params;
> >
> > if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> > @@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> > rx_adapter_id = strtoul(token, NULL, 10);
> > - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
> > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* Get device ID from parameter string */
> > eth_dev_id = strtoul(token, NULL, 10);
> > - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* Get Rx queue ID from parameter string */
> > rx_queue_id = strtoul(token, NULL, 10);
> > if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> > RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto error;
> > }
> >
> > token = strtok(NULL, "\0");
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> > eth_dev_id,
> > @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> > }
> >
> > return 0;
> > +
> > +error:
> > + free(l_params);
> > + return ret;
> > }
> >
> > RTE_INIT(rxa_init_telemetry)
> > --
> > 2.25.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-18 9:39 ` Jerin Jacob
@ 2022-02-22 10:08 ` Kevin Traynor
2022-02-23 7:21 ` Naga Harish K, S V
0 siblings, 1 reply; 21+ messages in thread
From: Kevin Traynor @ 2022-02-22 10:08 UTC (permalink / raw)
To: Jerin Jacob, Jayatheerthan, Jay
Cc: Weiguo Li, Kundapura, Ganapati, Naga Harish K, S V, stephen, dev
On 18/02/2022 09:39, Jerin Jacob wrote:
> On Fri, Feb 4, 2022 at 3:17 PM Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com> wrote:
>>
>> Looks good. Thanks Weiguo for posting this patch and addressing all the comments!
>>
>> Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com>
>
> Applied to dpdk-next-net-eventdev/for-main. Thanks
>
>
>>
>> -Jay
>>
>>
>>
>>> -----Original Message-----
>>> From: Weiguo Li <liwg06@foxmail.com>
>>> Sent: Friday, February 4, 2022 1:51 PM
>>> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
>>> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S V <s.v.naga.harish.k@intel.com>;
>>> stephen@networkplumber.org; dev@dpdk.org
>>> Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
>>>
>>> The memory get from strdup should be freed when parameter parsing
>>> finished, and also should be freed when error occurs.
>>>
>>> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
>>> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>>>
No stable tag - is it wanted for relevant stable branches?
>>> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
>>> Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
>>>
>>> ---
>>> v6:
>>> * fix double free problem in v5 as Jay noted
>>> v5:
>>> * fix according to Jay's comment.
>>> * fix anothor bypassing the freeing of memory problem
>>> v4:
>>> * fix compilation issue
>>> v3:
>>> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
>>> as Ganapati's comment.
>>> * validate "token" by macros to reduce code redundancy.
>>> * use new macros to avoid bypassing the freeing of memory problem.
>>> v2:
>>> * add memory check after strdup
>>> ---
>>> lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
>>> 1 file changed, 72 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
>>> index ae1e260c08..4da6f1ff90 100644
>>> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
>>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
>>> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
>>> } \
>>> } while (0)
>>>
>>> +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
>>> + if (!rxa_validate_id(id)) { \
>>> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
>>> + ret = retval; \
>>> + goto error; \
>>> + } \
>>> +} while (0)
>>> +
>>> +#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
>>> + if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
>>> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
>>> + ret = retval; \
>>> + goto error; \
>>> + } \
>>> +} while (0)
>>> +
>>> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
>>> + if (!rte_eth_dev_is_valid_port(port_id)) { \
>>> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
>>> + ret = retval; \
>>> + goto error; \
>>> + } \
>>> +} while (0)
>>> +
>>> static inline int
>>> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
>>> {
>>> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>>> {
>>> uint8_t rx_adapter_id;
>>> uint16_t rx_queue_id;
>>> - int eth_dev_id;
>>> + int eth_dev_id, ret = -1;
>>> char *token, *l_params;
>>> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
>>>
>>> @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>>>
>>> /* Get Rx adapter ID from parameter string */
>>> l_params = strdup(params);
>>> + if (l_params == NULL)
>>> + return -ENOMEM;
>>> token = strtok(l_params, ",");
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>> rx_adapter_id = strtoul(token, NULL, 10);
>>> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
>>> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* Get device ID from parameter string */
>>> eth_dev_id = strtoul(token, NULL, 10);
>>> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* Get Rx queue ID from parameter string */
>>> rx_queue_id = strtoul(token, NULL, 10);
>>> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
>>> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
>>> - return -EINVAL;
>>> + ret = -EINVAL;
>>> + goto error;
>>> }
>>>
>>> token = strtok(NULL, "\0");
>>> if (token != NULL)
>>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
>>> " telemetry command, ignoring");
>>> + /* Parsing parameter finished */
>>> + free(l_params);
>>>
>>> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
>>> rx_queue_id, &queue_conf)) {
>>> @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>>> RXA_ADD_DICT(queue_conf.ev, flow_id);
>>>
>>> return 0;
>>> +
>>> +error:
>>> + free(l_params);
>>> + return ret;
>>> }
>>>
>>> static int
>>> @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>>> {
>>> uint8_t rx_adapter_id;
>>> uint16_t rx_queue_id;
>>> - int eth_dev_id;
>>> + int eth_dev_id, ret = -1;
>>> char *token, *l_params;
>>> struct rte_event_eth_rx_adapter_queue_stats q_stats;
>>>
>>> @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>>>
>>> /* Get Rx adapter ID from parameter string */
>>> l_params = strdup(params);
>>> + if (l_params == NULL)
>>> + return -ENOMEM;
>>> token = strtok(l_params, ",");
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>> rx_adapter_id = strtoul(token, NULL, 10);
>>> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
>>> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* Get device ID from parameter string */
>>> eth_dev_id = strtoul(token, NULL, 10);
>>> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* Get Rx queue ID from parameter string */
>>> rx_queue_id = strtoul(token, NULL, 10);
>>> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
>>> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
>>> - return -EINVAL;
>>> + ret = -EINVAL;
>>> + goto error;
>>> }
>>>
>>> token = strtok(NULL, "\0");
>>> if (token != NULL)
>>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
>>> " telemetry command, ignoring");
>>> + /* Parsing parameter finished */
>>> + free(l_params);
>>>
>>> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
>>> rx_queue_id, &q_stats)) {
>>> @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>>> RXA_ADD_DICT(q_stats, rx_dropped);
>>>
>>> return 0;
>>> +
>>> +error:
>>> + free(l_params);
>>> + return ret;
>>> }
>>>
>>> static int
>>> @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>>> {
>>> uint8_t rx_adapter_id;
>>> uint16_t rx_queue_id;
>>> - int eth_dev_id;
>>> + int eth_dev_id, ret = -1;
>>> char *token, *l_params;
>>>
>>> if (params == NULL || strlen(params) == 0 || !isdigit(*params))
>>> @@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>>>
>>> /* Get Rx adapter ID from parameter string */
>>> l_params = strdup(params);
>>> + if (l_params == NULL)
>>> + return -ENOMEM;
>>> token = strtok(l_params, ",");
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>> rx_adapter_id = strtoul(token, NULL, 10);
>>> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
>>> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* Get device ID from parameter string */
>>> eth_dev_id = strtoul(token, NULL, 10);
>>> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* Get Rx queue ID from parameter string */
>>> rx_queue_id = strtoul(token, NULL, 10);
>>> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
>>> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
>>> - return -EINVAL;
>>> + ret = -EINVAL;
>>> + goto error;
>>> }
>>>
>>> token = strtok(NULL, "\0");
>>> if (token != NULL)
>>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
>>> " telemetry command, ignoring");
>>> + /* Parsing parameter finished */
>>> + free(l_params);
>>>
>>> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
>>> eth_dev_id,
>>> @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>>> }
>>>
>>> return 0;
>>> +
>>> +error:
>>> + free(l_params);
>>> + return ret;
>>> }
>>>
>>> RTE_INIT(rxa_init_telemetry)
>>> --
>>> 2.25.1
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
2022-02-22 10:08 ` Kevin Traynor
@ 2022-02-23 7:21 ` Naga Harish K, S V
0 siblings, 0 replies; 21+ messages in thread
From: Naga Harish K, S V @ 2022-02-23 7:21 UTC (permalink / raw)
To: Kevin Traynor, Jerin Jacob, Jayatheerthan, Jay
Cc: Weiguo Li, Kundapura, Ganapati, stephen, dev
Hi Kevin,
> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Tuesday, February 22, 2022 3:39 PM
> To: Jerin Jacob <jerinjacobk@gmail.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>
> Cc: Weiguo Li <liwg06@foxmail.com>; Kundapura, Ganapati
> <Ganapati.Kundapura@intel.com>; Naga Harish K, S V
> <s.v.naga.harish.k@intel.com>; stephen@networkplumber.org; dev@dpdk.org
> Subject: Re: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing
> finished
>
> On 18/02/2022 09:39, Jerin Jacob wrote:
> > On Fri, Feb 4, 2022 at 3:17 PM Jayatheerthan, Jay
> > <jay.jayatheerthan@intel.com> wrote:
> >>
> >> Looks good. Thanks Weiguo for posting this patch and addressing all the
> comments!
> >>
> >> Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com>
> >
> > Applied to dpdk-next-net-eventdev/for-main. Thanks
> >
> >
> >>
> >> -Jay
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Weiguo Li <liwg06@foxmail.com>
> >>> Sent: Friday, February 4, 2022 1:51 PM
> >>> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> >>> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish
> >>> K, S V <s.v.naga.harish.k@intel.com>; stephen@networkplumber.org;
> >>> dev@dpdk.org
> >>> Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token
> >>> parsing finished
> >>>
> >>> The memory get from strdup should be freed when parameter parsing
> >>> finished, and also should be freed when error occurs.
> >>>
> >>> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> >>> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
> >>>
>
> No stable tag - is it wanted for relevant stable branches?
>
Yes, this patch need to be included in relevant stable releases.
> >>> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> >>> Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> >>>
> >>> ---
> >>> v6:
> >>> * fix double free problem in v5 as Jay noted
> >>> v5:
> >>> * fix according to Jay's comment.
> >>> * fix anothor bypassing the freeing of memory problem
> >>> v4:
> >>> * fix compilation issue
> >>> v3:
> >>> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> >>> as Ganapati's comment.
> >>> * validate "token" by macros to reduce code redundancy.
> >>> * use new macros to avoid bypassing the freeing of memory problem.
> >>> v2:
> >>> * add memory check after strdup
> >>> ---
> >>> lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
> >>> 1 file changed, 72 insertions(+), 24 deletions(-)
> >>>
> >>> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> b/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> index ae1e260c08..4da6f1ff90 100644
> >>> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter
> *rx_adapter, uint16_t eth_dev_id,
> >>> } \
> >>> } while (0)
> >>>
> >>> +#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id,
> retval) do { \
> >>> + if (!rxa_validate_id(id)) { \
> >>> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d\n", id); \
> >>> + ret = retval; \
> >>> + goto error; \
> >>> + } \
> >>> +} while (0)
> >>> +
> >>> +#define
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> retval) do { \
> >>> + if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
> >>> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> >>> + ret = retval; \
> >>> + goto error; \
> >>> + } \
> >>> +} while (0)
> >>> +
> >>> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do
> { \
> >>> + if (!rte_eth_dev_is_valid_port(port_id)) { \
> >>> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> >>> + ret = retval; \
> >>> + goto error; \
> >>> + } \
> >>> +} while (0)
> >>> +
> >>> static inline int
> >>> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
> >>> {
> >>> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd
> __rte_unused,
> >>> {
> >>> uint8_t rx_adapter_id;
> >>> uint16_t rx_queue_id;
> >>> - int eth_dev_id;
> >>> + int eth_dev_id, ret = -1;
> >>> char *token, *l_params;
> >>> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
> >>>
> >>> @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd
> >>> __rte_unused,
> >>>
> >>> /* Get Rx adapter ID from parameter string */
> >>> l_params = strdup(params);
> >>> + if (l_params == NULL)
> >>> + return -ENOMEM;
> >>> token = strtok(l_params, ",");
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>> rx_adapter_id = strtoul(token, NULL, 10);
> >>> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id,
> -EINVAL);
> >>> +
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id,
> >>> + -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* Get device ID from parameter string */
> >>> eth_dev_id = strtoul(token, NULL, 10);
> >>> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* Get Rx queue ID from parameter string */
> >>> rx_queue_id = strtoul(token, NULL, 10);
> >>> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> >>> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> >>> - return -EINVAL;
> >>> + ret = -EINVAL;
> >>> + goto error;
> >>> }
> >>>
> >>> token = strtok(NULL, "\0");
> >>> if (token != NULL)
> >>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> >>> " telemetry command, ignoring");
> >>> + /* Parsing parameter finished */
> >>> + free(l_params);
> >>>
> >>> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id,
> eth_dev_id,
> >>> rx_queue_id,
> >>> &queue_conf)) { @@ -3378,6 +3406,10 @@
> handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> >>> RXA_ADD_DICT(queue_conf.ev, flow_id);
> >>>
> >>> return 0;
> >>> +
> >>> +error:
> >>> + free(l_params);
> >>> + return ret;
> >>> }
> >>>
> >>> static int
> >>> @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd
> __rte_unused,
> >>> {
> >>> uint8_t rx_adapter_id;
> >>> uint16_t rx_queue_id;
> >>> - int eth_dev_id;
> >>> + int eth_dev_id, ret = -1;
> >>> char *token, *l_params;
> >>> struct rte_event_eth_rx_adapter_queue_stats q_stats;
> >>>
> >>> @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd
> >>> __rte_unused,
> >>>
> >>> /* Get Rx adapter ID from parameter string */
> >>> l_params = strdup(params);
> >>> + if (l_params == NULL)
> >>> + return -ENOMEM;
> >>> token = strtok(l_params, ",");
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>> rx_adapter_id = strtoul(token, NULL, 10);
> >>> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id,
> -EINVAL);
> >>> +
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id,
> >>> + -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* Get device ID from parameter string */
> >>> eth_dev_id = strtoul(token, NULL, 10);
> >>> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* Get Rx queue ID from parameter string */
> >>> rx_queue_id = strtoul(token, NULL, 10);
> >>> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> >>> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> >>> - return -EINVAL;
> >>> + ret = -EINVAL;
> >>> + goto error;
> >>> }
> >>>
> >>> token = strtok(NULL, "\0");
> >>> if (token != NULL)
> >>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> >>> " telemetry command, ignoring");
> >>> + /* Parsing parameter finished */
> >>> + free(l_params);
> >>>
> >>> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id,
> eth_dev_id,
> >>> rx_queue_id,
> >>> &q_stats)) { @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const
> char *cmd __rte_unused,
> >>> RXA_ADD_DICT(q_stats, rx_dropped);
> >>>
> >>> return 0;
> >>> +
> >>> +error:
> >>> + free(l_params);
> >>> + return ret;
> >>> }
> >>>
> >>> static int
> >>> @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
> >>> {
> >>> uint8_t rx_adapter_id;
> >>> uint16_t rx_queue_id;
> >>> - int eth_dev_id;
> >>> + int eth_dev_id, ret = -1;
> >>> char *token, *l_params;
> >>>
> >>> if (params == NULL || strlen(params) == 0 ||
> >>> !isdigit(*params)) @@ -3458,33 +3498,37 @@
> >>> handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> >>>
> >>> /* Get Rx adapter ID from parameter string */
> >>> l_params = strdup(params);
> >>> + if (l_params == NULL)
> >>> + return -ENOMEM;
> >>> token = strtok(l_params, ",");
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>> rx_adapter_id = strtoul(token, NULL, 10);
> >>> - RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id,
> -EINVAL);
> >>> +
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id,
> >>> + -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* Get device ID from parameter string */
> >>> eth_dev_id = strtoul(token, NULL, 10);
> >>> - RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* Get Rx queue ID from parameter string */
> >>> rx_queue_id = strtoul(token, NULL, 10);
> >>> if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
> >>> RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
> >>> - return -EINVAL;
> >>> + ret = -EINVAL;
> >>> + goto error;
> >>> }
> >>>
> >>> token = strtok(NULL, "\0");
> >>> if (token != NULL)
> >>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> >>> " telemetry command, ignoring");
> >>> + /* Parsing parameter finished */
> >>> + free(l_params);
> >>>
> >>> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> >>> eth_dev_id, @@
> >>> -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
> >>> }
> >>>
> >>> return 0;
> >>> +
> >>> +error:
> >>> + free(l_params);
> >>> + return ret;
> >>> }
> >>>
> >>> RTE_INIT(rxa_init_telemetry)
> >>> --
> >>> 2.25.1
> >>
> >
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] eventdev/eth_rx: fix memory leak when token parsing finished
@ 2022-02-02 6:15 Weiguo Li
0 siblings, 0 replies; 21+ messages in thread
From: Weiguo Li @ 2022-02-02 6:15 UTC (permalink / raw)
To: jay.jayatheerthan; +Cc: ganapati.kundapura, s.v.naga.harish.k, dev
The memory get from strdup should be freed when parameter parsing
finished, and also should be freed when error occurs.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
lib/eventdev/rte_event_eth_rx_adapter.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index ae1e260c08..4173a52981 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -3338,6 +3338,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
token = strtok(NULL, ",");
if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ free(l_params);
return -1;
/* Get device ID from parameter string */
@@ -3346,12 +3347,14 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
token = strtok(NULL, ",");
if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ free(l_params);
return -1;
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3359,6 +3362,8 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &queue_conf)) {
@@ -3402,6 +3407,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
token = strtok(NULL, ",");
if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ free(l_params);
return -1;
/* Get device ID from parameter string */
@@ -3410,12 +3416,14 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
token = strtok(NULL, ",");
if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ free(l_params);
return -1;
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3423,6 +3431,8 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &q_stats)) {
@@ -3464,6 +3474,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
token = strtok(NULL, ",");
if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ free(l_params);
return -1;
/* Get device ID from parameter string */
@@ -3472,12 +3483,14 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
token = strtok(NULL, ",");
if (token == NULL || strlen(token) == 0 || !isdigit(*token))
+ free(l_params);
return -1;
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ free(l_params);
return -EINVAL;
}
@@ -3485,6 +3498,8 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
eth_dev_id,
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-02-23 7:21 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-02 7:13 [PATCH] eventdev/eth_rx: fix memory leak when token parsing finished Weiguo Li
2022-02-02 7:40 ` [PATCH v2] " Weiguo Li
2022-02-02 7:48 ` Kundapura, Ganapati
2022-02-02 8:25 ` Jayatheerthan, Jay
2022-02-02 14:54 ` Kundapura, Ganapati
2022-02-02 16:47 ` Kundapura, Ganapati
2022-02-03 3:38 ` Weiguo Li
2022-02-03 5:44 ` Kundapura, Ganapati
2022-02-03 8:49 ` [PATCH v4] " Weiguo Li
2022-02-03 13:09 ` Kundapura, Ganapati
2022-02-04 6:54 ` Jayatheerthan, Jay
2022-02-04 7:49 ` [PATCH v5] " Weiguo Li
2022-02-04 7:57 ` Jayatheerthan, Jay
2022-02-04 8:21 ` [PATCH v6] " Weiguo Li
2022-02-04 9:47 ` Jayatheerthan, Jay
2022-02-18 9:39 ` Jerin Jacob
2022-02-22 10:08 ` Kevin Traynor
2022-02-23 7:21 ` Naga Harish K, S V
2022-02-03 6:02 ` [PATCH] " Stephen Hemminger
2022-02-03 7:18 ` [PATCH v3] " Weiguo Li
-- strict thread matches above, loose matches on Subject: below --
2022-02-02 6:15 [PATCH] " Weiguo Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.