[xen stable-4.6] gnttab: don't blindly free status pages upon version change

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[xen stable-4.6] gnttab: don't blindly free status pages upon version change

patchbot
commit 4972c38f7930ca4a271eed2aed4f5f38bb3d02ac
Author:     Jan Beulich <[hidden email]>
AuthorDate: Tue Feb 27 14:37:39 2018 +0100
Commit:     Jan Beulich <[hidden email]>
CommitDate: Tue Feb 27 14:37:39 2018 +0100

    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 <[hidden email]>
    Reviewed-by: Stefano Stabellini <[hidden email]>
    Reviewed-by: Andrew Cooper <[hidden email]>
    master commit: 38bfcc165dda5f4284d7c218b91df9e144ddd88d
    master date: 2018-02-27 14:07:12 +0100
---
 xen/arch/arm/mm.c                 | 19 +++++++++---
 xen/common/grant_table.c          | 62 +++++++++++++++++++++++++++++++++++----
 xen/include/asm-arm/grant_table.h | 11 +++++++
 xen/include/asm-x86/grant_table.h | 12 ++++++--
 4 files changed, 93 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6d18e78150..ed85805275 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1089,12 +1089,23 @@ int xenmem_add_to_physmap_one(
                 mfn = INVALID_MFN;
         }
         
+        if ( mfn != INVALID_MFN &&
+             gfn_x(gnttab_get_frame_gfn(d, status, idx)) != INVALID_GFN )
+        {
+            rc = guest_physmap_remove_page(d,
+                                           gfn_x(gnttab_get_frame_gfn(d, status,
+                                                                      idx)),
+                                           mfn, 0);
+            if ( rc )
+            {
+                write_unlock(&d->grant_table->lock);
+                return rc;
+            }
+        }
+
         if ( mfn != INVALID_MFN )
         {
-            if ( status )
-                d->arch.grant_status_gfn[idx] = _gfn(gpfn);
-            else
-                d->arch.grant_shared_gfn[idx] = _gfn(gpfn);
+            gnttab_set_frame_gfn(d, status, idx, _gfn(gpfn));
 
             t = p2m_ram_rw;
         }
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index b946250cf1..692a8bcb25 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1512,23 +1512,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(d, 1, 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_x(gfn) == INVALID_GFN
+                     ? 0
+                     : guest_physmap_remove_page(d, gfn_x(gfn),
+                                                 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(d, 1, i, _gfn(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;
 }
 
 /*
@@ -2768,8 +2819,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++ )
diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index a51cc6579c..fb393d4f45 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -20,6 +20,17 @@ static inline int replace_grant_supported(void)
     return 1;
 }
 
+#define gnttab_set_frame_gfn(d, st, idx, gfn)                            \
+    do {                                                                 \
+        ((st) ? (d)->arch.grant_status_gfn                               \
+              : (d)->arch.grant_shared_gfn)[idx] = (gfn);                \
+    } while ( 0 )
+
+#define gnttab_get_frame_gfn(d, st, idx) ({                              \
+   _gfn((st) ? gnttab_status_gmfn(d, (d)->grant_table, idx)              \
+             : gnttab_shared_gmfn(d, (d)->grant_table, 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 9ca631c4e3..ff1ab2e327 100644
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -18,6 +18,14 @@ int create_grant_host_mapping(uint64_t addr, unsigned long frame,
 int replace_grant_host_mapping(
     uint64_t addr, unsigned long frame, uint64_t new_addr, unsigned int flags);
 
+#define gnttab_set_frame_gfn(d, st, idx, gfn) do {} while ( 0 )
+#define gnttab_get_frame_gfn(d, st, idx) ({                              \
+    unsigned long mfn_ = (st) ? gnttab_status_mfn((d)->grant_table, idx) \
+                              : gnttab_shared_mfn((d)->grant_table, idx); \
+    unsigned long gpfn_ = get_gpfn_from_mfn(mfn_);                       \
+    _gfn(VALID_M2P(gpfn_) ? gpfn_ : INVALID_GFN);                        \
+})
+
 #define gnttab_create_shared_page(d, t, i)                               \
     do {                                                                 \
         share_xen_page_with_guest(                                       \
@@ -33,11 +41,11 @@ int replace_grant_host_mapping(
     } 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)                         \
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.6

_______________________________________________
Xen-changelog mailing list
[hidden email]
https://lists.xenproject.org/xen-changelog