[Xen-devel] [PATCH] x86/p2m-pt: delay freeing of intermediate page tables

["Jan Beulich" <JBeulich@xxxxxxxx>]

Old intermediate page tables must be freed only after IOMMU side
updates/flushes have got carried out.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
This was split out from a previously submitted patch.

--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -487,8 +487,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
     /* XXX -- this might be able to be faster iff current->domain == d */
     void *table;
     unsigned long i, gfn_remainder = gfn;
-    l1_pgentry_t *p2m_entry;
-    l1_pgentry_t entry_content;
+    l1_pgentry_t *p2m_entry, entry_content;
+    /* Intermediate table to free if we're replacing it with a superpage. */
+    l1_pgentry_t intermediate_entry = l1e_empty();
     l2_pgentry_t l2e_content;
     l3_pgentry_t l3e_content;
     int rc;
@@ -538,7 +539,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
      */
     if ( page_order == PAGE_ORDER_1G )
     {
-        l1_pgentry_t old_entry = l1e_empty();
         p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn,
                                    L3_PAGETABLE_SHIFT - PAGE_SHIFT,
                                    L3_PAGETABLE_ENTRIES);
@@ -548,7 +548,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
         {
             /* We're replacing a non-SP page with a superpage.  Make sure to
              * handle freeing the table properly. */
-            old_entry = *p2m_entry;
+            intermediate_entry = *p2m_entry;
         }
 
         ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
@@ -566,10 +566,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
 
         p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 3);
         /* NB: paging_write_p2m_entry() handles tlb flushes properly */
-
-        /* Free old intermediate tables if necessary */
-        if ( l1e_get_flags(old_entry) & _PAGE_PRESENT )
-            p2m_free_entry(p2m, &old_entry, page_order);
     }
     else 
     {
@@ -610,7 +606,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
     }
     else if ( page_order == PAGE_ORDER_2M )
     {
-        l1_pgentry_t old_entry = l1e_empty();
         p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn,
                                    L2_PAGETABLE_SHIFT - PAGE_SHIFT,
                                    L2_PAGETABLE_ENTRIES);
@@ -622,7 +617,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
         {
             /* We're replacing a non-SP page with a superpage.  Make sure to
              * handle freeing the table properly. */
-            old_entry = *p2m_entry;
+            intermediate_entry = *p2m_entry;
         }
         
         ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
@@ -643,10 +638,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
 
         p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 2);
         /* NB: paging_write_p2m_entry() handles tlb flushes properly */
-
-        /* Free old intermediate tables if necessary */
-        if ( l1e_get_flags(old_entry) & _PAGE_PRESENT )
-            p2m_free_entry(p2m, &old_entry, page_order);
     }
 
     /* Track the highest gfn for which we have ever had a valid mapping */
@@ -674,6 +665,14 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
         }
     }
 
+    /*
+     * Free old intermediate tables if necessary.  This has to be the
+     * last thing we do, after removal from the IOMMU tables, so as to
+     * avoid a potential use-after-free.
+     */
+    if ( l1e_get_flags(intermediate_entry) & _PAGE_PRESENT )
+        p2m_free_entry(p2m, &intermediate_entry, page_order);
+
  out:
     unmap_domain_page(table);
     return rc;

Attachment: x86-p2m-pt-delay-free.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel