On Thu, Dec 21, 2023 at 06:40:01PM +0000, Sivaramakrishnan Venkat wrote:
> Incorrect ptypes list causes buffer overflow for Address Sanitizer
> run.Previously, the last element in the ptypes lists to be
> "RTE_PTYPE_UNKNOWN" for rte_eth_dev_get_supported_ptypes(), but this was
> not clearly documented and many PMDs did not follow this implementation.
> Instead, the dev_supported_ptypes_get() function pointer now returns the
> number of elements to eliminate the need for "RTE_PTYPE_UNKNOWN"
> as the last item.
>
> Fixes: 4790935 ("ethdev: make device operations struct private")
> Cc: ferruh.yi...@intel.com
> CC: sta...@dpdk.org
>
> --
> V3: Rework function pointer to return number of elements and remove
> the need for RTE_PTYPE_UNKNOWN in list
>
> Signed-off-by: Sivaramakrishnan Venkat <venkatx.sivaramakrish...@intel.com>
> ---
> drivers/net/atlantic/atl_ethdev.c | 13 ++++++++-----
> drivers/net/axgbe/axgbe_ethdev.c | 13 ++++++++-----
> drivers/net/bnxt/bnxt_ethdev.c | 7 ++++---
> drivers/net/cnxk/cnxk_ethdev.h | 3 ++-
> drivers/net/cnxk/cnxk_lookup.c | 7 ++++---
> drivers/net/cpfl/cpfl_ethdev.c | 7 ++++---
> drivers/net/cxgbe/cxgbe_ethdev.c | 10 ++++++----
> drivers/net/cxgbe/cxgbe_pfvf.h | 3 ++-
> drivers/net/dpaa/dpaa_ethdev.c | 7 +++++--
> drivers/net/dpaa2/dpaa2_ethdev.c | 10 ++++++----
> drivers/net/e1000/igb_ethdev.c | 13 ++++++++-----
> drivers/net/enetc/enetc_ethdev.c | 7 ++++---
> drivers/net/enic/enic_ethdev.c | 17 ++++++++++-------
> drivers/net/failsafe/failsafe_ops.c | 5 +++--
> drivers/net/fm10k/fm10k_ethdev.c | 15 +++++++++------
> drivers/net/hns3/hns3_rxtx.c | 16 +++++++++-------
> drivers/net/hns3/hns3_rxtx.h | 3 ++-
> drivers/net/i40e/i40e_rxtx.c | 11 +++++++----
> drivers/net/i40e/i40e_rxtx.h | 3 ++-
> drivers/net/iavf/iavf_ethdev.c | 10 ++++++----
> drivers/net/ice/ice_dcf_ethdev.c | 7 ++++---
> drivers/net/ice/ice_rxtx.c | 23 ++++++++++++++---------
> drivers/net/ice/ice_rxtx.h | 3 ++-
> drivers/net/idpf/idpf_ethdev.c | 7 ++++---
> drivers/net/igc/igc_ethdev.c | 10 ++++++----
> drivers/net/ionic/ionic_rxtx.c | 7 ++++---
> drivers/net/ionic/ionic_rxtx.h | 3 ++-
> drivers/net/ixgbe/ixgbe_ethdev.c | 18 ++++++++++++------
> drivers/net/mlx4/mlx4.h | 3 ++-
> drivers/net/mlx4/mlx4_ethdev.c | 17 ++++++++++-------
> drivers/net/mlx5/mlx5.h | 3 ++-
> drivers/net/mlx5/mlx5_ethdev.c | 11 +++++++----
> drivers/net/netvsc/hn_var.h | 3 ++-
> drivers/net/netvsc/hn_vf.c | 5 +++--
> drivers/net/nfp/nfp_net_common.c | 12 +++++++++---
> drivers/net/nfp/nfp_net_common.h | 3 ++-
> drivers/net/ngbe/ngbe_ethdev.c | 9 ++++++---
> drivers/net/ngbe/ngbe_ethdev.h | 3 ++-
> drivers/net/ngbe/ngbe_ptypes.c | 3 ++-
> drivers/net/ngbe/ngbe_ptypes.h | 2 +-
> drivers/net/octeontx/octeontx_ethdev.c | 11 +++++++----
> drivers/net/pfe/pfe_ethdev.c | 8 ++++++--
> drivers/net/qede/qede_ethdev.c | 11 +++++++----
> drivers/net/sfc/sfc_dp_rx.h | 2 +-
> drivers/net/sfc/sfc_ef10.h | 3 ++-
> drivers/net/sfc/sfc_ef100_rx.c | 7 ++++---
> drivers/net/sfc/sfc_ef10_rx.c | 11 ++++++-----
> drivers/net/sfc/sfc_ethdev.c | 5 +++--
> drivers/net/sfc/sfc_rx.c | 7 ++++---
> drivers/net/tap/rte_eth_tap.c | 4 +++-
> drivers/net/thunderx/nicvf_ethdev.c | 4 +++-
> drivers/net/txgbe/txgbe_ethdev.c | 9 ++++++---
> drivers/net/txgbe/txgbe_ethdev.h | 3 ++-
> drivers/net/txgbe/txgbe_ptypes.c | 6 +++---
> drivers/net/txgbe/txgbe_ptypes.h | 2 +-
> drivers/net/vmxnet3/vmxnet3_ethdev.c | 14 +++++++++-----
> lib/ethdev/ethdev_driver.h | 3 ++-
> lib/ethdev/rte_ethdev.c | 10 ++++++----
> 58 files changed, 280 insertions(+), 172 deletions(-)
>
> diff --git a/drivers/net/atlantic/atl_ethdev.c
> b/drivers/net/atlantic/atl_ethdev.c
> index 3a028f4290..d0d9e121db 100644
> --- a/drivers/net/atlantic/atl_ethdev.c
> +++ b/drivers/net/atlantic/atl_ethdev.c
> @@ -43,7 +43,8 @@ static int atl_dev_stats_reset(struct rte_eth_dev *dev);
> static int atl_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
> size_t fw_size);
>
> -static const uint32_t *atl_dev_supported_ptypes_get(struct rte_eth_dev *dev);
> +static const uint32_t *atl_dev_supported_ptypes_get(struct rte_eth_dev *dev,
> + uint32_t *no_of_elements);
based on the code that follows no_of_elements should probably be type
size_t * because the resulting type of the expression using RTE_DIM.
clearly it will never overflow uint32_t i suppose you could explicitly
cast to make it clear the potential truncation is on purpose.
>
> static int atl_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
>
> @@ -1132,7 +1133,8 @@ atl_dev_info_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
> }
>
> static const uint32_t *
> -atl_dev_supported_ptypes_get(struct rte_eth_dev *dev)
> +atl_dev_supported_ptypes_get(struct rte_eth_dev *dev,
> + uint32_t *no_of_elements)
> {
> static const uint32_t ptypes[] = {
> RTE_PTYPE_L2_ETHER,
> @@ -1143,12 +1145,13 @@ atl_dev_supported_ptypes_get(struct rte_eth_dev *dev)
> RTE_PTYPE_L4_TCP,
> RTE_PTYPE_L4_UDP,
> RTE_PTYPE_L4_SCTP,
> - RTE_PTYPE_L4_ICMP,
> - RTE_PTYPE_UNKNOWN
> + RTE_PTYPE_L4_ICMP
> };
>
> - if (dev->rx_pkt_burst == atl_recv_pkts)
> + if (dev->rx_pkt_burst == atl_recv_pkts) {
> + *no_of_elements = RTE_DIM(ptypes);
^^^^^^^^^^^^^^^ <-- is T = size_t
> return ptypes;
> + }
>
> return NULL;
> }
