RE: [PATCH v6 13/24] vfio/iommufd: Split the compat_ioas attach out from vfio_iommufd_bind()

Tian, Kevin
2023-03-13T02:06:54Z

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Saturday, March 11, 2023 6:24 PM
> > > >
> > > > - ret = vdev->ops->bind_iommufd(vdev, ictx, &device_id);
> > > > - if (ret)
> > > > - return ret;
> > > > + /* The legacy path has no way to return the device id */
> > > > + return vdev->ops->bind_iommufd(vdev, ictx, &device_id);
> > > > +}
> > > >
> > > > - ret = iommufd_vfio_compat_ioas_get_id(ictx, &ioas_id);
> > > > - if (ret)
> > > > - goto err_unbind;
> > > > - ret = vdev->ops->attach_ioas(vdev, &ioas_id);
> > > > - if (ret)
> > > > - goto err_unbind;
> > >
> > > after noiommu check and attach_ioas are moved out then this
> > > entire function can be removed now. Just call the ops in
> > > vfio_device_first_open().
> >
> > Yes. and also no vfio_iommufd_unbind().
>
> Seems still necessary to have this wrapper. .bind_iommufd callback would
> be NULL if CONFIG_IOMMUFD==n. If we call ops->bind_iommufd directly
> in vfio_device_first_open() of vfio_main.c, it may trigger kernel panic
> for NULL pointer dereference if there is wrong code that passes valid
> iommufd pointer.. Ideally, if CONFIG_IOMMUFD==n, vfio_device_first_open
> should not receive valid iommufd pointer hence won't call ops-
> >bind_iommufd
> at all. So it deserves a panic. However, if we have a wrapper for it, such code
> may just fail with -EOPNOTSUPPT.
>

ok, let's keep this wrapper then. I didn't realize it's NULL if
CONFIG_IOMMUFD==n.