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.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Wei Liu <wei.l...@citrix.com>
CC: Roger Pau Monné <roger....@citrix.com>
CC: Razvan Cojocaru <rcojoc...@bitdefender.com>
CC: Tamas K Lengyel <ta...@tklengyel.com>
CC: George Dunlap <george.dun...@eu.citrix.com>
---
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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
