[Xen-devel] [PATCH 04/14] x86/p2m: Fix locking in p2m_altp2m_lazy_copy()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The gfn references need to remain held until after the p2m_set_entry() has
completed.  This is only a latent bug for now, because there is no per-gfn
locking and we recursively hold the main p2m locks.

Rearrange the code to have a single exit path, and defer taking the ap2m lock
until it is necessary to do so.  Leave some comments behind to help people
attempting to follow the logic.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
CC: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
---
 xen/arch/x86/mm/p2m.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index b5a59d6..ae9cb20 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2184,24 +2184,29 @@ bool_t p2m_altp2m_lazy_copy(struct vcpu *v, paddr_t gpa,
     unsigned long mask;
     mfn_t mfn;
     int rv;
+    bool ret;
 
     *ap2m = p2m_get_altp2m(v);
 
     mfn = get_gfn_type_access(*ap2m, gfn_x(gfn), &p2mt, &p2ma,
                               0, &page_order);
-    __put_gfn(*ap2m, gfn_x(gfn));
 
+    /* Entry already present in ap2m?  Caller should handle the fault. */
     if ( !mfn_eq(mfn, INVALID_MFN) )
-        return 0;
+    {
+        ret = false;
+        goto put_ap2m;
+    }
 
     mfn = get_gfn_type_access(hp2m, gfn_x(gfn), &p2mt, &p2ma,
                               P2M_ALLOC, &page_order);
-    __put_gfn(hp2m, gfn_x(gfn));
 
+    /* Entry not present in hp2m?  Caller should handle the fault. */
     if ( mfn_eq(mfn, INVALID_MFN) )
-        return 0;
-
-    p2m_lock(*ap2m);
+    {
+        ret = false;
+        goto put_hp2m;
+    }
 
     /*
      * If this is a superpage mapping, round down both frame numbers
@@ -2211,6 +2216,7 @@ bool_t p2m_altp2m_lazy_copy(struct vcpu *v, paddr_t gpa,
     mfn = _mfn(mfn_x(mfn) & mask);
     gfn = _gfn(gfn_x(gfn) & mask);
 
+    p2m_lock(*ap2m);
     rv = p2m_set_entry(*ap2m, gfn, mfn, page_order, p2mt, p2ma);
     p2m_unlock(*ap2m);
 
@@ -2222,7 +2228,14 @@ bool_t p2m_altp2m_lazy_copy(struct vcpu *v, paddr_t gpa,
         domain_crash(hp2m->domain);
     }
 
-    return 1;
+    ret = true;
+
+put_hp2m:
+    __put_gfn(hp2m, gfn_x(gfn));
+put_ap2m:
+    __put_gfn(*ap2m, gfn_x(gfn));
+
+    return ret;
 }
 
 void p2m_flush_altp2m(struct domain *d)
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel