Skip to contentSkip to conversation

KVM: nVMX: Don't leak L1 MMIO regions to L2

PATCH Superseded Jim Mattson 2 participants 2 messages Oct 9, 2019

Subsystems (3)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 KVM: nVMX: Don't leak L1 MMIO regions to L2

If the "virtualize APIC accesses" VM-execution control is set in the VMCS, the APIC virtualization hardware is triggered when a page walk in VMX non-root mode terminates at a PTE wherein the address of the 4k page frame matches the APIC-access address specified in the VMCS. On hardware, the APIC-access address may be any valid 4k-aligned physical address.

KVM's nVMX implementation enforces the additional constraint that the APIC-access address specified in the vmcs12 must be backed by cacheable memory in L1. If not, L0 will simply clear the "virtualize APIC accesses" VM-execution control in the vmcs02.

The problem with this approach is that the L1 guest has arranged the vmcs12 EPT tables--or shadow page tables, if the "enable EPT" VM-execution control is clear in the vmcs12--so that the L2 guest physical address(es)--or L2 guest linear address(es)--that reference the L2 APIC map to the APIC-access address specified in the vmcs12. Without the "virtualize APIC accesses" VM-execution control in the vmcs02, the APIC accesses in the L2 guest will directly access the APIC-access page in L1.

When there is no mapping whatsoever for the APIC-access address in L1, the L2 VM just loses the intended APIC virtualization. However, when the APIC-access address is mapped to an MMIO region in L1, the L2 guest gets direct access to the L1 MMIO device. For example, if the APIC-access address specified in the vmcs12 is 0xfee00000, then L2 gets direct access to L1's APIC.

Since this vmcs12 configuration is something that KVM cannot faithfully emulate, the appropriate response is to exit to userspace with KVM_INTERNAL_ERROR_EMULATION.

Fixes fe3ef05c7572 ("KVM: nVMX: Prepare vmcs02 from vmcs01 and vmcs12")

---
v1 -> v2: Added enum enter_vmx_status

 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/vmx/nested.c       | 65 +++++++++++++++++++--------------
 arch/x86/kvm/vmx/nested.h       | 13 ++++++-
 arch/x86/kvm/x86.c              |  8 +++-
 4 files changed, 56 insertions(+), 32 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -1
index 50eb430b0ad8..24d6598dea29 100644
@@ -1189,7 +1189,7 @@ struct kvm_x86_ops {
 	int (*set_nested_state)(struct kvm_vcpu *vcpu,
 				struct kvm_nested_state __user *user_kvm_nested_state,
 				struct kvm_nested_state *kvm_state);
-	void (*get_vmcs12_pages)(struct kvm_vcpu *vcpu);
+	bool (*get_vmcs12_pages)(struct kvm_vcpu *vcpu);
 
 	int (*smi_allowed)(struct kvm_vcpu *vcpu);
 	int (*pre_enter_smm)(struct kvm_vcpu *vcpu, char *smstate);
arch/x86/kvm/vmx/nested.c +37 -28

136 lines not on this page · raw message

arch/x86/kvm/vmx/nested.h +12 -1
index 187d39bf0bf1..07cf5cef86f6 100644
@@ -6,6 +6,16 @@
 #include "vmcs12.h"
 #include "vmx.h"
 
+/*
+ * Status returned by nested_vmx_enter_non_root_mode():
+ */
+enum enter_vmx_status {
+	ENTER_VMX_SUCCESS,	/* Successfully entered VMX non-root mode */
+	ENTER_VMX_VMFAIL,	/* Consistency check VMFail */
+	ENTER_VMX_VMEXIT,	/* Consistency check VMExit */
+	ENTER_VMX_ERROR,	/* KVM internal error */
+};
+
 void vmx_leave_nested(struct kvm_vcpu *vcpu);
 void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps,
 				bool apicv);
@@ -13,7 +23,8 @@ void nested_vmx_hardware_unsetup(void);
 __init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *));
 void nested_vmx_vcpu_setup(void);
 void nested_vmx_free_vcpu(struct kvm_vcpu *vcpu);
-int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry);
+enum enter_vmx_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
+						     bool from_vmentry);
 bool nested_vmx_exit_reflected(struct kvm_vcpu *vcpu, u32 exit_reason);
 void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 		       u32 exit_intr_info, unsigned long exit_qualification);
arch/x86/kvm/x86.c +6 -2
index 661e2bf38526..2cf26f159071 100644
@@ -7941,8 +7941,12 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 	bool req_immediate_exit = false;
 
 	if (kvm_request_pending(vcpu)) {
-		if (kvm_check_request(KVM_REQ_GET_VMCS12_PAGES, vcpu))
-			kvm_x86_ops->get_vmcs12_pages(vcpu);
+		if (kvm_check_request(KVM_REQ_GET_VMCS12_PAGES, vcpu)) {
+			if (unlikely(!kvm_x86_ops->get_vmcs12_pages(vcpu))) {
+				r = 0;
+				goto out;
+			}
+		}
 		if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu))
 			kvm_mmu_unload(vcpu);
 		if (kvm_check_request(KVM_REQ_MIGRATE_TIMER, vcpu))
Signature · 1 line
2.23.0.581.g78d2f28ef7-goog
#1 lore raw
I love your patch! Perhaps something to improve:

Hi Jim,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kvm/linux-next]
[cannot apply to v5.4-rc2 next-20191008]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Jim-Mattson/KVM-nVMX-Don-t-leak-L1-MMIO-regions-to-L2/20191009-120808
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag

All warnings (new ones prefixed by >>):

