File xsa222-2.patch of Package xen.5015

Overview Repositories Revisions Requests Users Attributes Meta

File xsa222-2.patch of Package xen.5015

guest_physmap_remove_page() needs its return value checked

Callers, namely such subsequently freeing the page, must not blindly
assume success - the function may namely fail when needing to shatter a
super page, but there not being memory available for the then needed
intermediate page table.

As it happens, guest_remove_page() callers now also all check the
return value.

Furthermore a missed put_gfn() on an error path in gnttab_transfer() is
also being taken care of.

This is XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Index: xen-4.4.4-testing/xen/arch/arm/mm.c
===================================================================
--- xen-4.4.4-testing.orig/xen/arch/arm/mm.c
+++ xen-4.4.4-testing/xen/arch/arm/mm.c
@@ -1219,13 +1219,14 @@ int replace_grant_host_mapping(unsigned
 {
     unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
     struct domain *d = current->domain;
+    int rc;
 
     if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
         return GNTST_general_error;
 
-    guest_physmap_remove_page(d, gfn, mfn, 0);
+    rc = guest_physmap_remove_page(d, gfn, mfn, 0);
 
-    return GNTST_okay;
+    return rc ? GNTST_general_error : GNTST_okay;
 }
 
 int is_iomem_page(unsigned long mfn)
Index: xen-4.4.4-testing/xen/arch/arm/p2m.c
===================================================================
--- xen-4.4.4-testing.orig/xen/arch/arm/p2m.c
+++ xen-4.4.4-testing/xen/arch/arm/p2m.c
@@ -501,14 +501,13 @@ int guest_physmap_add_entry(struct domai
                              pfn_to_paddr(mfn), MATTR_MEM, t);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gpfn,
-                               unsigned long mfn, unsigned int page_order)
+int guest_physmap_remove_page(struct domain *d, unsigned long gfn,
+                              unsigned long mfn, unsigned int page_order)
 {
-    apply_p2m_changes(d, REMOVE,
-                      pfn_to_paddr(gpfn),
-                      pfn_to_paddr(gpfn + (1<<page_order)),
-                      pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid);
+    return apply_p2m_changes(d, REMOVE,
+                             pfn_to_paddr(gfn),
+                             pfn_to_paddr(gfn + (1 << page_order)),
+                             pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid);
 }
 
 int p2m_alloc_table(struct domain *d)
Index: xen-4.4.4-testing/xen/arch/x86/mm.c
===================================================================
--- xen-4.4.4-testing.orig/xen/arch/x86/mm.c
+++ xen-4.4.4-testing/xen/arch/x86/mm.c
@@ -4094,7 +4094,11 @@ static int replace_grant_p2m_mapping(
                  type, mfn_x(old_mfn), frame);
         return GNTST_general_error;
     }
-    guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K);
+    if ( guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K) )
+    {
+        put_gfn(d, gfn);
+        return GNTST_general_error;
+    }
 
     put_gfn(d, gfn);
     return GNTST_okay;
