From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753782AbcFPGGZ (ORCPT ); Thu, 16 Jun 2016 02:06:25 -0400 Received: from mga03.intel.com ([134.134.136.65]:4796 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752207AbcFPGGT (ORCPT ); Thu, 16 Jun 2016 02:06:19 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,478,1459839600"; d="scan'208";a="719899117" From: Haozhong Zhang To: kvm@vger.kernel.org Cc: Paolo Bonzini , rkrcmar@redhat.com, Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Gleb Natapov , Boris Petkov , Tony Luck , Andi Kleen , Ashok Raj , Haozhong Zhang Subject: [PATCH v2 2/3] KVM: VMX: validate individual bits of guest MSR_IA32_FEATURE_CONTROL Date: Thu, 16 Jun 2016 14:05:30 +0800 Message-Id: <20160616060531.30028-3-haozhong.zhang@intel.com> X-Mailer: git-send-email 2.9.0 In-Reply-To: <20160616060531.30028-1-haozhong.zhang@intel.com> References: <20160616060531.30028-1-haozhong.zhang@intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org KVM currently does not check the value written to guest MSR_IA32_FEATURE_CONTROL, though bits corresponding to disabled features may be set. This patch makes KVM to validate individual bits written to guest MSR_IA32_FEATURE_CONTROL according to enabled features. Signed-off-by: Haozhong Zhang --- arch/x86/kvm/vmx.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 6b63f2d..1dc89c5 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -602,7 +602,16 @@ struct vcpu_vmx { u32 guest_pkru; u32 host_pkru; + /* + * Only bits masked by msr_ia32_feature_control_valid_bits can be set in + * msr_ia32_feature_control. + * + * msr_ia32_feature_control_valid_bits should be modified by + * feature_control_valid_bits_add/del(), and only bits masked by + * FEATURE_CONTROL_MAX_VALID_BITS can be modified. + */ u64 msr_ia32_feature_control; + u64 msr_ia32_feature_control_valid_bits; }; enum segment_cache_field { @@ -624,6 +633,30 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu) return &(to_vmx(vcpu)->pi_desc); } +/* + * FEATURE_CONTROL_LOCKED is added/removed automatically by + * feature_control_valid_bits_add/del(), so it's not included here. + */ +#define FEATURE_CONTROL_MAX_VALID_BITS \ + FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX + +static void feature_control_valid_bits_add(struct kvm_vcpu *vcpu, uint64_t bits) +{ + ASSERT(!(bits & ~FEATURE_CONTROL_MAX_VALID_BITS)); + to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |= + bits | FEATURE_CONTROL_LOCKED; +} + +static void feature_control_valid_bits_del(struct kvm_vcpu *vcpu, uint64_t bits) +{ + uint64_t *valid_bits = + &to_vmx(vcpu)->msr_ia32_feature_control_valid_bits; + ASSERT(!(bits & ~FEATURE_CONTROL_MAX_VALID_BITS)); + *valid_bits &= ~bits; + if (!(*valid_bits & ~FEATURE_CONTROL_LOCKED)) + *valid_bits = 0; +} + #define VMCS12_OFFSET(x) offsetof(struct vmcs12, x) #define FIELD(number, name) [number] = VMCS12_OFFSET(name) #define FIELD64(number, name) [number] = VMCS12_OFFSET(name), \ @@ -2864,6 +2897,14 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) return 0; } +static inline bool vmx_feature_control_msr_valid(struct kvm_vcpu *vcpu, + uint64_t val) +{ + uint64_t valid_bits = to_vmx(vcpu)->msr_ia32_feature_control_valid_bits; + + return valid_bits && !(val & ~valid_bits); +} + /* * Reads an msr value (of 'msr_index') into 'pdata'. * Returns 0 on success, non-0 otherwise. @@ -2906,7 +2947,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) msr_info->data = vmcs_read64(GUEST_BNDCFGS); break; case MSR_IA32_FEATURE_CONTROL: - if (!nested_vmx_allowed(vcpu)) + if (!vmx_feature_control_msr_valid(vcpu, 0)) return 1; msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control; break; @@ -2999,7 +3040,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) ret = kvm_set_msr_common(vcpu, msr_info); break; case MSR_IA32_FEATURE_CONTROL: - if (!nested_vmx_allowed(vcpu) || + if (!vmx_feature_control_msr_valid(vcpu, data) || (to_vmx(vcpu)->msr_ia32_feature_control & FEATURE_CONTROL_LOCKED && !msr_info->host_initiated)) return 1; @@ -9095,6 +9136,13 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) vmx->nested.nested_vmx_secondary_ctls_high &= ~SECONDARY_EXEC_PCOMMIT; } + + if (nested_vmx_allowed(vcpu)) + feature_control_valid_bits_add + (vcpu, FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX); + else + feature_control_valid_bits_del + (vcpu, FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX); } static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry) -- 2.9.0