[dpdk-dev] [PATCH v21 4/7] dmadev: introduce DMA device library implementation

Kevin Laatz kevin.laatz at intel.com
Wed Sep 15 15:51:55 CEST 2021


On 07/09/2021 13:56, Chengwen Feng wrote:
> This patch introduce DMA device library implementation which includes
> configuration and I/O with the DMA devices.
>
> Signed-off-by: Chengwen Feng <fengchengwen at huawei.com>
> Acked-by: Bruce Richardson <bruce.richardson at intel.com>
> Acked-by: Morten Brørup <mb at smartsharesystems.com>
> Reviewed-by: Kevin Laatz <kevin.laatz at intel.com>
> Reviewed-by: Conor Walsh <conor.walsh at intel.com>
> ---
>   config/rte_config.h          |   3 +
>   lib/dmadev/meson.build       |   1 +
>   lib/dmadev/rte_dmadev.c      | 607 +++++++++++++++++++++++++++++++++++
>   lib/dmadev/rte_dmadev.h      | 118 ++++++-
>   lib/dmadev/rte_dmadev_core.h |   2 +
>   lib/dmadev/version.map       |   1 +
>   6 files changed, 720 insertions(+), 12 deletions(-)
>   create mode 100644 lib/dmadev/rte_dmadev.c
>
[snip]

>   /**
>    * @warning
> @@ -941,10 +1018,27 @@ rte_dmadev_completed(uint16_t dev_id, uint16_t vchan, const uint16_t nb_cpls,
>    *   status array are also set.
>    */
>   __rte_experimental
> -uint16_t
> +static inline uint16_t
>   rte_dmadev_completed_status(uint16_t dev_id, uint16_t vchan,
>   			    const uint16_t nb_cpls, uint16_t *last_idx,
> -			    enum rte_dma_status_code *status);
> +			    enum rte_dma_status_code *status)
> +{
> +	struct rte_dmadev *dev = &rte_dmadevices[dev_id];
> +	uint16_t idx;
> +
> +#ifdef RTE_DMADEV_DEBUG
> +	if (!rte_dmadev_is_valid_dev(dev_id) || !dev->data->dev_started ||
> +	    vchan >= dev->data->dev_conf.nb_vchans ||
> +	    nb_cpls == 0 || status == NULL)
> +		return 0;
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->completed_status, 0);
> +#endif
> +
> +	if (last_idx == NULL)
> +		last_idx = &idx;

Hi Chengwen,

An internal coverity scan on the IDXD dmadev driver patches flagged a 
potential null pointer dereference when using completed_status().

IMO it is a false positive for the driver code since it should be 
checked at the library API level, however the check is also not present 
in the library.

For the v22, can you add the NULL pointer check for status here, like 
you have for last_idx, please?

/Kevin

> +
> +	return (*dev->completed_status)(dev, vchan, nb_cpls, last_idx, status);
> +}
>   
>   #ifdef __cplusplus
>   }
>


More information about the dev mailing list