[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen staging-4.15] gnttab: add preemption check to gnttab_release_mappings()



commit 9bfbde40bc268dc479dde785d2435fd5a2e61efd
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Wed Aug 25 14:44:23 2021 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Aug 25 14:44:23 2021 +0200

    gnttab: add preemption check to gnttab_release_mappings()
    
    A guest may die with many grant mappings still in place, or simply with
    a large maptrack table. Iterating through this may take more time than
    is reasonable without intermediate preemption (to run softirqs and
    perhaps the scheduler).
    
    Move the invocation of the function to the section where other
    restartable functions get invoked, and have the function itself check
    for preemption every once in a while. Have it iterate the table
    backwards, such that decreasing the maptrack limit is all it takes to
    convey restart information.
    
    In domain_teardown() introduce PROG_none such that inserting at the
    front will be easier going forward.
    
    This is part of CVE-2021-28698 / XSA-380.
    
    Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
    master commit: b1ee10be5625b7d502cef1e6ee3818610ab0d29c
    master date: 2021-08-25 14:18:18 +0200
---
 xen/common/domain.c           | 12 ++++++++---
 xen/common/grant_table.c      | 46 ++++++++++++++++++++++++++++++++++++-------
 xen/include/xen/grant_table.h |  6 ++----
 3 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index d85984638a..dabb15a06c 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -412,11 +412,18 @@ static int domain_teardown(struct domain *d)
         v = d->teardown.vcpu
 
         enum {
-            PROG_vcpu_teardown = 1,
+            PROG_none,
+            PROG_gnttab_mappings,
+            PROG_vcpu_teardown,
             PROG_done,
         };
 
-    case 0:
+    case PROG_none:
+        rc = gnttab_release_mappings(d);
+        if ( rc )
+            return rc;
+
+    PROGRESS(gnttab_mappings):
         for_each_vcpu ( d, v )
         {
             PROGRESS_VCPU(teardown);
@@ -908,7 +915,6 @@ int domain_kill(struct domain *d)
             return domain_kill(d);
         d->is_dying = DOMDYING_dying;
         argo_destroy(d);
-        gnttab_release_mappings(d);
         vnuma_destroy(d->vnuma);
         domain_set_outstanding_pages(d, 0);
         /* fallthrough */
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index ab30e2e8cf..e43ff7df4f 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -64,7 +64,13 @@ struct grant_table {
     unsigned int          nr_grant_frames;
     /* Number of grant status frames shared with guest (for version 2) */
     unsigned int          nr_status_frames;
-    /* Number of available maptrack entries. */
+    /*
+     * Number of available maptrack entries.  For cleanup purposes it is
+     * important to realize that this field and @maptrack further down will
+     * only ever be accessed by the local domain.  Thus it is okay to clean
+     * up early, and to shrink the limit for the purpose of tracking cleanup
+     * progress.
+     */
     unsigned int          maptrack_limit;
     /* Shared grant table (see include/public/grant_table.h). */
     union {
@@ -3691,9 +3697,7 @@ do_grant_table_op(
 #include "compat/grant_table.c"
 #endif
 
-void
-gnttab_release_mappings(
-    struct domain *d)
+int gnttab_release_mappings(struct domain *d)
 {
     struct grant_table   *gt = d->grant_table, *rgt;
     struct grant_mapping *map;
@@ -3707,8 +3711,32 @@ gnttab_release_mappings(
 
     BUG_ON(!d->is_dying);
 
-    for ( handle = 0; handle < gt->maptrack_limit; handle++ )
+    if ( !gt || !gt->maptrack )
+        return 0;
+
+    for ( handle = gt->maptrack_limit; handle; )
     {
+        /*
+         * Deal with full pages such that their freeing (in the body of the
+         * if()) remains simple.
+         */
+        if ( handle < gt->maptrack_limit && !(handle % MAPTRACK_PER_PAGE) )
+        {
+            /*
+             * Changing maptrack_limit alters nr_maptrack_frames()'es return
+             * value. Free the then excess trailing page right here, rather
+             * than leaving it to grant_table_destroy() (and in turn requiring
+             * to leave gt->maptrack_limit unaltered).
+             */
+            gt->maptrack_limit = handle;
+            FREE_XENHEAP_PAGE(gt->maptrack[nr_maptrack_frames(gt)]);
+
+            if ( hypercall_preempt_check() )
+                return -ERESTART;
+        }
+
+        --handle;
+
         map = &maptrack_entry(gt, handle);
         if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) )
             continue;
@@ -3792,6 +3820,11 @@ gnttab_release_mappings(
 
         map->flags = 0;
     }
+
+    gt->maptrack_limit = 0;
+    FREE_XENHEAP_PAGE(gt->maptrack[0]);
+
+    return 0;
 }
 
 void grant_table_warn_active_grants(struct domain *d)
@@ -3855,8 +3888,7 @@ grant_table_destroy(
         free_xenheap_page(t->shared_raw[i]);
     xfree(t->shared_raw);
 
-    for ( i = 0; i < nr_maptrack_frames(t); i++ )
-        free_xenheap_page(t->maptrack[i]);
+    ASSERT(!t->maptrack_limit);
     vfree(t->maptrack);
 
     for ( i = 0; i < nr_active_grant_frames(t); i++ )
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 63b6dc78f4..cbd1ce37db 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -46,9 +46,7 @@ void grant_table_init_vcpu(struct vcpu *v);
 void grant_table_warn_active_grants(struct domain *d);
 
 /* Domain death release of granted mappings of other domains' memory. */
-void
-gnttab_release_mappings(
-    struct domain *d);
+int gnttab_release_mappings(struct domain *d);
 
 int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
                             gfn_t *gfn, uint16_t *status);
@@ -79,7 +77,7 @@ static inline void grant_table_init_vcpu(struct vcpu *v) {}
 
 static inline void grant_table_warn_active_grants(struct domain *d) {}
 
-static inline void gnttab_release_mappings(struct domain *d) {}
+static inline int gnttab_release_mappings(struct domain *d) { return 0; }
 
 static inline int mem_sharing_gref_to_gfn(struct grant_table *gt,
                                           grant_ref_t ref,
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.15



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.