x86/shadow: hold references for the duration of emulated writes

The (misnamed) emulate_gva_to_mfn() function translates a linear address to an
mfn, but releases its page reference before returning the mfn to its caller.

sh_emulate_map_dest() uses the results of one or two translations to construct
a virtual mapping to the underlying frames, completes an emulated
write/cmpxchg, then unmaps the virtual mappings.

The page references need holding until the mappings are unmapped, or the
frames can change ownership before the writes occurs.

This is XSA-219.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
master commit: 26217aff67ae1538d4e1b2226afab6993cdbe772
master date: 2017-06-20 14:36:11 +0200

diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 7589d23eaa1bf6eb7165a0bbf450e1861f979264..e16bf1b4923dd291651da06500ceda32a20732fd 100644 (file)
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -4555,7 +4555,10 @@ static void sh_pagetable_dying(struct vcpu *v, paddr_t gpa)
 /**************************************************************************/
 /* Handling HVM guest writes to pagetables  */
 
-/* Translate a VA to an MFN, injecting a page-fault if we fail */
+/*
+ * Translate a VA to an MFN, injecting a page-fault if we fail.  If the
+ * mapping succeeds, a reference will be held on the underlying page.
+ */
 #define BAD_GVA_TO_GFN (~0UL)
 #define BAD_GFN_TO_MFN (~1UL)
 #define READONLY_GFN   (~2UL)
@@ -4604,14 +4607,15 @@ static mfn_t emulate_gva_to_mfn(struct vcpu *v,
     ASSERT(mfn_valid(mfn));
 
     v->arch.paging.last_write_was_pt = !!sh_mfn_is_a_page_table(mfn);
-    /* Note shadow cannot page out or unshare this mfn, so the map won't
-     * disappear. Otherwise, caller must hold onto page until done. */
-    put_page(page);
+
     return mfn;
 }
 
-/* Check that the user is allowed to perform this write. 
- * Returns a mapped pointer to write to, or NULL for error. */
+/*
+ * Check that the user is allowed to perform this write.  If a mapping is
+ * returned, page references will be held on sh_ctxt->mfn1 and
+ * sh_ctxt->mfn2 iff !INVALID_MFN.
+ */
 #define MAPPING_UNHANDLEABLE ((void *)(unsigned long)X86EMUL_UNHANDLEABLE)
 #define MAPPING_EXCEPTION    ((void *)(unsigned long)X86EMUL_EXCEPTION)
 #define MAPPING_SILENT_FAIL  ((void *)(unsigned long)X86EMUL_OKAY)
@@ -4623,13 +4627,6 @@ static void *emulate_map_dest(struct vcpu *v,
 {
     void *map = NULL;
 
-    sh_ctxt->mfn1 = emulate_gva_to_mfn(v, vaddr, sh_ctxt);
-    if ( !mfn_valid(sh_ctxt->mfn1) ) 
-        return ((mfn_x(sh_ctxt->mfn1) == BAD_GVA_TO_GFN) ?
-                MAPPING_EXCEPTION :
-                (mfn_x(sh_ctxt->mfn1) == READONLY_GFN) ?
-                MAPPING_SILENT_FAIL : MAPPING_UNHANDLEABLE);
-
 #ifndef NDEBUG
     /* We don't emulate user-mode writes to page tables */
     if ( hvm_get_seg_reg(x86_seg_ss, sh_ctxt)->attr.fields.dpl == 3 )
@@ -4640,6 +4637,17 @@ static void *emulate_map_dest(struct vcpu *v,
     }
 #endif
                 
+    sh_ctxt->mfn1 = emulate_gva_to_mfn(v, vaddr, sh_ctxt);
+    if ( !mfn_valid(sh_ctxt->mfn1) )
+    {
+        switch ( mfn_x(sh_ctxt->mfn1) )
+        {
+        case BAD_GVA_TO_GFN: return MAPPING_EXCEPTION;
+        case READONLY_GFN:   return MAPPING_SILENT_FAIL;
+        default:             return MAPPING_UNHANDLEABLE;
+        }
+    }
+
     /* Unaligned writes mean probably this isn't a pagetable */
     if ( vaddr & (bytes - 1) )
         sh_remove_shadows(v, sh_ctxt->mfn1, 0, 0 /* Slow, can fail */ );
@@ -4657,16 +4665,24 @@ static void *emulate_map_dest(struct vcpu *v,
         /* Cross-page emulated writes are only supported for HVM guests; 
          * PV guests ought to know better */
         if ( !is_hvm_vcpu(v) )
+        {
+            put_page(mfn_to_page(sh_ctxt->mfn1));
             return MAPPING_UNHANDLEABLE;
+        }
 
         /* This write crosses a page boundary.  Translate the second page */
         sh_ctxt->mfn2 = emulate_gva_to_mfn(v, (vaddr + bytes - 1) & PAGE_MASK,
                                            sh_ctxt);
-        if ( !mfn_valid(sh_ctxt->mfn2) ) 
-            return ((mfn_x(sh_ctxt->mfn2) == BAD_GVA_TO_GFN) ?
-                    MAPPING_EXCEPTION :
-                    (mfn_x(sh_ctxt->mfn2) == READONLY_GFN) ?
-                    MAPPING_SILENT_FAIL : MAPPING_UNHANDLEABLE);
+        if ( !mfn_valid(sh_ctxt->mfn2) )
+        {
+            put_page(mfn_to_page(sh_ctxt->mfn1));
+            switch ( mfn_x(sh_ctxt->mfn2) )
+            {
+            case BAD_GVA_TO_GFN: return MAPPING_EXCEPTION;
+            case READONLY_GFN:   return MAPPING_SILENT_FAIL;
+            default:             return MAPPING_UNHANDLEABLE;
+            }
+        }
 
         /* Cross-page writes mean probably not a pagetable */
         sh_remove_shadows(v, sh_ctxt->mfn2, 0, 0 /* Slow, can fail */ );
@@ -4675,7 +4691,11 @@ static void *emulate_map_dest(struct vcpu *v,
         mfns[1] = mfn_x(sh_ctxt->mfn2);
         map = vmap(mfns, 2);
         if ( !map )
+        {
+            put_page(mfn_to_page(sh_ctxt->mfn1));
+            put_page(mfn_to_page(sh_ctxt->mfn2));
             return MAPPING_UNHANDLEABLE;
+        }
         map += (vaddr & ~PAGE_MASK);
     }
 
@@ -4750,10 +4770,12 @@ static void emulate_unmap_dest(struct vcpu *v,
     }
 
     paging_mark_dirty(v->domain, mfn_x(sh_ctxt->mfn1));
+    put_page(mfn_to_page(sh_ctxt->mfn1));
 
     if ( unlikely(mfn_valid(sh_ctxt->mfn2)) )
     {
         paging_mark_dirty(v->domain, mfn_x(sh_ctxt->mfn2));
+        put_page(mfn_to_page(sh_ctxt->mfn2));
         vunmap((void *)((unsigned long)addr & PAGE_MASK));
     }
     else