x86/p2m: guard (in particular) identity mapping entries

Such entries, created by set_identity_p2m_entry(), should only be
destroyed by clear_identity_p2m_entry(). However, similarly, entries
created by set_mmio_p2m_entry() should only be torn down by
clear_mmio_p2m_entry(), so the logic gets based upon p2m_mmio_direct as
the entry type (separation between "ordinary" and 1:1 mappings would
require a further indicator to tell apart the two).

As to the guest_remove_page() change, commit 48dfb297a20a ("x86/PVH:
allow guest_remove_page to remove p2m_mmio_direct pages"), which
introduced the call to clear_mmio_p2m_entry(), claimed this was done for
hwdom only without this actually having been the case. However, this
code shouldn't be there in the first place, as MMIO entries shouldn't be
dropped this way. Avoid triggering the warning again that 48dfb297a20a
silenced by an adjustment to xenmem_add_to_physmap_one() instead.

Note that guest_physmap_mark_populate_on_demand() gets tightened beyond
the immediate purpose of this change.

Note also that I didn't inspect code which isn't security supported,
e.g. sharing, paging, or altp2m.

This is CVE-2021-28694 / part of XSA-378.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index ae153fa6e606d541ae6ad45594df8e57e0a326f4..8abc57265c106060b2a1c5bb89a5990e209ab606 100644 (file)
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1299,17 +1299,17 @@ guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn_l,
 
         p2m->get_entry(p2m, gfn_add(gfn, i), &ot, &a, 0, &cur_order, NULL);
         n = 1UL << min(order, cur_order);
-        if ( p2m_is_ram(ot) )
+        if ( ot == p2m_populate_on_demand )
+        {
+            /* Count how many PoD entries we'll be replacing if successful */
+            pod_count += n;
+        }
+        else if ( ot != p2m_invalid && ot != p2m_mmio_dm )
         {
             P2M_DEBUG("gfn_to_mfn returned type %d!\n", ot);
             rc = -EBUSY;
             goto out;
         }
-        else if ( ot == p2m_populate_on_demand )
-        {
-            /* Count how man PoD entries we'll be replacing if successful */
-            pod_count += n;
-        }
     }
 
     /* Now, actually do the two-way mapping */

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 0ca849ec777cf23d173722ed38f0966d005495f9..09cbd8a07d8e896d5aa04018d792a77cd99aed98 100644 (file)
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -805,7 +805,8 @@ p2m_remove_page(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
                                           &cur_order, NULL);
 
         if ( p2m_is_valid(t) &&
-             (!mfn_valid(mfn) || !mfn_eq(mfn_add(mfn, i), mfn_return)) )
+             (!mfn_valid(mfn) || t == p2m_mmio_direct ||
+              !mfn_eq(mfn_add(mfn, i), mfn_return)) )
             return -EILSEQ;
 
         i += (1UL << cur_order) -
@@ -912,7 +913,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
     if ( p2m_is_foreign(t) )
         return -EINVAL;
 
-    if ( !mfn_valid(mfn) )
+    if ( !mfn_valid(mfn) || t == p2m_mmio_direct )
     {
         ASSERT_UNREACHABLE();
         return -EINVAL;
@@ -956,7 +957,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
         }
         if ( p2m_is_special(ot) )
         {
-            /* Don't permit unmapping grant/foreign this way. */
+            /* Don't permit unmapping grant/foreign/direct-MMIO this way. */
             domain_crash(d);
             p2m_unlock(p2m);
             
@@ -1364,8 +1365,8 @@ int set_mmio_p2m_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
  *    order+1  for caller to retry with order (guaranteed smaller than
  *             the order value passed in)
  */
-int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn_l, mfn_t mfn,
-                         unsigned int order)
+static int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn_l,
+                                mfn_t mfn, unsigned int order)
 {
     int rc = -EINVAL;
     gfn_t gfn = _gfn(gfn_l);
@@ -2766,7 +2767,9 @@ int xenmem_add_to_physmap_one(
 
     /* Remove previously mapped page if it was present. */
     prev_mfn = get_gfn(d, gfn_x(gpfn), &p2mt);
-    if ( mfn_valid(prev_mfn) )
+    if ( p2mt == p2m_mmio_direct )
+        rc = -EPERM;
+    else if ( mfn_valid(prev_mfn) )
     {
         if ( is_special_page(mfn_to_page(prev_mfn)) )
             /* Special pages are simply unhooked from this phys slot. */

diff --git a/xen/common/memory.c b/xen/common/memory.c
index e07bd9a5ea4b769feabafecf52135098fe3056ff..74babb0bd7661a97b22149d5b48f7741e1197b2e 100644 (file)
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -332,7 +332,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
     }
     if ( p2mt == p2m_mmio_direct )
     {
-        rc = clear_mmio_p2m_entry(d, gmfn, mfn, PAGE_ORDER_4K);
+        rc = -EPERM;
         goto out_put_gfn;
     }
 #else
@@ -1888,6 +1888,15 @@ int check_get_page_from_gfn(struct domain *d, gfn_t gfn, bool readonly,
         return -EAGAIN;
     }
 #endif
+#ifdef CONFIG_X86
+    if ( p2mt == p2m_mmio_direct )
+    {
+        if ( page )
+            put_page(page);
+
+        return -EPERM;
+    }
+#endif
 
     if ( !page )
         return -EINVAL;

diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index bf9967a0231de743eff4eb21559cbf6eafb012f2..c6d41ac0b6b3e2242a1910813745ed49d62c29a5 100644 (file)
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -156,7 +156,8 @@ typedef unsigned int p2m_query_t;
 
 /* Types established/cleaned up via special accessors. */
 #define P2M_SPECIAL_TYPES (P2M_GRANT_TYPES | \
-                           p2m_to_mask(p2m_map_foreign))
+                           p2m_to_mask(p2m_map_foreign) | \
+                           p2m_to_mask(p2m_mmio_direct))
 
 /* Valid types not necessarily associated with a (valid) MFN. */
 #define P2M_INVALID_MFN_TYPES (P2M_POD_TYPES                  \
@@ -627,19 +628,9 @@ int p2m_finish_type_change(struct domain *d,
 int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start,
                           unsigned long end);
 
-#ifdef CONFIG_HVM
 /* Set mmio addresses in the p2m table (for pass-through) */
 int set_mmio_p2m_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
                        unsigned int order);
-int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
-                         unsigned int order);
-#else
-static inline int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn,
-                                       mfn_t mfn, unsigned int order)
-{
-    return -EIO;
-}
-#endif
 
 /* Set identity addresses in the p2m table (for pass-through) */
 int set_identity_p2m_entry(struct domain *d, unsigned long gfn,