[0/5] fix segment fault when parse args
Message ID | 20230314124813.39521-1-fengchengwen@huawei.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4EB9A41E28; Tue, 14 Mar 2023 13:54:54 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 287E740F18; Tue, 14 Mar 2023 13:54:54 +0100 (CET) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id F2AFA40A7E for <dev@dpdk.org>; Tue, 14 Mar 2023 13:54:51 +0100 (CET) Received: from dggpeml500024.china.huawei.com (unknown [172.30.72.56]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4PbYLb6KVRzSl0C; Tue, 14 Mar 2023 20:51:35 +0800 (CST) Received: from localhost.localdomain (10.50.163.32) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Tue, 14 Mar 2023 20:54:49 +0800 From: Chengwen Feng <fengchengwen@huawei.com> To: <thomas@monjalon.net>, <ferruh.yigit@amd.com> CC: <dev@dpdk.org> Subject: [PATCH 0/5] fix segment fault when parse args Date: Tue, 14 Mar 2023 12:48:08 +0000 Message-ID: <20230314124813.39521-1-fengchengwen@huawei.com> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.50.163.32] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org |
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'. It may leads to segment fault when parse args with 'only key', this patchset fixes rest of them. Chengwen Feng (5): app/pdump: fix segment fault when parse args net/memif: fix segment fault when parse devargs net/pcap: fix segment fault when parse devargs net/ring: fix segment fault when parse devargs net/sfc: fix segment fault when parse devargs app/pdump/main.c | 12 ++++++++++++ drivers/net/memif/rte_eth_memif.c | 30 ++++++++++++++++++++++++++++++ drivers/net/pcap/pcap_ethdev.c | 13 +++++++++++-- drivers/net/ring/rte_eth_ring.c | 6 ++++++ drivers/net/sfc/sfc.c | 3 +++ drivers/net/sfc/sfc_ev.c | 3 +++ drivers/net/sfc/sfc_kvargs.c | 6 ++++++ 7 files changed, 71 insertions(+), 2 deletions(-)
Comments
On 11/3/2023 7:38 AM, Chengwen Feng wrote: > The rte_kvargs_process() was used to parse key-value (e.g. socket_id=0), > it also supports to parse only-key (e.g. socket_id). But many drivers's > callback can only handle key-value, it will segment fault if handles > only-key. so the patchset [1] was introduced. > > Because the patchset [1] modified too much drivers, therefore: > 1) A new API rte_kvargs_process_opt() was introduced, it inherits the > function of rte_kvargs_process() which could parse both key-value and > only-key. > 2) Constraint the rte_kvargs_process() can only parse key-value. > > This patchset also include one bugfix for kvargs of mvneta driver. > > [1] https://patches.dpdk.org/project/dpdk/patch/20230320092110.37295-1-fengchengwen@huawei.com/ > > Chengwen Feng (5): > kvargs: add one new process API > net/af_packet: use new API to parse kvargs > net/sfc: use new API to parse kvargs > net/tap: use new API to parse kvargs > net/mvneta: fix possible out-of-bounds write > Hi Chengwen, I checked the driver code updates above, but it is hard to know if there are more missing, each requires investigating one by one. Perhaps it can be easier to trace back from your original patch [1] and update the ones that doesn't need "value == NULL" check, I assume this is what you did.
Hi Ferruh, On 2023/11/3 21:41, Ferruh Yigit wrote: > On 11/3/2023 7:38 AM, Chengwen Feng wrote: >> The rte_kvargs_process() was used to parse key-value (e.g. socket_id=0), >> it also supports to parse only-key (e.g. socket_id). But many drivers's >> callback can only handle key-value, it will segment fault if handles >> only-key. so the patchset [1] was introduced. >> >> Because the patchset [1] modified too much drivers, therefore: >> 1) A new API rte_kvargs_process_opt() was introduced, it inherits the >> function of rte_kvargs_process() which could parse both key-value and >> only-key. >> 2) Constraint the rte_kvargs_process() can only parse key-value. >> >> This patchset also include one bugfix for kvargs of mvneta driver. >> >> [1] https://patches.dpdk.org/project/dpdk/patch/20230320092110.37295-1-fengchengwen@huawei.com/ >> >> Chengwen Feng (5): >> kvargs: add one new process API >> net/af_packet: use new API to parse kvargs >> net/sfc: use new API to parse kvargs >> net/tap: use new API to parse kvargs >> net/mvneta: fix possible out-of-bounds write >> > > Hi Chengwen, > > I checked the driver code updates above, but it is hard to know if there > are more missing, each requires investigating one by one. > Perhaps it can be easier to trace back from your original patch [1] and > update the ones that doesn't need "value == NULL" check, I assume this > is what you did. There are 51 files modified in v2 [1], there will about 80 rte_kvargs_process() invoke in current git (stats by command [2]). Exclude kvargs lib and it's test and part comment, there are 30 file was not in v2: drivers/baseband/null/bbdev_null.c ---already treat NULL value as a error drivers/baseband/turbo_sw/bbdev_turbo_software.c ---already treat NULL value as a error drivers/bus/ifpga/ifpga_bus.c ---already treat NULL value as a error drivers/common/nfp/nfp_common_pci.c ---could handle NULL value drivers/common/sfc_efx/sfc_efx.c ---could handle NULL value drivers/dma/skeleton/skeleton_dmadev.c ---already treat NULL value as a error drivers/ml/cnxk/cn10k_ml_dev.c ---part treat NULL value as a error, other segment fault when with NULL value drivers/ml/cnxk/mvtvm_ml_dev.c ---segment fault when with NULL value drivers/net/af_packet/rte_eth_af_packet.c ---don't care about value, suggested don't change in v3's comment drivers/net/bnxt/bnxt_ethdev.c ---already treat NULL value as a error drivers/net/bonding/rte_eth_bond_pmd.c ---already treat NULL value as a error drivers/net/cpfl/cpfl_ethdev.c ---segment fault when with NULL value drivers/net/failsafe/failsafe_args.c ---already treat NULL value as a error drivers/net/hns3/hns3_common.c ---already treat NULL value as a error drivers/net/ixgbe/ixgbe_ethdev.c ---already treat NULL value as a error drivers/net/null/rte_eth_null.c ---already treat NULL value as a error drivers/net/softnic/rte_eth_softnic.c ---already treat NULL value as a error drivers/net/tap/rte_eth_tap.c ---could handle NULL value drivers/net/txgbe/txgbe_ethdev.c ---already treat NULL value as a error drivers/net/vhost/rte_eth_vhost.c ---already treat NULL value as a error drivers/net/virtio/virtio_ethdev.c ---already treat NULL value as a error drivers/net/virtio/virtio_pci_ethdev.c ---already treat NULL value as a error drivers/net/virtio/virtio_user_ethdev.c ---already treat NULL value as a error drivers/raw/ifpga/ifpga_rawdev.c ---already treat NULL value as a error drivers/raw/skeleton/skeleton_rawdev.c ---already treat NULL value as a error drivers/vdpa/ifc/ifcvf_vdpa.c ---already treat NULL value as a error drivers/vdpa/sfc/sfc_vdpa_filter.c ---already treat NULL value as a error lib/compressdev/rte_compressdev_pmd.c ---already treat NULL value as a error lib/cryptodev/cryptodev_pmd.c ---already treat NULL value as a error lib/ethdev/rte_ethdev_telemetry.c ---already treat NULL value as a error so we should only process three drivers: common/nfp, sfc, tap, and these are what v4 doing. [1] https://patches.dpdk.org/project/dpdk/patch/20230320092110.37295-1-fengchengwen@huawei.com/ [2] grep -rn "rte_kvargs_process(" | cut -d ":" -f 1 | sort | uniq -c | wc -l Thanks Chengwen > > . >