On Tue, Feb 28, 2023 at 02:57:42AM +0000, Liu, Yi L wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > Sent: Tuesday, February 28, 2023 2:45 AM
> >
> > On Mon, Feb 27, 2023 at 03:11:27AM -0800, Yi Liu wrote:
> > > diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-
> > mc/vfio_fsl_mc.c
> > > index c89a047a4cd8..d540cf683d93 100644
> > > --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> > > +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> > > @@ -594,6 +594,7 @@ static const struct vfio_device_ops
> > vfio_fsl_mc_ops = {
> > > .bind_iommufd = vfio_iommufd_physical_bind,
> > > .unbind_iommufd = vfio_iommufd_physical_unbind,
> > > .attach_ioas = vfio_iommufd_physical_attach_ioas,
> > > + .detach_ioas = vfio_iommufd_physical_detach_ioas,
> > > };
> > >
> > > static struct fsl_mc_driver vfio_fsl_mc_driver = {
> > > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
> > > index beef6ca21107..bfaa9876499b 100644
> > > --- a/drivers/vfio/iommufd.c
> > > +++ b/drivers/vfio/iommufd.c
> > > @@ -88,6 +88,14 @@ int vfio_iommufd_physical_attach_ioas(struct
> > vfio_device *vdev, u32 *pt_id)
> > > {
> > > int rc;
> > >
> > > + lockdep_assert_held(&vdev->dev_set->lock);
> > > +
> > > + if (!vdev->iommufd_device)
> > > + return -EINVAL;
> >
> > This should be a WARN_ON. The vdev->iommufd_ctx should be NULL if it
> > hasn't been bound, and it can't be bound unless the
> > iommufd_device/attach was created.
>
> sure. But it is a user-triggerable warn. If userspace triggers it on
> purpose, will it be a bad thing for kernel? Maybe use
> dev_warn_ratelimited()?
How can it be user triggerable? You shouldn't be able to reach this
function until the device is bound because the ioctl should be after
the is it bound check
Jason
