[Xen-devel] [PATCH] ept: Put locks around ept_get_entry (UPDATED)

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

There's a subtle race in ept_get_entry, such that if tries to read an
entry that ept_set_entry is modifying, it gets neither the old entry
nor the new entry, but empty.  In the case of multi-cpu populate-on-demand
guests, this manifests as a guest crash when one vcpu tries to read a
page which another page is trying to populate, and ept_get_entry returns
p2m_mmio_dm.

This bug can also be fixed by making both ept_set_entry and ept_next_level
access-once (i.e., ept_next_level reads full ept_entry and then works with
local value; ept_set_entry construct the entry locally and then sets it
in one write).  But there doesn't seem to be any major performance implications
of just making ept_get_entry use locks; so the simpler, the better.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

diff -r 3c4c3d48a835 -r 600bbe524a66 xen/arch/x86/mm/hap/p2m-ept.c
--- a/xen/arch/x86/mm/hap/p2m-ept.c     Thu Aug 26 11:16:56 2010 +0100
+++ b/xen/arch/x86/mm/hap/p2m-ept.c     Fri Aug 27 12:58:29 2010 +0100
@@ -431,6 +431,10 @@
     int i;
     int ret = 0;
     mfn_t mfn = _mfn(INVALID_MFN);
+    int do_locking = !p2m_locked_by_me(p2m);
+
+    if ( do_locking )
+        p2m_lock(p2m);
 
     *t = p2m_mmio_dm;
 
@@ -507,6 +511,8 @@
     }
 
 out:
+    if ( do_locking )
+        p2m_unlock(p2m);
     unmap_domain_page(table);
     return mfn;
 }

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel