Re: [PATCH] iommu/iommufd: Fix NULL pointer deref in iommufd_ioas_change_process when racing with iopt_map_file_pages

[Jason Gunthorpe]

> @@ -535,6 +535,10 @@ int iommufd_ioas_change_process(struct iommufd_ucmd *ucmd)
>  		return rc;
>  
>  	for_each_ioas_area(&ioas_list, index, ioas, area)  {
> +		if (!area->pages) {
> +			rc = -EBUSY;
> +			goto out;
> +		}
>  		if (area->pages->type != IOPT_ADDRESS_FILE) {
>  			rc = -EINVAL;
>  			goto out;

If we do this then a concurrent map will get corrupted accounting. I think we
have to prevent map from progressing too. I belive the issue is we don't take
enough locks.

Does this fix it?

--- a/drivers/iommu/iommufd/ioas.c
+++ b/drivers/iommu/iommufd/ioas.c
@@ -383,6 +383,7 @@ static void iommufd_release_all_iova_rwsem(struct iommufd_ctx *ictx,
 
        xa_for_each(ioas_list, index, ioas) {
                up_write(&ioas->iopt.iova_rwsem);
+               up_write(&ioas->iopt.devices);
                refcount_dec(&ioas->obj.users);
        }
        up_write(&ictx->ioas_creation_lock);
@@ -422,6 +423,8 @@ static int iommufd_take_all_iova_rwsem(struct iommufd_ctx *ictx,
                xa_unlock(&ictx->objects);
 
                ioas = container_of(obj, struct iommufd_ioas, obj);
+               down_write_nest_lock(&ioas->iopt.devices_rwsem,
+                                    &ictx->ioas_creation_lock);
                down_write_nest_lock(&ioas->iopt.iova_rwsem,
                                     &ictx->ioas_creation_lock);

-- 
Jason