Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Xiaoyao Li <xiaoyao.li@linux.intel.com>
To:     kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
Cc:     Xiaoyao Li <xiaoyao.li@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>, linux-kernel@vger.kernel.org,
        chao.gao@intel.com
Subject: [PATCH v2 1/2] kvm/vmx: avoid CPUID faulting leaking to guest
Date:   Mon, 18 Mar 2019 19:43:23 +0800
Message-Id: <20190318114324.14198-2-xiaoyao.li@linux.intel.com>
In-Reply-To: <20190318114324.14198-1-xiaoyao.li@linux.intel.com>
References: <20190318114324.14198-1-xiaoyao.li@linux.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

cpuid faulting is a feature about CPUID instruction. When cpuif faulting
is enabled, all execution of the CPUID instruction outside system-management
mode (SMM) cause a general-protection (#GP) if the CPL > 0.

About this feature, detailed information can be found at
https://www.intel.com/content/dam/www/public/us/en/documents/application-notes/virtualization-technology-flexmigration-application-note.pdf

Current KVM provides software emulation of this feature for guest.
However, because cpuid faulting takes higher priority over CPUID vm exit (Intel
SDM vol3.25.1.1), there is a risk of leaking cpuid faulting to guest when host
enables it. If host enables cpuid faulting by setting the bit 0 of
MSR_MISC_FEATURES_ENABLES, it will pass to guest since there is no handling of
MSR_MISC_FEATURES_ENABLES yet. As a result, when guest calls CPUID instruction
in CPL > 0, it will generate a #GP instead of CPUID vm eixt.

This issue will cause guest boot failure when guest uses *modprobe*
to load modules. *modprobe* calls CPUID instruction, thus causing #GP in
guest. Since there is no handling of cpuid faulting in #GP handler, guest
fails boot.

To fix this issue, we should switch cpuid faulting bit between host and guest.
Since MSR_MISC_FEATURES_ENABLES is intel-specific, this patch implement the
switching only in vmx. It clears the cpuid faulting bit and save host's
value before switching to guest, and restores the cpuid faulting settings of
host before switching to host.

Because kvm provides the software emulation of cpuid faulting, we can
just clear the cpuid faulting bit in hardware MSR when switching to
guest.

Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
---
Changes in v2:
- move the save/restore of cpuid faulting bit to
vmx_prepare_swich_to_guest/vmx_prepare_swich_to_host to avoid every
vmentry RDMSR, based on Paolo's comment.

---
 arch/x86/kvm/vmx/vmx.c | 34 ++++++++++++++++++++++++++++++++++
 arch/x86/kvm/vmx/vmx.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 541d442edd4e..2c59e0209e36 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1035,6 +1035,23 @@ static void pt_guest_exit(struct vcpu_vmx *vmx)
 	wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
 }
 
+static void vmx_save_host_cpuid_fault(struct vcpu_vmx *vmx)
+{
+	u64 host_val;
+
+	if (!boot_cpu_has(X86_FEATURE_CPUID_FAULT))
+		return;
+
+	rdmsrl(MSR_MISC_FEATURES_ENABLES, host_val);
+	vmx->host_msr_misc_features_enables = host_val;
+
+	/* clear cpuid fault bit to avoid it leak to guest */
+	if (host_val & MSR_MISC_FEATURES_ENABLES_CPUID_FAULT) {
+		wrmsrl(MSR_MISC_FEATURES_ENABLES,
+		       host_val & ~MSR_MISC_FEATURES_ENABLES_CPUID_FAULT);
+	}
+}
+
 void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -1068,6 +1085,8 @@ void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
 	vmx->loaded_cpu_state = vmx->loaded_vmcs;
 	host_state = &vmx->loaded_cpu_state->host_state;
 
+	vmx_save_host_cpuid_fault(vmx);
+
 	/*
 	 * Set host fs and gs selectors.  Unfortunately, 22.2.3 does not
 	 * allow segment selectors with cpl > 0 or ti == 1.
@@ -1124,6 +1143,19 @@ void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
 	}
 }
 
+static void vmx_restore_host_cpuid_fault(struct vcpu_vmx *vmx)
+{
+	u64 msrval;
+
+	if (!boot_cpu_has(X86_FEATURE_CPUID_FAULT))
+		return;
+
+	rdmsrl(MSR_MISC_FEATURES_ENABLES, msrval);
+	msrval |= vmx->host_msr_misc_features_enables &
+		MSR_MISC_FEATURES_ENABLES_CPUID_FAULT;
+	wrmsrl(MSR_MISC_FEATURES_ENABLES, msrval);
+}
+
 static void vmx_prepare_switch_to_host(struct vcpu_vmx *vmx)
 {
 	struct vmcs_host_state *host_state;
@@ -1137,6 +1169,8 @@ static void vmx_prepare_switch_to_host(struct vcpu_vmx *vmx)
 	++vmx->vcpu.stat.host_state_reload;
 	vmx->loaded_cpu_state = NULL;
 
+	vmx_restore_host_cpuid_fault(vmx);
+
 #ifdef CONFIG_X86_64
 	rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
 #endif
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 5df73b36fa49..ba867bbc5676 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -268,6 +268,8 @@ struct vcpu_vmx {
 	u64 msr_ia32_feature_control_valid_bits;
 	u64 ept_pointer;
 
+	u64 host_msr_misc_features_enables;
+
 	struct pt_desc pt_desc;
 };
 
-- 
2.19.1