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