[0/5] fix segment fault when parse args

  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(-)
  

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

> 
> .
>