@@ -4617,7 +4621,7 @@ int xenmem_add_to_physmap_one(
     struct page_info *page = NULL;
     unsigned long gfn = 0; /* gcc ... */
     unsigned long prev_mfn, mfn = 0, old_gpfn;
-    int rc;
+    int rc = 0;
     p2m_type_t p2mt;
 
     switch ( space )
@@ -4689,25 +4693,30 @@ int xenmem_add_to_physmap_one(
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot. */
-            guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
+            rc = guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(d, gpfn);
+            rc = guest_remove_page(d, gpfn);
     }
     /* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
     put_gfn(d, gpfn);
 
+    if ( rc )
+        goto put_both;
+
     /* Unmap from old location, if any. */
     old_gpfn = get_gpfn_from_mfn(mfn);
     ASSERT( old_gpfn != SHARED_M2P_ENTRY );
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         ASSERT( old_gpfn == gfn );
     if ( old_gpfn != INVALID_M2P_ENTRY )
-        guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
+        rc = guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
 
     /* Map at new location. */
-    rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
+    if ( !rc )
+        rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
 
+ put_both:
     /* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         put_gfn(d, gfn);
Index: xen-4.4.4-testing/xen/arch/x86/mm/p2m.c
===================================================================
--- xen-4.4.4-testing.orig/xen/arch/x86/mm/p2m.c
+++ xen-4.4.4-testing/xen/arch/x86/mm/p2m.c
@@ -504,7 +504,7 @@ void p2m_final_teardown(struct domain *d
 }
 
 
-static void
+static int
 p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn, unsigned long mfn,
                 unsigned int page_order)
 {
@@ -518,7 +518,7 @@ p2m_remove_page(struct p2m_domain *p2m,
         if ( need_iommu(p2m->domain) )
             for ( i = 0; i < (1 << page_order); i++ )
                 iommu_unmap_page(p2m->domain, mfn + i);
-        return;
+        return 0;
     }
 
     ASSERT(gfn_locked_by_me(p2m, gfn));
@@ -534,17 +534,22 @@ p2m_remove_page(struct p2m_domain *p2m,
             ASSERT( !p2m_is_valid(t) || mfn + i == mfn_x(mfn_return) );
         }
     }
-    set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), page_order, p2m_invalid, p2m->default_access);
+    return set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), page_order,
+                         p2m_invalid, p2m->default_access) ? 0 : -ENOMEM;
 }
 
-void
+int
 guest_physmap_remove_page(struct domain *d, unsigned long gfn,
                           unsigned long mfn, unsigned int page_order)
 {
+    int rc;
+
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
     gfn_lock(p2m, gfn, page_order);
-    p2m_remove_page(p2m, gfn, mfn, page_order);
+    rc = p2m_remove_page(p2m, gfn, mfn, page_order);
     gfn_unlock(p2m, gfn, page_order);
+
+    return rc;
 }
 
 int
Index: xen-4.4.4-testing/xen/common/grant_table.c
===================================================================
--- xen-4.4.4-testing.orig/xen/common/grant_table.c
+++ xen-4.4.4-testing/xen/common/grant_table.c
@@ -1529,6 +1529,7 @@ gnttab_transfer(
     for ( i = 0; i < count; i++ )
     {
         bool_t okay;
+        int rc;
 
         if (i && hypercall_preempt_check())
             return i;
@@ -1579,27 +1580,33 @@ gnttab_transfer(
             goto copyback;
         }
 
-        guest_physmap_remove_page(d, gop.mfn, mfn, 0);
+        rc = guest_physmap_remove_page(d, gop.mfn, mfn, 0);
         flush_tlb_mask(d->domain_dirty_cpumask);
+        if ( rc )
+        {
+            gdprintk(XENLOG_INFO,
+                     "gnttab_transfer: can't remove GFN %"PRI_xen_pfn" (MFN %lx)\n",
+                     gop.mfn, mfn);
+            gop.status = GNTST_general_error;
+            goto put_gfn_and_copyback;
+        }
 
         /* Find the target domain. */
         if ( unlikely((e = rcu_lock_domain_by_id(gop.domid)) == NULL) )
         {
-            put_gfn(d, gop.mfn);
             gdprintk(XENLOG_INFO, "gnttab_transfer: can't find domain %d\n",
                     gop.domid);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_bad_domain;
-            goto copyback;
+            goto put_gfn_and_copyback;
         }
 
         if ( xsm_grant_transfer(XSM_HOOK, d, e) )
         {
-            put_gfn(d, gop.mfn);
             gop.status = GNTST_permission_denied;
         unlock_and_copyback:
             rcu_unlock_domain(e);
+        put_gfn_and_copyback:
+            put_gfn(d, gop.mfn);
             page->count_info &= ~(PGC_count_mask|PGC_allocated);
             free_domheap_page(page);
             goto copyback;
@@ -1650,12 +1657,8 @@ gnttab_transfer(
                          "Transferee (d%d) has no headroom (tot %u, max %u)\n",
                          e->domain_id, e->tot_pages, e->max_pages);
 
-            rcu_unlock_domain(e);
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         /* Okay, add the page to 'e'. */
@@ -1684,13 +1687,8 @@ gnttab_transfer(
 
             if ( drop_dom_ref )
                 put_domain(e);
-            rcu_unlock_domain(e);
-
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         page_list_add_tail(page, &e->page_list);
Index: xen-4.4.4-testing/xen/common/memory.c
===================================================================
--- xen-4.4.4-testing.orig/xen/common/memory.c
+++ xen-4.4.4-testing/xen/common/memory.c
@@ -249,8 +249,12 @@ int guest_remove_page(struct domain *d,
     mfn = mfn_x(get_gfn_query(d, gmfn, &p2mt)); 
     if ( unlikely(p2m_is_paging(p2mt)) )
     {
-        guest_physmap_remove_page(d, gmfn, mfn, 0);
+        rc = guest_physmap_remove_page(d, gmfn, mfn, 0);
         put_gfn(d, gmfn);
+
+        if ( rc )
+            return rc;
+
         /* If the page hasn't yet been paged out, there is an
          * actual page that needs to be released. */
         if ( p2mt == p2m_ram_paging_out )
@@ -307,18 +311,18 @@ int guest_remove_page(struct domain *d,
         return -ENXIO;
     }
 
-    if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
+    rc = guest_physmap_remove_page(d, gmfn, mfn, 0);
+
+    if ( !rc && test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
         put_page_and_type(page);
             
-    if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
+    if ( !rc && test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
-    guest_physmap_remove_page(d, gmfn, mfn, 0);
-
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 0;
+    return rc;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -559,7 +563,8 @@ static long memory_exchange(XEN_GUEST_HA
             gfn = mfn_to_gmfn(d, mfn);
             /* Pages were unshared above */
             BUG_ON(SHARED_M2P(gfn));
-            guest_physmap_remove_page(d, gfn, mfn, 0);
+            if ( guest_physmap_remove_page(d, gfn, mfn, 0) )
+                domain_crash(d);
             put_page(page);
         }
 
@@ -1004,7 +1009,7 @@ long do_memory_op(unsigned long cmd, XEN
         page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
         if ( page )
         {
-            guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
+            rc = guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
             put_page(page);
         }
         else
Index: xen-4.4.4-testing/xen/include/asm-arm/p2m.h
===================================================================
--- xen-4.4.4-testing.orig/xen/include/asm-arm/p2m.h
+++ xen-4.4.4-testing/xen/include/asm-arm/p2m.h
@@ -108,10 +108,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gpfn,
-                               unsigned long mfn, unsigned int page_order);
-
 unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
 
 /*
Index: xen-4.4.4-testing/xen/include/asm-x86/p2m.h
===================================================================
--- xen-4.4.4-testing.orig/xen/include/asm-x86/p2m.h
+++ xen-4.4.4-testing/xen/include/asm-x86/p2m.h
@@ -501,11 +501,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-/* Remove a page from a domain's p2m table */
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gfn,
-                               unsigned long mfn, unsigned int page_order);
-
 /* Set a p2m range as populate-on-demand */
 int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn,
                                           unsigned int order);
Index: xen-4.4.4-testing/xen/include/xen/mm.h
===================================================================
--- xen-4.4.4-testing.orig/xen/include/xen/mm.h
+++ xen-4.4.4-testing/xen/include/xen/mm.h
@@ -361,7 +361,12 @@ int xenmem_add_to_physmap_one(struct dom
                               unsigned long idx, xen_pfn_t gpfn);
 
 /* Returns 0 on success, or negative on error. */
-int guest_remove_page(struct domain *d, unsigned long gmfn);
+int __must_check guest_remove_page(struct domain *d, unsigned long gmfn);
+/* Remove a page from a domain's p2m table */
+int __must_check
+guest_physmap_remove_page(struct domain *d, unsigned long gfn,
+                          unsigned long mfn, unsigned int page_order);
+
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001
 #define RAM_TYPE_RESERVED     0x00000002

Places

File xsa222-2.patch of Package xen.5015

Places