gnttab: don't blindly free status pages upon version change

There may still be active mappings, which would trigger the respective
BUG_ON(). Split the loop into one dealing with the page attributes and
the second (when the first fully passed) freeing the pages. Return an
error if any pages still have pending references.

This is part of XSA-255.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 43ec69d5190cd17ca2ee0d810c350603c0cfaa4b..210de0f37ce49afbe6ba86f279dac2260696f125 100644 (file)
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1636,23 +1636,74 @@ status_alloc_failed:
     return -ENOMEM;
 }
 
-static void
+static int
 gnttab_unpopulate_status_frames(struct domain *d, struct grant_table *gt)
 {
-    int i;
+    unsigned int i;
 
     for ( i = 0; i < nr_status_frames(gt); i++ )
     {
         struct page_info *pg = virt_to_page(gt->status[i]);
+        gfn_t gfn = gnttab_get_frame_gfn(gt, true, i);
+
+        /*
+         * For translated domains, recovering from failure after partial
+         * changes were made is more complicated than it seems worth
+         * implementing at this time. Hence respective error paths below
+         * crash the domain in such a case.
+         */
+        if ( paging_mode_translate(d) )
+        {
+            int rc = gfn_eq(gfn, INVALID_GFN)
+                     ? 0
+                     : guest_physmap_remove_page(d, gfn,
+                                                 _mfn(page_to_mfn(pg)), 0);
+
+            if ( rc )
+            {
+                gprintk(XENLOG_ERR,
+                        "Could not remove status frame %u (GFN %#lx) from P2M\n",
+                        i, gfn_x(gfn));
+                domain_crash(d);
+                return rc;
+            }
+            gnttab_set_frame_gfn(gt, true, i, INVALID_GFN);
+        }
 
         BUG_ON(page_get_owner(pg) != d);
         if ( test_and_clear_bit(_PGC_allocated, &pg->count_info) )
             put_page(pg);
-        BUG_ON(pg->count_info & ~PGC_xen_heap);
+
+        if ( pg->count_info & ~PGC_xen_heap )
+        {
+            if ( paging_mode_translate(d) )
+            {
+                gprintk(XENLOG_ERR,
+                        "Wrong page state %#lx of status frame %u (GFN %#lx)\n",
+                        pg->count_info, i, gfn_x(gfn));
+                domain_crash(d);
+            }
+            else
+            {
+                if ( get_page(pg, d) )
+                    set_bit(_PGC_allocated, &pg->count_info);
+                while ( i-- )
+                    gnttab_create_status_page(d, gt, i);
+            }
+            return -EBUSY;
+        }
+
+        page_set_owner(pg, NULL);
+    }
+
+    for ( i = 0; i < nr_status_frames(gt); i++ )
+    {
         free_xenheap_page(gt->status[i]);
         gt->status[i] = NULL;
     }
     gt->nr_status_frames = 0;
+
+    return 0;
 }
 
 /*
@@ -2962,8 +3013,9 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
         break;
     }
 
-    if ( op.version < 2 && gt->gt_version == 2 )
-        gnttab_unpopulate_status_frames(currd, gt);
+    if ( op.version < 2 && gt->gt_version == 2 &&
+         (res = gnttab_unpopulate_status_frames(currd, gt)) != 0 )
+        goto out_unlock;
 
     /* Make sure there's no crud left over from the old version. */
     for ( i = 0; i < nr_grant_frames(gt); i++ )
@@ -3804,6 +3856,11 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
             rc = -EINVAL;
     }
 
+    if ( !rc && paging_mode_translate(d) &&
+         !gfn_eq(gnttab_get_frame_gfn(gt, status, idx), INVALID_GFN) )
+        rc = guest_physmap_remove_page(d, gnttab_get_frame_gfn(gt, status, idx),
+                                       *mfn, 0);
+
     if ( !rc )
         gnttab_set_frame_gfn(gt, status, idx, gfn);
 

diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index f5361c7dcf9ed611904c96b15bd764d3f64b0917..5b8994cbd5efe87c0ec9e1f903d7bd0bd20ea871 100644 (file)
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -73,6 +73,11 @@ static inline unsigned int gnttab_dom0_max(void)
             (gfn);                                                       \
     } while ( 0 )
 
+#define gnttab_get_frame_gfn(gt, st, idx) ({                             \
+   _gfn((st) ? gnttab_status_gmfn(NULL, gt, idx)                         \
+             : gnttab_shared_gmfn(NULL, gt, idx));                       \
+})
+
 #define gnttab_create_shared_page(d, t, i)                               \
     do {                                                                 \
         share_xen_page_with_guest(                                       \

diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h
index 3177d82b69bb4590e62dc5faf0c4021006035a12..66e9742003662c9b25f42e0f08a1d7935c468378 100644 (file)
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -47,6 +47,12 @@ static inline unsigned int gnttab_dom0_max(void)
 #define gnttab_init_arch(gt) 0
 #define gnttab_destroy_arch(gt) do {} while ( 0 )
 #define gnttab_set_frame_gfn(gt, st, idx, gfn) do {} while ( 0 )
+#define gnttab_get_frame_gfn(gt, st, idx) ({                             \
+    unsigned long mfn_ = (st) ? gnttab_status_mfn(gt, idx)               \
+                              : gnttab_shared_mfn(gt, idx);              \
+    unsigned long gpfn_ = get_gpfn_from_mfn(mfn_);                       \
+    VALID_M2P(gpfn_) ? _gfn(gpfn_) : INVALID_GFN;                        \
+})
 
 #define gnttab_create_shared_page(d, t, i)                               \
     do {                                                                 \
@@ -63,11 +69,11 @@ static inline unsigned int gnttab_dom0_max(void)
     } while ( 0 )
 
 
-#define gnttab_shared_mfn(d, t, i)                      \
+#define gnttab_shared_mfn(t, i)                         \
     ((virt_to_maddr((t)->shared_raw[i]) >> PAGE_SHIFT))
 
 #define gnttab_shared_gmfn(d, t, i)                     \
-    (mfn_to_gmfn(d, gnttab_shared_mfn(d, t, i)))
+    (mfn_to_gmfn(d, gnttab_shared_mfn(t, i)))
 
 
 #define gnttab_status_mfn(t, i)                         \