[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen stable-4.8] x86/vtx: Fix the checking for unknown/invalid MSR_DEBUGCTL bits
commit ed6fcdb902434096d56a1bef46c37befa1cd643d Author: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> AuthorDate: Mon Jun 18 16:12:39 2018 +0800 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Tue Aug 14 17:27:26 2018 +0100 x86/vtx: Fix the checking for unknown/invalid MSR_DEBUGCTL bits The VPMU_MODE_OFF early-exit in vpmu_do_wrmsr() introduced by c/s 11fe998e56 bypasses all reserved bit checking in the general case. As a result, a guest can enable BTS when it shouldn't be permitted to, and lock up the entire host. With vPMU active (not a security supported configuration, but useful for debugging), the reserved bit checking in broken, caused by the original BTS changeset 1a8aa75ed. From a correctness standpoint, it is not possible to have two different pieces of code responsible for different parts of value checking, if there isn't an accumulation of bits which have been checked. A practical upshot of this is that a guest can set any value it wishes (usually resulting in a vmentry failure for bad guest state). Therefore, fix this by implementing all the reserved bit checking in the main MSR_DEBUGCTL block, and removing all handling of DEBUGCTL from the vPMU MSR logic. This is XSA-269. Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> (cherry picked from commit 2a8a8e99feb950504559196521bc9fd63ed3a962) --- xen/arch/x86/cpu/vpmu_intel.c | 24 ------------------------ xen/arch/x86/hvm/vmx/vmx.c | 27 ++++++++++++++++++++------- xen/include/asm-x86/vpmu.h | 4 ++++ 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c index f189c00b7a..72fbead0d6 100644 --- a/xen/arch/x86/cpu/vpmu_intel.c +++ b/xen/arch/x86/cpu/vpmu_intel.c @@ -68,10 +68,6 @@ #define MSR_PMC_ALIAS_MASK (~(MSR_IA32_PERFCTR0 ^ MSR_IA32_A_PERFCTR0)) static bool_t __read_mostly full_width_write; -/* Intel-specific VPMU features */ -#define VPMU_CPU_HAS_DS 0x100 /* Has Debug Store */ -#define VPMU_CPU_HAS_BTS 0x200 /* Has Branch Trace Store */ - /* * MSR_CORE_PERF_FIXED_CTR_CTRL contains the configuration of all fixed * counters. 4 bits for every counter. @@ -561,27 +557,7 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t *enabled_cntrs; if ( !core2_vpmu_msr_common_check(msr, &type, &index) ) - { - /* Special handling for BTS */ - if ( msr == MSR_IA32_DEBUGCTLMSR ) - { - supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS | - IA32_DEBUGCTLMSR_BTINT; - - if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) ) - supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS | - IA32_DEBUGCTLMSR_BTS_OFF_USR; - if ( !(msr_content & ~supported) && - vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) ) - return 0; - if ( (msr_content & supported) && - !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) ) - printk(XENLOG_G_WARNING - "%pv: Debug Store unsupported on this CPU\n", - current); - } return -EINVAL; - } ASSERT(!supported); diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 126483d73d..2cac5b7eb0 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -2984,6 +2984,8 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content) switch ( msr ) { + uint64_t rsvd; + case MSR_IA32_SYSENTER_CS: __vmwrite(GUEST_SYSENTER_CS, msr_content); break; @@ -2998,17 +3000,28 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content) __vmwrite(GUEST_SYSENTER_EIP, msr_content); break; case MSR_IA32_DEBUGCTLMSR: { - uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF; + uint32_t ebx, ecx = 0; + + rsvd = ~(IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF); - if ( boot_cpu_has(X86_FEATURE_RTM) ) - supported |= IA32_DEBUGCTLMSR_RTM; - if ( msr_content & ~supported ) + /* TODO: Wire vPMU settings properly through the CPUID policy */ + if ( vpmu_is_set(vcpu_vpmu(v), VPMU_CPU_HAS_BTS) ) { - /* Perhaps some other bits are supported in vpmu. */ - if ( vpmu_do_wrmsr(msr, msr_content, supported) ) - break; + rsvd &= ~(IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS | + IA32_DEBUGCTLMSR_BTINT); + + if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) ) + rsvd &= ~(IA32_DEBUGCTLMSR_BTS_OFF_OS | + IA32_DEBUGCTLMSR_BTS_OFF_USR); } + hvm_cpuid(7, NULL, &ebx, &ecx, NULL); + if ( ebx & cpufeat_mask(X86_FEATURE_RTM) ) + rsvd &= ~IA32_DEBUGCTLMSR_RTM; + + if ( msr_content & rsvd ) + goto gp_fault; + /* * When a guest first enables LBR, arrange to save and restore the LBR * MSRs and allow the guest direct access. diff --git a/xen/include/asm-x86/vpmu.h b/xen/include/asm-x86/vpmu.h index ed9ec077f3..75b1973859 100644 --- a/xen/include/asm-x86/vpmu.h +++ b/xen/include/asm-x86/vpmu.h @@ -77,6 +77,10 @@ struct vpmu_struct { /* PV(H) guests: VPMU registers are accessed by guest from shared page */ #define VPMU_CACHED 0x40 +/* Intel-specific VPMU features */ +#define VPMU_CPU_HAS_DS 0x100 /* Has Debug Store */ +#define VPMU_CPU_HAS_BTS 0x200 /* Has Branch Trace Store */ + static inline void vpmu_set(struct vpmu_struct *vpmu, const u32 mask) { vpmu->flags |= mask; -- generated by git-patchbot for /home/xen/git/xen.git#stable-4.8 _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/xen-changelog
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |