[PATCH] eventdev/eth_rx: fix memory leak when token parsing finished

[PATCH] eventdev/eth_rx: fix memory leak when token parsing finished

[Weiguo Li]

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

[PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished

[Weiguo Li]

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

RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished

[Kundapura, Ganapati]

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

RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished

[Jayatheerthan, Jay]

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

RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished

[Kundapura, Ganapati]

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

RE: [PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished

[Kundapura, Ganapati]

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

RE:[PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished

[Weiguo Li]

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?
	
	

RE: RE:[PATCH v2] eventdev/eth_rx: fix memory leak when token parsing finished

[Kundapura, Ganapati]

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. 

[PATCH v4] eventdev/eth_rx: fix memory leak when token parsing finished

[Weiguo Li]

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

RE: [PATCH v4] eventdev/eth_rx: fix memory leak when token parsing finished

[Kundapura, Ganapati]

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

RE: [PATCH v4] eventdev/eth_rx: fix memory leak when token parsing finished

[Jayatheerthan, Jay]

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

[PATCH v5] eventdev/eth_rx: fix memory leak when token parsing finished

[Weiguo Li]

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

RE: [PATCH v5] eventdev/eth_rx: fix memory leak when token parsing finished

[Jayatheerthan, Jay]

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

[PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished

[Weiguo Li]

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

RE: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished

[Jayatheerthan, Jay]

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

Re: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished

[Jerin Jacob]

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
>

Re: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished

[Kevin Traynor]

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
>>
> 

RE: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished

[Naga Harish K, S V]

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
> >>
> >
> 

Re: [PATCH] eventdev/eth_rx: fix memory leak when token parsing finished

[Stephen Hemminger]

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...

[PATCH v3] eventdev/eth_rx: fix memory leak when token parsing finished

[Weiguo Li]

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