[PATCH 0/9] fix kvargs callback prototype not clearly defined

[PATCH 0/9] fix kvargs callback prototype not clearly defined

[Chengwen Feng]

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

But where there is no detailed definition of 'value' maybe NULL. this 
leads to a lot of processing errors (some may cause segment errors). 
This patchset fixes some of them.

Chengwen Feng (9):
  kvargs: detailed definition of callback prototype
  compressdev: fix segment fault when parse input args
  compressdev: fix null name when parse input args
  cryptodev: fix segment fault when parse input args
  cryptodev: fix null name when parse input args
  net/hns3: fix segment fault when parse runtime config
  net/virtio: fix segment fault when parse devargs
  dma/skeleton: fix segment fault when parse devargs
  raw/skeleton: fix segment fault when parse devargs

 drivers/dma/skeleton/skeleton_dmadev.c |  8 +++++++-
 drivers/net/hns3/hns3_common.c         |  9 +++++++++
 drivers/net/virtio/virtio_ethdev.c     |  3 +++
 drivers/net/virtio/virtio_pci_ethdev.c |  3 +++
 drivers/raw/skeleton/skeleton_rawdev.c |  2 ++
 lib/compressdev/rte_compressdev_pmd.c  |  6 ++++++
 lib/cryptodev/cryptodev_pmd.c          |  7 +++++++
 lib/kvargs/rte_kvargs.h                | 14 +++++++++++++-
 8 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH 1/9] kvargs: detailed definition of callback prototype

[Chengwen Feng]

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
(which prototype is arg_handler_t) parameter 'value' is NULL when
parsed 'only keys'.

