From: Jan Beulich Subject: x86/shadow: fix ref-counting error handling The old-Linux handling in shadow_set_l4e() mistakenly ORed together the results of sh_get_ref() and sh_pin(). As the latter failing is not a correctness problem, simply ignore its return value. In sh_set_toplevel_shadow() a failing sh_get_ref() must not be accompanied by installing the entry, despite the domain being crashed. This is XSA-250. Signed-off-by: Jan Beulich Reviewed-by: Tim Deegan --- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -923,7 +923,7 @@ static int shadow_set_l4e(struct domain shadow_l4e_t new_sl4e, mfn_t sl4mfn) { - int flags = 0, ok; + int flags = 0; shadow_l4e_t old_sl4e; paddr_t paddr; ASSERT(sl4e != NULL); @@ -938,15 +938,16 @@ static int shadow_set_l4e(struct domain { /* About to install a new reference */ mfn_t sl3mfn = shadow_l4e_get_mfn(new_sl4e); - ok = sh_get_ref(d, sl3mfn, paddr); - /* Are we pinning l3 shadows to handle wierd linux behaviour? */ - if ( sh_type_is_pinnable(d, SH_type_l3_64_shadow) ) - ok |= sh_pin(d, sl3mfn); - if ( !ok ) + + if ( !sh_get_ref(d, sl3mfn, paddr) ) { domain_crash(d); return SHADOW_SET_ERROR; } + + /* Are we pinning l3 shadows to handle weird Linux behaviour? */ + if ( sh_type_is_pinnable(d, SH_type_l3_64_shadow) ) + sh_pin(d, sl3mfn); } /* Write the new entry */ @@ -3965,14 +3966,15 @@ sh_set_toplevel_shadow(struct vcpu *v, /* Take a ref to this page: it will be released in sh_detach_old_tables() * or the next call to set_toplevel_shadow() */ - if ( !sh_get_ref(d, smfn, 0) ) + if ( sh_get_ref(d, smfn, 0) ) + new_entry = pagetable_from_mfn(smfn); + else { SHADOW_ERROR("can't install %#lx as toplevel shadow\n", mfn_x(smfn)); domain_crash(d); + new_entry = pagetable_null(); } - new_entry = pagetable_from_mfn(smfn); - install_new_entry: /* Done. Install it */ SHADOW_PRINTK("%u/%u [%u] gmfn %#"PRI_mfn" smfn %#"PRI_mfn"\n",