On Thu, Mar 16, 2023 at 05:15:22AM -0700, Yi Liu wrote:

> +int iommufd_access_attach(struct iommufd_access *access, u32 ioas_id)
> +{
> +     struct iommufd_ioas *new_ioas;
> +     struct iommufd_object *obj;
> +     int rc = 0;
> +
> +     if (access->ioas != NULL && access->ioas->obj.id != ioas_id)
> +             return -EINVAL;
> +
> +     obj = iommufd_get_object(access->ictx, ioas_id, IOMMUFD_OBJ_IOAS);
> +     if (IS_ERR(obj))
> +             return PTR_ERR(obj);
> +     new_ioas = container_of(obj, struct iommufd_ioas, obj);

This is

iommufd_get_ioas()

> @@ -161,15 +170,20 @@ EXPORT_SYMBOL_GPL(vfio_iommufd_emulated_unbind);
>  
>  int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id)
>  {
> -     struct iommufd_access *user;
> +     int rc;
>  
>       lockdep_assert_held(&vdev->dev_set->lock);
>  
> -     user = iommufd_access_create(vdev->iommufd_ictx, *pt_id, &vfio_user_ops,
> -                                  vdev);
> -     if (IS_ERR(user))
> -             return PTR_ERR(user);
> -     vdev->iommufd_access = user;
> +     if (WARN_ON(!vdev->iommufd_ictx))
> +             return -EINVAL;

If you are going to delete these lines in the next patch don't add
them here.

> +     if (WARN_ON(!vdev->iommufd_access))
> +             return -ENOENT;

Just let it NULL pointer deref crash on this impossible case.

Looks OK otherwise

Reviewed-by: Jason Gunthorpe <[email protected]>

Jason

Reply via email to