[xen master] xen/arm: p2m: Handle preemption when freeing intermediate page tables

[patchbot@xxxxxxx]

commit 3202084566bba0ef0c45caf8c24302f83d92f9c8
Author:     Julien Grall <jgrall@xxxxxxxxxx>
AuthorDate: Mon Jun 6 06:17:26 2022 +0000
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Oct 11 14:20:56 2022 +0200

    xen/arm: p2m: Handle preemption when freeing intermediate page tables
    
    At the moment the P2M page tables will be freed when the domain structure
    is freed without any preemption. As the P2M is quite large, iterating
    through this may take more time than it is reasonable without intermediate
    preemption (to run softirqs and perhaps scheduler).
    
    Split p2m_teardown() in two parts: one preemptible and called when
    relinquishing the resources, the other one non-preemptible and called
    when freeing the domain structure.
    
    As we are now freeing the P2M pages early, we also need to prevent
    further allocation if someone call p2m_set_entry() past p2m_teardown()
    (I wasn't able to prove this will never happen). This is done by
    the checking domain->is_dying from previous patch in p2m_set_entry().
    
    Similarly, we want to make sure that no-one can accessed the free
    pages. Therefore the root is cleared before freeing pages.
    
    This is part of CVE-2022-33746 / XSA-410.
    
    Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
    Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
    Tested-by: Henry Wang <Henry.Wang@xxxxxxx>
    Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
---
 xen/arch/arm/domain.c          | 10 +++++++--
 xen/arch/arm/include/asm/p2m.h | 13 ++++++++++--
 xen/arch/arm/p2m.c             | 47 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2d6253181a..746ad3438a 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -795,10 +795,10 @@ fail:
 void arch_domain_destroy(struct domain *d)
 {
     /* IOMMU page table is shared with P2M, always call
-     * iommu_domain_destroy() before p2m_teardown().
+     * iommu_domain_destroy() before p2m_final_teardown().
      */
     iommu_domain_destroy(d);
-    p2m_teardown(d);
+    p2m_final_teardown(d);
     domain_vgic_free(d);
     domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
@@ -1001,6 +1001,7 @@ enum {
     PROG_xen,
     PROG_page,
     PROG_mapping,
+    PROG_p2m,
     PROG_done,
 };
 
@@ -1061,6 +1062,11 @@ int domain_relinquish_resources(struct domain *d)
         if ( ret )
             return ret;
 
+    PROGRESS(p2m):
+        ret = p2m_teardown(d);
+        if ( ret )
+            return ret;
+
     PROGRESS(done):
         break;
 
diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h
index 8cce459b67..a15ea67f9b 100644
--- a/xen/arch/arm/include/asm/p2m.h
+++ b/xen/arch/arm/include/asm/p2m.h
@@ -192,8 +192,17 @@ void setup_virt_paging(void);
 /* Init the datastructures for later use by the p2m code */
 int p2m_init(struct domain *d);
 
-/* Return all the p2m resources to Xen. */
-void p2m_teardown(struct domain *d);
+/*
+ * The P2M resources are freed in two parts:
+ *  - p2m_teardown() will be called when relinquish the resources. It
+ *    will free large resources (e.g. intermediate page-tables) that
+ *    requires preemption.
+ *  - p2m_final_teardown() will be called when domain struct is been
+ *    freed. This *cannot* be preempted and therefore one small
+ *    resources should be freed here.
+ */
+int p2m_teardown(struct domain *d);
+void p2m_final_teardown(struct domain *d);
 
 /*
  * Remove mapping refcount on each mapping page in the p2m
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index c2e0b116c4..b445f4d754 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1551,17 +1551,58 @@ static void p2m_free_vmid(struct domain *d)
     spin_unlock(&vmid_alloc_lock);
 }
 
-void p2m_teardown(struct domain *d)
+int p2m_teardown(struct domain *d)
 {
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
+    unsigned long count = 0;
     struct page_info *pg;
+    unsigned int i;
+    int rc = 0;
+
+    p2m_write_lock(p2m);
+
+    /*
+     * We are about to free the intermediate page-tables, so clear the
+     * root to prevent any walk to use them.
+     */
+    for ( i = 0; i < P2M_ROOT_PAGES; i++ )
+        clear_and_clean_page(p2m->root + i);
+
+    /*
+     * The domain will not be scheduled anymore, so in theory we should
+     * not need to flush the TLBs. Do it for safety purpose.
+     *
+     * Note that all the devices have already been de-assigned. So we don't
+     * need to flush the IOMMU TLB here.
+     */
+    p2m_force_tlb_flush_sync(p2m);
+
+    while ( (pg = page_list_remove_head(&p2m->pages)) )
+    {
+        free_domheap_page(pg);
+        count++;
+        /* Arbitrarily preempt every 512 iterations */
+        if ( !(count % 512) && hypercall_preempt_check() )
+        {
+            rc = -ERESTART;
+            break;
+        }
+    }
+
+    p2m_write_unlock(p2m);
+
+    return rc;
+}
+
+void p2m_final_teardown(struct domain *d)
+{
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);
 
     /* p2m not actually initialized */
     if ( !p2m->domain )
         return;
 
-    while ( (pg = page_list_remove_head(&p2m->pages)) )
-        free_domheap_page(pg);
+    ASSERT(page_list_empty(&p2m->pages));
 
     if ( p2m->root )
         free_domheap_pages(p2m->root, P2M_ROOT_ORDER);
--
generated by git-patchbot for /home/xen/git/xen.git#master