But where there is no detailed definition of 'value' maybe NULL, so
this patch adds it.

Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/kvargs/rte_kvargs.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/kvargs/rte_kvargs.h b/lib/kvargs/rte_kvargs.h
index 359a9f5b09..4900b750bc 100644
--- a/lib/kvargs/rte_kvargs.h
+++ b/lib/kvargs/rte_kvargs.h
@@ -36,7 +36,19 @@ extern "C" {
 /** separator character used between key and value */
 #define RTE_KVARGS_KV_DELIM	"="
 
-/** Type of callback function used by rte_kvargs_process() */
+/**
+ * Callback prototype used by rte_kvargs_process().
+ *
+ * @param key
+ *   The key to consider, it will not be NULL.
+ * @param value
+ *   The value corresponding to the key, it may be NULL (e.g. only with key)
+ * @param opaque
+ *   An opaque pointer coming from the caller.
+ * @return
+ *   - >=0 handle key success.
+ *   - <0 on error.
+ */
 typedef int (*arg_handler_t)(const char *key, const char *value, void *opaque);
 
 /** A key/value association */
-- 
2.17.1

[PATCH 2/9] compressdev: fix segment fault when parse input args

[Chengwen Feng]

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 input args with 'only keys'
(e.g. socket_id).

Fixes: ed7dd94f7f66 ("compressdev: add basic device management")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/compressdev/rte_compressdev_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/compressdev/rte_compressdev_pmd.c b/lib/compressdev/rte_compressdev_pmd.c
index e139bc86e7..93c31fbac8 100644
--- a/lib/compressdev/rte_compressdev_pmd.c
+++ b/lib/compressdev/rte_compressdev_pmd.c
@@ -40,6 +40,9 @@ rte_compressdev_pmd_parse_uint_arg(const char *key __rte_unused,
 	int i;
 	char *end;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	errno = 0;
 	i = strtol(value, &end, 10);
 	if (*end != 0 || errno != 0 || i < 0)
-- 
2.17.1

[PATCH 3/9] compressdev: fix null name when parse input args

[Chengwen Feng]

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

If the input args is just "name" (which mean only has keys), then it
will get (null) name in rte_compressdev_pmd_parse_name_arg(). This
patch fixes it.

Fixes: ed7dd94f7f66 ("compressdev: add basic device management")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/compressdev/rte_compressdev_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/compressdev/rte_compressdev_pmd.c b/lib/compressdev/rte_compressdev_pmd.c
index 93c31fbac8..156bccd972 100644
--- a/lib/compressdev/rte_compressdev_pmd.c
+++ b/lib/compressdev/rte_compressdev_pmd.c
@@ -23,6 +23,9 @@ rte_compressdev_pmd_parse_name_arg(const char *key __rte_unused,
 	struct rte_compressdev_pmd_init_params *params = extra_args;
 	int n;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	n = strlcpy(params->name, value, RTE_COMPRESSDEV_NAME_MAX_LEN);
 	if (n >= RTE_COMPRESSDEV_NAME_MAX_LEN)
 		return -EINVAL;
-- 
2.17.1

[PATCH 4/9] cryptodev: fix segment fault when parse input args

[Chengwen Feng]

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 input args with 'only keys'
(e.g. socket_id,max_nb_queue_pairs).

Fixes: 9e6edea41805 ("cryptodev: add APIs to assist PMD initialisation")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/cryptodev/cryptodev_pmd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/cryptodev/cryptodev_pmd.c b/lib/cryptodev/cryptodev_pmd.c
index 77b269f312..4122000839 100644
--- a/lib/cryptodev/cryptodev_pmd.c
+++ b/lib/cryptodev/cryptodev_pmd.c
@@ -38,6 +38,10 @@ rte_cryptodev_pmd_parse_uint_arg(const char *key __rte_unused,
 {
 	int i;
 	char *end;
+
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	errno = 0;
 
 	i = strtol(value, &end, 10);
-- 
2.17.1

[PATCH 5/9] cryptodev: fix null name when parse input args

[Chengwen Feng]

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

If the input args is just "name" (which mean only has keys), then it
will get (null) name in rte_cryptodev_pmd_parse_name_arg(). This patch
fixes it.

Fixes: 9e6edea41805 ("cryptodev: add APIs to assist PMD initialisation")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/cryptodev/cryptodev_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/cryptodev/cryptodev_pmd.c b/lib/cryptodev/cryptodev_pmd.c
index 4122000839..d8073a601d 100644
--- a/lib/cryptodev/cryptodev_pmd.c
+++ b/lib/cryptodev/cryptodev_pmd.c
@@ -22,6 +22,9 @@ rte_cryptodev_pmd_parse_name_arg(const char *key __rte_unused,
 	struct rte_cryptodev_pmd_init_params *params = extra_args;
 	int n;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	n = strlcpy(params->name, value, RTE_CRYPTODEV_NAME_MAX_LEN);
 	if (n >= RTE_CRYPTODEV_NAME_MAX_LEN)
 		return -EINVAL;
-- 
2.17.1

[PATCH 6/9] net/hns3: fix segment fault when parse runtime config

[Chengwen Feng]

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 input args with 'only keys'
(e.g. rx_func_hint).

Fixes: a124f9e9591b ("net/hns3: add runtime config to select IO burst function")
Fixes: 70791213242e ("net/hns3: support masking device capability")
Fixes: 2fc3e696a7f1 ("net/hns3: add runtime config for mailbox limit time")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/net/hns3/hns3_common.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/hns3/hns3_common.c b/drivers/net/hns3/hns3_common.c
index a0c9e66c2c..f077ef5057 100644
--- a/drivers/net/hns3/hns3_common.c
+++ b/drivers/net/hns3/hns3_common.c
@@ -163,6 +163,9 @@ hns3_parse_io_hint_func(const char *key, const char *value, void *extra_args)
 
 	RTE_SET_USED(key);
 
+	if (value == NULL || extra_args == NULL)
+		return 0;
+
 	if (strcmp(value, "vec") == 0)
 		hint = HNS3_IO_FUNC_HINT_VEC;
 	else if (strcmp(value, "sve") == 0)
@@ -203,6 +206,9 @@ hns3_parse_dev_caps_mask(const char *key, const char *value, void *extra_args)
 
 	RTE_SET_USED(key);
 
+	if (value == NULL || extra_args == NULL)
+		return 0;
+
 	val = strtoull(value, NULL, HNS3_CONVERT_TO_HEXADECIMAL);
 	*(uint64_t *)extra_args = val;
 
@@ -216,6 +222,9 @@ hns3_parse_mbx_time_limit(const char *key, const char *value, void *extra_args)
 
 	RTE_SET_USED(key);
 
+	if (value == NULL || extra_args == NULL)
+		return 0;
+
 	val = strtoul(value, NULL, HNS3_CONVERT_TO_DECIMAL);
 
 	/*
-- 
2.17.1

[PATCH 7/9] net/virtio: fix segment fault when parse devargs

[Chengwen Feng]

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 input args with 'only keys'
(e.g. vectorized,vdpa).

Fixes: 4710e16a4a7b ("net/virtio: add parameter to enable vectorized path")
Fixes: 44d7b2e87b69 ("net/virtio: refactor devargs parsing")
Fixes: 440f03c25378 ("net/virtio: skip device probe in vDPA mode")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/net/virtio/virtio_ethdev.c     | 3 +++
 drivers/net/virtio/virtio_pci_ethdev.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 0103d95920..d10f42bba2 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2054,6 +2054,9 @@ virtio_dev_speed_capa_get(uint32_t speed)
 static int vectorized_check_handler(__rte_unused const char *key,
 		const char *value, void *ret_val)
 {
+	if (value == NULL || ret_val == NULL)
+		return -EINVAL;
+
 	if (strcmp(value, "1") == 0)
 		*(int *)ret_val = 1;
 	else
diff --git a/drivers/net/virtio/virtio_pci_ethdev.c b/drivers/net/virtio/virtio_pci_ethdev.c
index abc63b0935..9b4b846f8a 100644
--- a/drivers/net/virtio/virtio_pci_ethdev.c
+++ b/drivers/net/virtio/virtio_pci_ethdev.c
@@ -148,6 +148,9 @@ eth_virtio_pci_uninit(struct rte_eth_dev *eth_dev)
 static int vdpa_check_handler(__rte_unused const char *key,
 		const char *value, void *ret_val)
 {
+	if (value == NULL || ret_val == NULL)
+		return -EINVAL;
+
 	if (strcmp(value, "1") == 0)
 		*(int *)ret_val = 1;
 	else
-- 
2.17.1

[PATCH 8/9] dma/skeleton: fix segment fault when parse devargs

[Chengwen Feng]

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 input args with 'only keys'
(e.g. lcore).

Fixes: 05d5fc66a269 ("dma/skeleton: introduce skeleton driver")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/dma/skeleton/skeleton_dmadev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/skeleton/skeleton_dmadev.c b/drivers/dma/skeleton/skeleton_dmadev.c
index 9b6da655fd..daf35eccce 100644
--- a/drivers/dma/skeleton/skeleton_dmadev.c
+++ b/drivers/dma/skeleton/skeleton_dmadev.c
@@ -515,9 +515,15 @@ skeldma_parse_lcore(const char *key __rte_unused,
 		    const char *value,
 		    void *opaque)
 {
-	int lcore_id = atoi(value);
+	int lcore_id;
+
+	if (value == NULL || opaque == NULL)
+		return -EINVAL;
+
+	lcore_id = atoi(value);
 	if (lcore_id >= 0 && lcore_id < RTE_MAX_LCORE)
 		*(int *)opaque = lcore_id;
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH 9/9] raw/skeleton: fix segment fault when parse devargs

[Chengwen Feng]

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 input args with 'only keys'
(e.g. self_test).

Fixes: 55ca1b0f2151 ("raw/skeleton: add test cases")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/raw/skeleton/skeleton_rawdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c
index b2ca1cc5cd..53fe49f936 100644
--- a/drivers/raw/skeleton/skeleton_rawdev.c
+++ b/drivers/raw/skeleton/skeleton_rawdev.c
@@ -664,6 +664,8 @@ skeldev_get_selftest(const char *key __rte_unused,
 		     void *opaque)
 {
 	int *flag = opaque;
+	if (value == NULL || opaque == NULL)
+		return -EINVAL;
 	*flag = atoi(value);
 	return 0;
 }
-- 
2.17.1

Re: [PATCH 0/9] fix kvargs callback prototype not clearly defined

[David Marchand]

On Thu, Mar 2, 2023 at 8:56 AM Chengwen Feng <fengchengwen@huawei.com> 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'.
>
> But where there is no detailed definition of 'value' maybe NULL. this
> leads to a lot of processing errors (some may cause segment errors).
> This patchset fixes some of them.
>
> Chengwen Feng (9):
>   kvargs: detailed definition of callback prototype
>   compressdev: fix segment fault when parse input args
>   compressdev: fix null name when parse input args
>   cryptodev: fix segment fault when parse input args
>   cryptodev: fix null name when parse input args
>   net/hns3: fix segment fault when parse runtime config
>   net/virtio: fix segment fault when parse devargs
>   dma/skeleton: fix segment fault when parse devargs
>   raw/skeleton: fix segment fault when parse devargs
>
>  drivers/dma/skeleton/skeleton_dmadev.c |  8 +++++++-
>  drivers/net/hns3/hns3_common.c         |  9 +++++++++
>  drivers/net/virtio/virtio_ethdev.c     |  3 +++
>  drivers/net/virtio/virtio_pci_ethdev.c |  3 +++
>  drivers/raw/skeleton/skeleton_rawdev.c |  2 ++
>  lib/compressdev/rte_compressdev_pmd.c  |  6 ++++++
>  lib/cryptodev/cryptodev_pmd.c          |  7 +++++++
>  lib/kvargs/rte_kvargs.h                | 14 +++++++++++++-
>  8 files changed, 50 insertions(+), 2 deletions(-)

Series applied, thanks Chengwen.

-- 
David Marchand

Re: [PATCH 0/9] fix kvargs callback prototype not clearly defined

[fengchengwen]

Sorry to resend, because the original v1 --cc-cmd failed.

On 2023/3/2 15:50, 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'.
> 
> But where there is no detailed definition of 'value' maybe NULL. this 
> leads to a lot of processing errors (some may cause segment errors). 
> This patchset fixes some of them.
> 
> Chengwen Feng (9):
>   kvargs: detailed definition of callback prototype
>   compressdev: fix segment fault when parse input args
>   compressdev: fix null name when parse input args
>   cryptodev: fix segment fault when parse input args
>   cryptodev: fix null name when parse input args
>   net/hns3: fix segment fault when parse runtime config
>   net/virtio: fix segment fault when parse devargs
>   dma/skeleton: fix segment fault when parse devargs
>   raw/skeleton: fix segment fault when parse devargs
> 
>  drivers/dma/skeleton/skeleton_dmadev.c |  8 +++++++-
>  drivers/net/hns3/hns3_common.c         |  9 +++++++++
>  drivers/net/virtio/virtio_ethdev.c     |  3 +++
>  drivers/net/virtio/virtio_pci_ethdev.c |  3 +++
>  drivers/raw/skeleton/skeleton_rawdev.c |  2 ++
>  lib/compressdev/rte_compressdev_pmd.c  |  6 ++++++
>  lib/cryptodev/cryptodev_pmd.c          |  7 +++++++
>  lib/kvargs/rte_kvargs.h                | 14 +++++++++++++-
>  8 files changed, 50 insertions(+), 2 deletions(-)
> 

[PATCH 0/9] fix kvargs callback prototype not clearly defined

[Chengwen Feng]

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

But where there is no detailed definition of 'value' maybe NULL. this 
leads to a lot of processing errors (some may cause segment errors). 
This patchset fixes some of them.

Chengwen Feng (9):
  kvargs: detailed definition of callback prototype
  compressdev: fix segment fault when parse input args
  compressdev: fix null name when parse input args
  cryptodev: fix segment fault when parse input args
  cryptodev: fix null name when parse input args
  net/hns3: fix segment fault when parse runtime config
  net/virtio: fix segment fault when parse devargs
  dma/skeleton: fix segment fault when parse devargs
  raw/skeleton: fix segment fault when parse devargs

 drivers/dma/skeleton/skeleton_dmadev.c |  8 +++++++-
 drivers/net/hns3/hns3_common.c         |  9 +++++++++
 drivers/net/virtio/virtio_ethdev.c     |  3 +++
 drivers/net/virtio/virtio_pci_ethdev.c |  3 +++
 drivers/raw/skeleton/skeleton_rawdev.c |  2 ++
 lib/compressdev/rte_compressdev_pmd.c  |  6 ++++++
 lib/cryptodev/cryptodev_pmd.c          |  7 +++++++
 lib/kvargs/rte_kvargs.h                | 14 +++++++++++++-
 8 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.17.1