[RFC] kvargs: don't pass parse handler a NULL pointer
Checks
Commit Message
There is class of problems in current DPDK where rte_kvargs
is used a key/value pair is passed without an associated value.
Currently, this can cause a NULL dereference in the rte_kvargs_process
handler.
Reported-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/kvargs/rte_kvargs.c | 3 +++
lib/kvargs/rte_kvargs.h | 3 +++
2 files changed, 6 insertions(+)
Comments
There are a few only-key situation which use rte_kvargs_process to process, If we make
this change, the above case will fail.
Actually I locally make similar change plus a new API (rte_kvargs_process_opt), this
new API will cover key/value and only-key match, while rte_kvargs_process make restrict
only work for key/value match.
I didn't send the patches because it too late for adding this new API due to already RC1.
/**
* Call a handler function for each key/value or only-key matching the key
*
* For each key/value or only-key association that matches the given key, calls
* the handler function with the for a given arg_name passing the value on the
* dictionary for that key and a given extra argument.
*
* @param kvlist
* The rte_kvargs structure. No error if NULL.
* @param key_match
* The key on which the handler should be called, or NULL to process handler
* on all associations
* @param handler
* The function to call for each matching key
* @param opaque_arg
* A pointer passed unchanged to the handler
*
* @return
* - 0 on success
* - Negative on error
*/
int rte_kvargs_process_opt(const struct rte_kvargs *kvlist,
const char *key_match, arg_handler_t handler, void *opaque_arg);
On 2023/11/1 4:58, Stephen Hemminger wrote:
> There is class of problems in current DPDK where rte_kvargs
> is used a key/value pair is passed without an associated value.
> Currently, this can cause a NULL dereference in the rte_kvargs_process
> handler.
>
> Reported-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/kvargs/rte_kvargs.c | 3 +++
> lib/kvargs/rte_kvargs.h | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/lib/kvargs/rte_kvargs.c b/lib/kvargs/rte_kvargs.c
> index c77bb82feb..5481f58584 100644
> --- a/lib/kvargs/rte_kvargs.c
> +++ b/lib/kvargs/rte_kvargs.c
> @@ -185,6 +185,9 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
> for (i = 0; i < kvlist->count; i++) {
> pair = &kvlist->pairs[i];
> if (key_match == NULL || strcmp(pair->key, key_match) == 0) {
> + if (pair->value == NULL)
> + return -1;
> +
> if ((*handler)(pair->key, pair->value, opaque_arg) < 0)
> return -1;
> }
> diff --git a/lib/kvargs/rte_kvargs.h b/lib/kvargs/rte_kvargs.h
> index 4900b750bc..fd9a3238f0 100644
> --- a/lib/kvargs/rte_kvargs.h
> +++ b/lib/kvargs/rte_kvargs.h
> @@ -178,6 +178,9 @@ const char *rte_kvargs_get_with_value(const struct rte_kvargs *kvlist,
> * handler function with the for a given arg_name passing the value on the
> * dictionary for that key and a given extra argument.
> *
> + * If no value was specified with the given key, then this
> + * function will return -1 and the handlere will not be called.
> + *
> * @param kvlist
> * The rte_kvargs structure. No error if NULL.
> * @param key_match
>
@@ -185,6 +185,9 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
for (i = 0; i < kvlist->count; i++) {
pair = &kvlist->pairs[i];
if (key_match == NULL || strcmp(pair->key, key_match) == 0) {
+ if (pair->value == NULL)
+ return -1;
+
if ((*handler)(pair->key, pair->value, opaque_arg) < 0)
return -1;
}
@@ -178,6 +178,9 @@ const char *rte_kvargs_get_with_value(const struct rte_kvargs *kvlist,
* handler function with the for a given arg_name passing the value on the
* dictionary for that key and a given extra argument.
*
+ * If no value was specified with the given key, then this
+ * function will return -1 and the handlere will not be called.
+ *
* @param kvlist
* The rte_kvargs structure. No error if NULL.
* @param key_match