[PATCH] xen/iommu: arm: Don't insert an IOMMU mapping when the grantee and granter...

[Julien Grall <julien@xen.org>]

From: Julien Grall <jgrall@amazon.com>

... are the same.

When the IOMMU is enabled and the domain is direct mapped (e.g. Dom0),
Xen will insert a 1:1 mapping for each grant mapping in the P2M to
allow DMA.

This works quite well when the grantee and granter and not the same
because the GFN in the P2M should not be mapped. However, if they are
the same, we will overwrite the mapping. Worse, it will be completely
removed when the grant is unmapped.

As the domain is direct mapped, a 1:1 mapping should always present in
the P2M. This is not 100% guaranteed if the domain decides to mess with
the P2M. However, such domain would already end up in trouble as the
page would be soon be freed (when the last reference dropped).

Add an additional check in arm_iommu_{,un}map_page() to check whether
the page belongs to the domain. If it is belongs to it, then ignore the
request.

Note that we don't need to hold an extra reference on the page because
the grant code will already do it for us.

Reported-by: Rahul Singh <Rahul.Singh@arm.com>
Fixes: 552710b38863 ("xen/arm: grant: Add another entry to map MFN 1:1 in dom0 p2m")
Signed-off-by: Julien Grall <jgrall@amazon.com>

---

This is a candidate for 4.15. Without the patch, it would not be
possible to get the frontend and backend in the same domain (useful when
trying to map the guest block device in dom0).
---
 xen/drivers/passthrough/arm/iommu_helpers.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/xen/drivers/passthrough/arm/iommu_helpers.c b/xen/drivers/passthrough/arm/iommu_helpers.c
index a36e2b8e6c42..35a813308b8c 100644
--- a/xen/drivers/passthrough/arm/iommu_helpers.c
+++ b/xen/drivers/passthrough/arm/iommu_helpers.c
@@ -53,6 +53,17 @@ int __must_check arm_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
 
     t = (flags & IOMMUF_writable) ? p2m_iommu_map_rw : p2m_iommu_map_ro;
 
+    /*
+     * The granter and grantee can be the same domain. In normal
+     * condition, the 1:1 mapping should already present in the P2M so
+     * we want to avoid overwriting it.
+     *
+     * Note that there is no guarantee the 1:1 mapping will be present
+     * if the domain decides to mess with the P2M.
+     */
+    if ( page_get_owner(mfn_to_page(mfn)) == d )
+        return 0;
+
     /*
      * The function guest_physmap_add_entry replaces the current mapping
      * if there is already one...
@@ -71,6 +82,13 @@ int __must_check arm_iommu_unmap_page(struct domain *d, dfn_t dfn,
     if ( !is_domain_direct_mapped(d) )
         return -EINVAL;
 
+    /*
+     * When the granter and grantee are the same, we didn't insert a
+     * mapping. So ignore the unmap request.
+     */
+    if ( page_get_owner(mfn_to_page(_mfn(dfn_x(dfn)))) == d )
+        return 0;
+
     return guest_physmap_remove_page(d, _gfn(dfn_x(dfn)), _mfn(dfn_x(dfn)), 0);
 }
 
-- 
2.17.1