[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] x86/shadow: Correct the behaviour of a guests view of maxphysaddr
XSA-173 (c/s 8b1764833) introduces gfn_bits, and an upper limit which might be lower than the real maxphysaddr, to avoid overflowing the superpage shadow backpointer. However, plenty of hardware has a physical address width less that 44 bits, and the code added in shadow_domain_init() is a straight asignment. This causes gfn_bits to be increased beyond the physical address width on most Intel consumer hardware (which has a width of 39). It also means that the effective maxphysaddr for shadowed guests differs from the value reported to the guest in CPUID. This means that a guest can create PTEs which either should fault but don't, or shouldn't fault but do. Remove gfn_bits and rework its logic in terms of a guests maxphysaddr. recalculate_cpuid_policy() is updated to properly clamp maxphysaddr between the host maximum, a possibly-smaller shadow restriction, and 32 as a minimum. Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> --- CC: Jan Beulich <JBeulich@xxxxxxxx> CC: Tim Deegan <tim@xxxxxxx> CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx> CC: Kevin Tian <kevin.tian@xxxxxxxxx> --- xen/arch/x86/cpuid.c | 20 ++++++++++++++------ xen/arch/x86/hvm/vmx/vvmx.c | 2 +- xen/arch/x86/mm/guest_walk.c | 3 ++- xen/arch/x86/mm/hap/hap.c | 2 -- xen/arch/x86/mm/p2m.c | 3 ++- xen/arch/x86/mm/shadow/common.c | 10 ---------- xen/arch/x86/mm/shadow/multi.c | 3 ++- xen/include/asm-x86/domain.h | 3 --- 8 files changed, 21 insertions(+), 25 deletions(-) diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c index e0a387e..02a3451 100644 --- a/xen/arch/x86/cpuid.c +++ b/xen/arch/x86/cpuid.c @@ -1,4 +1,5 @@ #include <xen/init.h> +#include <xen/kconfig.h> #include <xen/lib.h> #include <xen/sched.h> #include <asm/cpuid.h> @@ -6,6 +7,7 @@ #include <asm/hvm/nestedhvm.h> #include <asm/hvm/svm/svm.h> #include <asm/hvm/vmx/vmcs.h> +#include <asm/paging.h> #include <asm/processor.h> #include <asm/xstate.h> @@ -424,7 +426,7 @@ void recalculate_cpuid_policy(struct domain *d) const struct cpuid_policy *max = is_pv_domain(d) ? &pv_max_policy : &hvm_max_policy; uint32_t fs[FSCAPINTS], max_fs[FSCAPINTS]; - unsigned int i; + unsigned int i, maxphysaddr_limit; p->x86_vendor = get_cpu_vendor(p->basic.vendor_ebx, p->basic.vendor_ecx, p->basic.vendor_edx, gcv_guest); @@ -502,11 +504,17 @@ void recalculate_cpuid_policy(struct domain *d) cpuid_featureset_to_policy(fs, p); - p->extd.maxphysaddr = min(p->extd.maxphysaddr, max->extd.maxphysaddr); - p->extd.maxphysaddr = min_t(uint8_t, p->extd.maxphysaddr, - d->arch.paging.gfn_bits + PAGE_SHIFT); - p->extd.maxphysaddr = max_t(uint8_t, p->extd.maxphysaddr, - (p->basic.pae || p->basic.pse36) ? 36 : 32); + maxphysaddr_limit = paging_mode_hap(d) ? hap_paddr_bits : paddr_bits; + + if ( !IS_ENABLED(BIGMEM) && paging_mode_shadow(d) && + (!is_pv_domain(d) || opt_allow_superpage) ) + { + /* Shadowed superpages store GFNs in 32-bit page_info fields. */ + maxphysaddr_limit = min(maxphysaddr_limit, 32U + PAGE_SHIFT); + } + + p->extd.maxphysaddr = min_t(uint8_t, p->extd.maxphysaddr, maxphysaddr_limit); + p->extd.maxphysaddr = max_t(uint8_t, p->extd.maxphysaddr, 32); p->extd.maxlinaddr = p->extd.lm ? 48 : 32; diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index 5acb88a..4ea8a6b 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -1380,7 +1380,7 @@ int nvmx_handle_vmxon(struct cpu_user_regs *regs) } if ( (gpa & ~PAGE_MASK) || - (gpa >> (v->domain->arch.paging.gfn_bits + PAGE_SHIFT)) ) + (gpa >> v->domain->arch.cpuid->extd.maxphysaddr) ) { vmfail_invalid(regs); return X86EMUL_OKAY; diff --git a/xen/arch/x86/mm/guest_walk.c b/xen/arch/x86/mm/guest_walk.c index a67fd5a..5ad8cf6 100644 --- a/xen/arch/x86/mm/guest_walk.c +++ b/xen/arch/x86/mm/guest_walk.c @@ -435,7 +435,8 @@ guest_walk_tables(struct vcpu *v, struct p2m_domain *p2m, /* If this guest has a restricted physical address space then the * target GFN must fit within it. */ if ( !(rc & _PAGE_PRESENT) - && gfn_x(guest_l1e_get_gfn(gw->l1e)) >> d->arch.paging.gfn_bits ) + && gfn_x(guest_l1e_get_gfn(gw->l1e)) >> + (d->arch.cpuid->extd.maxphysaddr - PAGE_SHIFT) ) rc |= _PAGE_INVALID_BITS; return rc; diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c index 6dbb3cc..928cd5e 100644 --- a/xen/arch/x86/mm/hap/hap.c +++ b/xen/arch/x86/mm/hap/hap.c @@ -447,8 +447,6 @@ void hap_domain_init(struct domain *d) { INIT_PAGE_LIST_HEAD(&d->arch.paging.hap.freelist); - d->arch.paging.gfn_bits = hap_paddr_bits - PAGE_SHIFT; - /* Use HAP logdirty mechanism. */ paging_log_dirty_init(d, hap_enable_log_dirty, hap_disable_log_dirty, diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 6548e9f..fc706c6 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1790,7 +1790,8 @@ void *map_domain_gfn(struct p2m_domain *p2m, gfn_t gfn, mfn_t *mfn, { struct page_info *page; - if ( gfn_x(gfn) >> p2m->domain->arch.paging.gfn_bits ) + if ( gfn_x(gfn) >> + (p2m->domain->arch.cpuid->extd.maxphysaddr - PAGE_SHIFT) ) { *rc = _PAGE_INVALID_BIT; return NULL; diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c index a619d65..2235a0a 100644 --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -52,16 +52,6 @@ int shadow_domain_init(struct domain *d, unsigned int domcr_flags) INIT_PAGE_LIST_HEAD(&d->arch.paging.shadow.freelist); INIT_PAGE_LIST_HEAD(&d->arch.paging.shadow.pinned_shadows); - d->arch.paging.gfn_bits = paddr_bits - PAGE_SHIFT; -#ifndef CONFIG_BIGMEM - /* - * Shadowed superpages store GFNs in 32-bit page_info fields. - * Note that we cannot use guest_supports_superpages() here. - */ - if ( !is_pv_domain(d) || opt_allow_superpage ) - d->arch.paging.gfn_bits = 32; -#endif - /* Use shadow pagetables for log-dirty support */ paging_log_dirty_init(d, sh_enable_log_dirty, sh_disable_log_dirty, sh_clean_dirty_bitmap); diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c index d4090d7..e951daf 100644 --- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -537,7 +537,8 @@ _sh_propagate(struct vcpu *v, /* Check there's something for the shadows to map to */ if ( (!p2m_is_valid(p2mt) && !p2m_is_grant(p2mt)) - || gfn_x(target_gfn) >> d->arch.paging.gfn_bits ) + || gfn_x(target_gfn) >> + (d->arch.cpuid->extd.maxphysaddr - PAGE_SHIFT) ) { *sp = shadow_l1e_empty(); goto done; diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index e6c7e13..2270e96 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -195,9 +195,6 @@ struct paging_domain { /* log dirty support */ struct log_dirty_domain log_dirty; - /* Number of valid bits in a gfn. */ - unsigned int gfn_bits; - /* preemption handling */ struct { const struct domain *dom; -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |