Commit 71e2409a authored by Jason Gunthorpe's avatar Jason Gunthorpe
Browse files

iommufd: Do not map/unmap revoked DMABUFs 

Once a DMABUF is revoked the domain will be unmapped under the pages
mutex. Double unmapping will trigger a WARN, and mapping while revoked
will fail.

Check for revoked DMABUFs along all the map and unmap paths to resolve
this. Ensure that map/unmap is always done under the pages mutex so it is
synchronized with the revoke notifier.

If a revoke happens between allocating the iopt_pages and the population
to a domain then the population will succeed, and leave things unmapped as
though revoke had happened immediately after.

Currently there is no way to repopulate the domains. Userspace is expected
to know if it is going to do something that would trigger revoke (eg if it
is about to do a FLR) then it should go and remove the DMABUF mappings
before and put the back after. The revoke is only to protect the kernel
from mis-behaving userspace.

Link: https://patch.msgid.link/r/3-v2-b2c110338e3f+5c2-iommufd_dmabuf_jgg@nvidia.com


Reviewed-by: default avatarNicolin Chen <nicolinc@nvidia.com>
Reviewed-by: default avatarKevin Tian <kevin.tian@intel.com>
Tested-by: default avatarNicolin Chen <nicolinc@nvidia.com>
Tested-by: default avatarShuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
parent 71db84a0

drivers/iommu/iommufd/io_pagetable.c

+10 −1
Original line number Diff line number Diff line
@@ -970,8 +970,13 @@ static void iopt_unfill_domain(struct io_pagetable *iopt, 
				WARN_ON(!area->storage_domain);

			if (area->storage_domain == domain)

				area->storage_domain = storage_domain;

			if (iopt_is_dmabuf(pages)) {

				if (!iopt_dmabuf_revoked(pages))

					iopt_area_unmap_domain(area, domain);

			}

			mutex_unlock(&pages->mutex);



			if (!iopt_is_dmabuf(pages))

				iopt_area_unmap_domain(area, domain);

		}

		return;
@@ -1261,6 +1266,10 @@ static int iopt_area_split(struct iopt_area *area, unsigned long iova) 
	if (!pages || area->prevent_access)

		return -EBUSY;



	/* Maintaining the domains_itree below is a bit complicated */

	if (iopt_is_dmabuf(pages))

		return -EOPNOTSUPP;



	if (new_start & (alignment - 1) ||

	    iopt_area_start_byte(area, new_start) & (alignment - 1))

		return -EINVAL;

drivers/iommu/iommufd/io_pagetable.h

+8 −0
Original line number Diff line number Diff line
@@ -238,6 +238,14 @@ static inline bool iopt_is_dmabuf(struct iopt_pages *pages) 
	return pages->type == IOPT_ADDRESS_DMABUF;

}



static inline bool iopt_dmabuf_revoked(struct iopt_pages *pages)

{

	lockdep_assert_held(&pages->mutex);

	if (iopt_is_dmabuf(pages))

		return pages->dmabuf.phys.len == 0;

	return false;

}



struct iopt_pages *iopt_alloc_user_pages(void __user *uptr,

					 unsigned long length, bool writable);

struct iopt_pages *iopt_alloc_file_pages(struct file *file, unsigned long start,

drivers/iommu/iommufd/pages.c

+34 −20
Original line number Diff line number Diff line
@@ -1650,6 +1650,9 @@ void iopt_area_unmap_domain(struct iopt_area *area, struct iommu_domain *domain) 
void iopt_area_unfill_domain(struct iopt_area *area, struct iopt_pages *pages,

			     struct iommu_domain *domain)

{

	if (iopt_dmabuf_revoked(pages))

		return;



	__iopt_area_unfill_domain(area, pages, domain,

				  iopt_area_last_index(area));

}
@@ -1670,6 +1673,9 @@ int iopt_area_fill_domain(struct iopt_area *area, struct iommu_domain *domain) 


	lockdep_assert_held(&area->pages->mutex);



	if (iopt_dmabuf_revoked(area->pages))

		return 0;



	rc = pfn_reader_first(&pfns, area->pages, iopt_area_index(area),

			      iopt_area_last_index(area));

	if (rc)
@@ -1729,6 +1735,7 @@ int iopt_area_fill_domains(struct iopt_area *area, struct iopt_pages *pages) 
		return 0;



	mutex_lock(&pages->mutex);

	if (!iopt_dmabuf_revoked(pages)) {

		rc = pfn_reader_first(&pfns, pages, iopt_area_index(area),

				      iopt_area_last_index(area));

		if (rc)
@@ -1753,9 +1760,13 @@ int iopt_area_fill_domains(struct iopt_area *area, struct iopt_pages *pages) 
		if (rc)

			goto out_unmap;



		pfn_reader_destroy(&pfns);

	}



	area->storage_domain = xa_load(&area->iopt->domains, 0);

	interval_tree_insert(&area->pages_node, &pages->domains_itree);

	goto out_destroy;

	mutex_unlock(&pages->mutex);

	return 0;



out_unmap:

	pfn_reader_release_pins(&pfns);
@@ -1782,7 +1793,6 @@ int iopt_area_fill_domains(struct iopt_area *area, struct iopt_pages *pages) 
							end_index);

		}

	}

out_destroy:

	pfn_reader_destroy(&pfns);

out_unlock:

	mutex_unlock(&pages->mutex);
@@ -1809,11 +1819,15 @@ void iopt_area_unfill_domains(struct iopt_area *area, struct iopt_pages *pages) 
	if (!area->storage_domain)

		goto out_unlock;



	xa_for_each(&iopt->domains, index, domain)

		if (domain != area->storage_domain)

	xa_for_each(&iopt->domains, index, domain) {

		if (domain == area->storage_domain)

			continue;



		if (!iopt_dmabuf_revoked(pages))

			iopt_area_unmap_domain_range(

				area, domain, iopt_area_index(area),

				iopt_area_last_index(area));

	}



	if (IS_ENABLED(CONFIG_IOMMUFD_TEST))

		WARN_ON(RB_EMPTY_NODE(&area->pages_node.rb));