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

Jason Gunthorpe <[email protected]>
Newsgroups dev.linux.lists.iommu,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <178639198310.282103.12552740515074532170.b4-review@b4>
> @@ -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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.