From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6704D43258; Wed, 1 Nov 2023 02:16:09 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5118440ED6; Wed, 1 Nov 2023 02:16:09 +0100 (CET) Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by mails.dpdk.org (Postfix) with ESMTP id 7D78F402ED for ; Wed, 1 Nov 2023 02:16:07 +0100 (CET) Received: from dggpeml100024.china.huawei.com (unknown [172.30.72.57]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4SKpsX3K9nz1P7Fp; Wed, 1 Nov 2023 09:13:04 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml100024.china.huawei.com (7.185.36.115) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Wed, 1 Nov 2023 09:16:05 +0800 Subject: Re: [RFC] kvargs: don't pass parse handler a NULL pointer To: Stephen Hemminger , References: <20230314124813.39521-1-fengchengwen@huawei.com> <20231031205844.91232-1-stephen@networkplumber.org> From: fengchengwen CC: Thomas Monjalon , Ferruh Yigit Message-ID: Date: Wed, 1 Nov 2023 09:16:04 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <20231031205844.91232-1-stephen@networkplumber.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggpeml100024.china.huawei.com (7.185.36.115) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 > Signed-off-by: Stephen Hemminger > --- > 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 >