arch/x86/kvm/vmx/nested.c: In function 'nested_vmx_run':

>> arch/x86/kvm/vmx/nested.c:3253:3: warning: enumeration value 'ENTER_VMX_SUCCESS' not handled in switch [-Wswitch]
      switch (status) {
      ^~~~~~

vim +/ENTER_VMX_SUCCESS +3253 arch/x86/kvm/vmx/nested.c

3180
3181 /*
3182 * nested_vmx_run() handles a nested entry, i.e., a VMLAUNCH or VMRESUME on L1
3183 * for running an L2 nested guest.
3184 */
3185 static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
3186 {
3187 struct vmcs12 *vmcs12;
3188 enum enter_vmx_status status;
3189 struct vcpu_vmx *vmx = to_vmx(vcpu);
3190 u32 interrupt_shadow = vmx_get_interrupt_shadow(vcpu);
3191
3192 if (!nested_vmx_check_permission(vcpu))
3193 return 1;
3194
3195 if (!nested_vmx_handle_enlightened_vmptrld(vcpu, launch))
3196 return 1;
3197
3198 if (!vmx->nested.hv_evmcs && vmx->nested.current_vmptr == -1ull)
3199 return nested_vmx_failInvalid(vcpu);
3200
3201 vmcs12 = get_vmcs12(vcpu);
3202
3203 /*
3204 * Can't VMLAUNCH or VMRESUME a shadow VMCS. Despite the fact
3205 * that there *is* a valid VMCS pointer, RFLAGS.CF is set
3206 * rather than RFLAGS.ZF, and no error number is stored to the
3207 * VM-instruction error field.
3208 */
3209 if (vmcs12->hdr.shadow_vmcs)
3210 return nested_vmx_failInvalid(vcpu);
3211
3212 if (vmx->nested.hv_evmcs) {
3213 copy_enlightened_to_vmcs12(vmx);
3214 /* Enlightened VMCS doesn't have launch state */
3215 vmcs12->launch_state = !launch;
3216 } else if (enable_shadow_vmcs) {
3217 copy_shadow_to_vmcs12(vmx);
3218 }
3219
3220 /*
3221 * The nested entry process starts with enforcing various prerequisites
3222 * on vmcs12 as required by the Intel SDM, and act appropriately when
3223 * they fail: As the SDM explains, some conditions should cause the
3224 * instruction to fail, while others will cause the instruction to seem
3225 * to succeed, but return an EXIT_REASON_INVALID_STATE.
3226 * To speed up the normal (success) code path, we should avoid checking
3227 * for misconfigurations which will anyway be caught by the processor
3228 * when using the merged vmcs02.
3229 */
3230 if (interrupt_shadow & KVM_X86_SHADOW_INT_MOV_SS)
3231 return nested_vmx_failValid(vcpu,
3232 VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS);
3233
3234 if (vmcs12->launch_state == launch)
3235 return nested_vmx_failValid(vcpu,
3236 launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS
3237 : VMXERR_VMRESUME_NONLAUNCHED_VMCS);
3238
3239 if (nested_vmx_check_controls(vcpu, vmcs12))
3240 return nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD);
3241
3242 if (nested_vmx_check_host_state(vcpu, vmcs12))
3243 return nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
3244
3245 /*
3246 * We're finally done with prerequisite checking, and can start with
3247 * the nested entry.
3248 */
3249 vmx->nested.nested_run_pending = 1;
3250 status = nested_vmx_enter_non_root_mode(vcpu, true);
3251 if (status != ENTER_VMX_SUCCESS) {
3252 vmx->nested.nested_run_pending = 0;

> 3253			switch (status) {
  3254			case ENTER_VMX_VMFAIL:
  3255				return nested_vmx_failValid(vcpu,
  3256					VMXERR_ENTRY_INVALID_CONTROL_FIELD);
  3257			case ENTER_VMX_VMEXIT:
  3258				return 1;
  3259			case ENTER_VMX_ERROR:
  3260				return 0;
  3261			}
  3262		}
  3263	
  3264		/* Hide L1D cache contents from the nested guest.  */
  3265		vmx->vcpu.arch.l1tf_flush_l1d = true;
  3266	
  3267		/*
  3268		 * Must happen outside of nested_vmx_enter_non_root_mode() as it will
  3269		 * also be used as part of restoring nVMX state for
  3270		 * snapshot restore (migration).
  3271		 *
  3272		 * In this flow, it is assumed that vmcs12 cache was
  3273		 * trasferred as part of captured nVMX state and should
  3274		 * therefore not be read from guest memory (which may not
  3275		 * exist on destination host yet).
  3276		 */
  3277		nested_cache_shadow_vmcs12(vcpu, vmcs12);
  3278	
  3279		/*
  3280		 * If we're entering a halted L2 vcpu and the L2 vcpu won't be
  3281		 * awakened by event injection or by an NMI-window VM-exit or
  3282		 * by an interrupt-window VM-exit, halt the vcpu.
  3283		 */
  3284		if ((vmcs12->guest_activity_state == GUEST_ACTIVITY_HLT) &&
  3285		    !(vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK) &&
  3286		    !(vmcs12->cpu_based_vm_exec_control & CPU_BASED_VIRTUAL_NMI_PENDING) &&
  3287		    !((vmcs12->cpu_based_vm_exec_control & CPU_BASED_VIRTUAL_INTR_PENDING) &&

3288 (vmcs12->guest_rflags & X86_EFLAGS_IF))) {
3289 vmx->nested.nested_run_pending = 0;
3290 return kvm_vcpu_halt(vcpu);
3291 }
3292 return 1;
3293 }
3294

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation