Re: [dpdk-dev] [PATCH] net/vdev_netvsc: fix erronous cast

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 3/17/2019 6:16 AM, Matan Azrad wrote:
> Hi
> 
> From: Stephen Hemminger
>>> Hi
>>>
>>> From: Stephen Hemminger
>>>> The return value from bus->find_device is a rte_device which is not
>>>> safe to cast to a rte_vdev_device structure.
>>>> It doesn't really matter since only being checked for NULL but
>>>> static checkers might find a bug here.
>>>>
>>>
>>> Fix line is missing.
>>>
>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>>> ---
>>>>  drivers/net/vdev_netvsc/vdev_netvsc.c | 7 ++++---
>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c
>>>> b/drivers/net/vdev_netvsc/vdev_netvsc.c
>>>> index ba63fac2a598..801f54c96e01 100644
>>>> --- a/drivers/net/vdev_netvsc/vdev_netvsc.c
>>>> +++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
>>>> @@ -808,7 +808,7 @@ vdev_netvsc_cmp_rte_device(const struct
>>>> rte_device *dev1,  static void
>>>> vdev_netvsc_scan_callback(__rte_unused
>>>> void *arg)  {
>>>> -	struct rte_vdev_device *dev;
>>>> +	struct rte_device *dev;
>>>>  	struct rte_devargs *devargs;
>>>>  	struct rte_bus *vbus = rte_bus_find_by_name("vdev");
>>>>
>>>> @@ -816,8 +816,9 @@ vdev_netvsc_scan_callback(__rte_unused void
>> *arg)
>>>>  		if (!strncmp(devargs->name, VDEV_NETVSC_DRIVER_NAME,
>>>>  			     VDEV_NETVSC_DRIVER_NAME_LEN))
>>>>  			return;
>>>> -	dev = (struct rte_vdev_device *)vbus->find_device(NULL,
>>>> -		vdev_netvsc_cmp_rte_device,
>>>> VDEV_NETVSC_DRIVER_NAME);
>>>> +
>>>> +	dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
>>>> +				VDEV_NETVSC_DRIVER_NAME);
>>>
>>> Since the device must be vdev here,
>>> It is better to use explicit cast to make the checker happy.
>>
>> The cast is converting from rte_device to rte_vdev_device which incorrect.
>>
>> The device returned by vbus->find_device is a rte_device not a
>> rte_vdev_device:
>>
>> vbus->find_device points to rte_vdev_find_device
>>
>> struct rte_device *
>> rte_vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
>> 		     const void *data)
>>
>> but definition of rte_vdev_device is:
>>
>> struct rte_vdev_device {
>> 	TAILQ_ENTRY(rte_vdev_device) next;      /**< Next attached vdev */
>> 	struct rte_device device;               /**< Inherit core device */
>> };
>>
>> The only safe way to get rte_vdev_device would be to use container_of().
>>
>> 	dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
>> 				VDEV_NETVSC_DRIVER_NAME);
>> 	vdev = dev ? container_of(dev, struct rte_vdev_device, device) :
>> NULL;
>>
>> But as the PATCH demonstrated, this is not necessary.
> 
> Yes, you right.
> Please add fix line to the patch + stable line.

Adding explicit cast from Matan:
Acked-by: Matan Azrad <matan@mellanox.com>

    Fixes: 56252de779a6 ("net/vdev_netvsc: add automatic probing")
    Cc: stable@dpdk.org

Applied to dpdk-next-net/master, thanks.