| Message ID | 20170524162255.183206-1-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2017-05-24 09:22-0700, Jim Mattson: > The BNDCFGS MSR should only be exposed to the guest if the guest > supports MPX. (cf. the TSC_AUX MSR and RDTSCP.) > > Fixes: 0dd376e709975779 ("KVM: x86: add MSR_IA32_BNDCFGS to msrs_to_save") > Change-Id: I3ad7c01bda616715137ceac878f3fa7e66b6b387 > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > @@ -144,6 +144,14 @@ static inline bool guest_cpuid_has_rtm(struct kvm_vcpu *vcpu) > return best && (best->ebx & bit(X86_FEATURE_RTM)); > } > > +static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu) > +{ > + struct kvm_cpuid_entry2 *best; > + > + best = kvm_find_cpuid_entry(vcpu, 7, 0); > + return best && (best->ebx & bit(X86_FEATURE_MPX)); > +} > + > static inline bool guest_cpuid_has_rdtscp(struct kvm_vcpu *vcpu) > { > struct kvm_cpuid_entry2 *best; > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > @@ -3195,7 +3195,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > msr_info->data = vmcs_readl(GUEST_SYSENTER_ESP); > break; > case MSR_IA32_BNDCFGS: > - if (!kvm_mpx_supported()) > + if (!guest_cpuid_has_mpx(vcpu)) > return 1; > msr_info->data = vmcs_read64(GUEST_BNDCFGS); Userspace can force guest_cpuid_has_mpx() to return true even if the host does not have MPX (GUEST_BNDCFGS in VMCS), which would allow it to trigger vmread/vmwrite errors at will. I think it would make most sense to fail KVM_SET_CPUID that tries to do that, but checking for host support or silently clearing the bit still seem better than the host error. Thanks.
On Wed, May 24, 2017 at 10:28 AM, Radim Krčmář <rkrcmar@redhat.com> wrote: > 2017-05-24 09:22-0700, Jim Mattson: >> The BNDCFGS MSR should only be exposed to the guest if the guest >> supports MPX. (cf. the TSC_AUX MSR and RDTSCP.) >> >> Fixes: 0dd376e709975779 ("KVM: x86: add MSR_IA32_BNDCFGS to msrs_to_save") >> Change-Id: I3ad7c01bda616715137ceac878f3fa7e66b6b387 >> Signed-off-by: Jim Mattson <jmattson@google.com> >> --- >> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h >> @@ -144,6 +144,14 @@ static inline bool guest_cpuid_has_rtm(struct kvm_vcpu *vcpu) >> return best && (best->ebx & bit(X86_FEATURE_RTM)); >> } >> >> +static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu) >> +{ >> + struct kvm_cpuid_entry2 *best; >> + >> + best = kvm_find_cpuid_entry(vcpu, 7, 0); >> + return best && (best->ebx & bit(X86_FEATURE_MPX)); >> +} >> + >> static inline bool guest_cpuid_has_rdtscp(struct kvm_vcpu *vcpu) >> { >> struct kvm_cpuid_entry2 *best; >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> @@ -3195,7 +3195,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> msr_info->data = vmcs_readl(GUEST_SYSENTER_ESP); >> break; >> case MSR_IA32_BNDCFGS: >> - if (!kvm_mpx_supported()) >> + if (!guest_cpuid_has_mpx(vcpu)) >> return 1; >> msr_info->data = vmcs_read64(GUEST_BNDCFGS); > > Userspace can force guest_cpuid_has_mpx() to return true even if the > host does not have MPX (GUEST_BNDCFGS in VMCS), which would allow it to > trigger vmread/vmwrite errors at will. Oops. I had wrongly assumed that the guest cpuid settings were validated. > I think it would make most sense to fail KVM_SET_CPUID that tries to do > that, but checking for host support or silently clearing the bit still > seem better than the host error. Guest cpuid settings should be validated, but I'm not going to bite that off now. Let me just do both checks.
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h index a6fd40aade7c..da6728383052 100644 --- a/arch/x86/kvm/cpuid.h +++ b/arch/x86/kvm/cpuid.h @@ -144,6 +144,14 @@ static inline bool guest_cpuid_has_rtm(struct kvm_vcpu *vcpu) return best && (best->ebx & bit(X86_FEATURE_RTM)); } +static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu) +{ + struct kvm_cpuid_entry2 *best; + + best = kvm_find_cpuid_entry(vcpu, 7, 0); + return best && (best->ebx & bit(X86_FEATURE_MPX)); +} + static inline bool guest_cpuid_has_rdtscp(struct kvm_vcpu *vcpu) { struct kvm_cpuid_entry2 *best; diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 763d27ee00fb..846c60c74258 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3195,7 +3195,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) msr_info->data = vmcs_readl(GUEST_SYSENTER_ESP); break; case MSR_IA32_BNDCFGS: - if (!kvm_mpx_supported()) + if (!guest_cpuid_has_mpx(vcpu)) return 1; msr_info->data = vmcs_read64(GUEST_BNDCFGS); break; @@ -3277,7 +3277,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) vmcs_writel(GUEST_SYSENTER_ESP, data); break; case MSR_IA32_BNDCFGS: - if (!kvm_mpx_supported()) + if (!guest_cpuid_has_mpx(vcpu)) return 1; vmcs_write64(GUEST_BNDCFGS, data); break;
The BNDCFGS MSR should only be exposed to the guest if the guest supports MPX. (cf. the TSC_AUX MSR and RDTSCP.) Fixes: 0dd376e709975779 ("KVM: x86: add MSR_IA32_BNDCFGS to msrs_to_save") Change-Id: I3ad7c01bda616715137ceac878f3fa7e66b6b387 Signed-off-by: Jim Mattson <jmattson@google.com> --- arch/x86/kvm/cpuid.h | 8 ++++++++ arch/x86/kvm/vmx.c | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-)