[dpdk-dev] [PATCH v21 4/7] dmadev: introduce DMA device library implementation
Kevin Laatz
kevin.laatz at intel.com
Wed Sep 15 16:47:48 CEST 2021
On 15/09/2021 15:34, Bruce Richardson wrote:
> On Wed, Sep 15, 2021 at 02:51:55PM +0100, Kevin Laatz wrote:
>> 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?
>>
> I think the check would have to be different than that for last_idx, since
> the status pointer is a pointer to an array, rather than a single value -
> which procludes a simple replacement in the wrapper function that the
> compiler can inline away if unnecessary.
> It's probably best to add it as a check in the debug block, with an
> error-return if status is NULL.
+1
/Kevin
More information about the dev
mailing list