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

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 14 Mar 2019 11:13:10 +0000
Matan Azrad <matan@mellanox.com> wrote:

> 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.