# HG changeset patch # User Keir Fraser # Date 1283153992 -3600 # Node ID c5aed2e049bce2724b035dd6aa09c4c4e609c27c # Parent 20920c12bc4815b1f755786c0924393809664807 ept: Put locks around ept_get_entry 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 --- a/xen/arch/x86/mm/hap/p2m-ept.c +++ b/xen/arch/x86/mm/hap/p2m-ept.c @@ -387,6 +387,10 @@ static mfn_t ept_get_entry(struct domain int i; int ret = 0; mfn_t mfn = _mfn(INVALID_MFN); + int do_locking = !p2m_locked_by_me(d->arch.p2m); + + if ( do_locking ) + p2m_lock(d->arch.p2m); *t = p2m_mmio_dm; @@ -464,6 +468,8 @@ static mfn_t ept_get_entry(struct domain } out: + if ( do_locking ) + p2m_unlock(d->arch.p2m); unmap_domain_page(table); return mfn; }