Re: [Intel-gfx] [PATCH v2 1/5] iommufd: Create access in vfio_iommufd_emulated_bind()

[Jason Gunthorpe <jgg@nvidia.com>]

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 <jgg@nvidia.com>

Jason