diff mbox series

[1/5] app/pdump: fix segment fault when parse args

Message ID 20230314124813.39521-2-fengchengwen@huawei.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series fix segment fault when parse args |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

fengchengwen March 14, 2023, 12:48 p.m. UTC 
  The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

This patch fixes segment fault when parse --pdump args with 'only keys'
(e.g. 'port,queue=*').

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/pdump/main.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Ferruh Yigit March 16, 2023, 5:34 p.m. UTC | #1 
On 3/14/2023 12:48 PM, Chengwen Feng wrote:
> The rte_kvargs_process() was used to parse KV pairs, it also supports
> to parse 'only keys' (e.g. socket_id) type. And the callback function
> parameter 'value' is NULL when parsed 'only keys'.
> 
> This patch fixes segment fault when parse --pdump args with 'only keys'
> (e.g. 'port,queue=*').
> 
> Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

Hi Chengwen,

Thanks for the fix, +1 to patch.

> ---
>  app/pdump/main.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index c6cf9d9c87..d286952483 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -170,6 +170,9 @@ parse_device_id(const char *key __rte_unused, const char *value,
>  {
>  	struct pdump_tuples *pt = extra_args;
>  
> +	if (value == NULL || extra_args == NULL)
> +		return -EINVAL;
> +

Do we need to check 'extra_args'?
It is not something changes in the runtime, functions provides this
callback is local to this file and in control, also if 'extra_args' is
not provided correctly it will crash immediately so easy to detect it.

But +1 to check 'value', since that depends what user provided and need
to verify user input.

Same comment for all set.
diff mbox series

Patch

diff --git a/app/pdump/main.c b/app/pdump/main.c
index c6cf9d9c87..d286952483 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -170,6 +170,9 @@  parse_device_id(const char *key __rte_unused, const char *value,
 {
 	struct pdump_tuples *pt = extra_args;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	pt->device_id = strdup(value);
 	pt->dump_by_type = DEVICE_ID;
 
@@ -182,6 +185,9 @@  parse_queue(const char *key __rte_unused, const char *value, void *extra_args)
 	unsigned long n;
 	struct pdump_tuples *pt = extra_args;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	if (!strcmp(value, "*"))
 		pt->queue = RTE_PDUMP_ALL_QUEUES;
 	else {
@@ -197,6 +203,9 @@  parse_rxtxdev(const char *key, const char *value, void *extra_args)
 
 	struct pdump_tuples *pt = extra_args;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
 		strlcpy(pt->rx_dev, value, sizeof(pt->rx_dev));
 		/* identify the tx stream type for pcap vdev */
@@ -220,6 +229,9 @@  parse_uint_value(const char *key, const char *value, void *extra_args)
 	char *end;
 	int ret = 0;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	errno = 0;
 	v = extra_args;
 	t = strtoul(value, &end, 10);