diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c index 519b80cc3d..b1f0abe02d 100644 --- a/xen/arch/x86/mm/p2m-pod.c +++ b/xen/arch/x86/mm/p2m-pod.c @@ -729,8 +729,9 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, unsigned long gfn) } /* Try to remove the page, restoring old mapping if it fails. */ - p2m_set_entry(p2m, gfn, _mfn(INVALID_MFN), PAGE_ORDER_2M, - p2m_populate_on_demand, p2m->default_access); + if ( p2m_set_entry(p2m, gfn, _mfn(INVALID_MFN), PAGE_ORDER_2M, + p2m_populate_on_demand, p2m->default_access) ) + goto out; /* Make none of the MFNs are used elsewhere... for example, mapped * via the grant table interface, or by qemu. Allow one refcount for @@ -786,9 +787,18 @@ p2m_pod_zero_check_superpage(struct p2m_domain *p2m, unsigned long gfn) ret = SUPERPAGE_PAGES; out_reset: - if ( reset ) - p2m_set_entry(p2m, gfn, mfn0, 9, type0, p2m->default_access); - + /* + * This p2m_set_entry() call shouldn't be able to fail, since the same order + * on the same gfn succeeded above. If that turns out to be false, crashing + * the domain should be the safest way of making sure we don't leak memory. + */ + if ( reset && p2m_set_entry(p2m, gfn, mfn0, PAGE_ORDER_2M, + type0, p2m->default_access) ) + { + ASSERT_UNREACHABLE(); + domain_crash(d); + } + out: gfn_unlock(p2m, gfn, SUPERPAGE_ORDER); return ret; @@ -845,19 +855,30 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long *gfns, int count) } /* Try to remove the page, restoring old mapping if it fails. */ - p2m_set_entry(p2m, gfns[i], _mfn(INVALID_MFN), PAGE_ORDER_4K, - p2m_populate_on_demand, p2m->default_access); + if ( p2m_set_entry(p2m, gfns[i], _mfn(INVALID_MFN), PAGE_ORDER_4K, + p2m_populate_on_demand, p2m->default_access) ) + goto skip; /* See if the page was successfully unmapped. (Allow one refcount * for being allocated to a domain.) */ if ( (mfn_to_page(mfns[i])->count_info & PGC_count_mask) > 1 ) { + /* + * If the previous p2m_set_entry call succeeded, this one shouldn't + * be able to fail. If it does, crashing the domain should be safe. + */ + if ( p2m_set_entry(p2m, gfns[i], mfns[i], PAGE_ORDER_4K, + types[i], p2m->default_access) ) + { + ASSERT_UNREACHABLE(); + domain_crash(d); + goto out_unmap; + } + + skip: unmap_domain_page(map[i]); map[i] = NULL; - p2m_set_entry(p2m, gfns[i], mfns[i], PAGE_ORDER_4K, - types[i], p2m->default_access); - continue; } } @@ -874,12 +895,25 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long *gfns, int count) unmap_domain_page(map[i]); - /* See comment in p2m_pod_zero_check_superpage() re gnttab - * check timing. */ - if ( j < PAGE_SIZE/sizeof(*map[i]) ) + map[i] = NULL; + + /* + * See comment in p2m_pod_zero_check_superpage() re gnttab + * check timing. + */ + if ( j < (PAGE_SIZE / sizeof(*map[i])) ) { - p2m_set_entry(p2m, gfns[i], mfns[i], PAGE_ORDER_4K, - types[i], p2m->default_access); + /* + * If the previous p2m_set_entry call succeeded, this one shouldn't + * be able to fail. If it does, crashing the domain should be safe. + */ + if ( p2m_set_entry(p2m, gfns[i], mfns[i], PAGE_ORDER_4K, + types[i], p2m->default_access) ) + { + ASSERT_UNREACHABLE(); + domain_crash(d); + goto out_unmap; + } } else { @@ -903,7 +937,17 @@ p2m_pod_zero_check(struct p2m_domain *p2m, unsigned long *gfns, int count) p2m->pod.entry_count++; } } - + + return; + +out_unmap: + /* + * Something went wrong, probably crashing the domain. Unmap + * everything and return. + */ + for ( i = 0; i < count; i++ ) + if ( map[i] ) + unmap_domain_page(map[i]); } #define POD_SWEEP_LIMIT 1024