From: dv <dv@openbsd.org>
Date: Tue, 3 Sep 2024 13:36:19 +0000 (+0000)
Subject: vmm(4)/vmx: avoid VPID leakage by allocating at vcpu init.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=04c660e357877d627f1bb36e317572bef05b3296;p=openbsd

vmm(4)/vmx: avoid VPID leakage by allocating at vcpu init.

VPID allocation being dependent on the host and guest config
(consequently pushing it into the reset register handler) creates
a leak where previous VPIDs are not freed if the hypervisor program
resets a vcpu's registers.

Recent SVM related changes pulled the VPID (ASID in AMD world)
allocation up into vcpu initialization. This change does the same
for VMX and cleans up appropriate logic. Minor changes to keep SVM
and VMX styles in line with each other.

ok bluhm@
---

diff --git a/sys/arch/amd64/amd64/vmm_machdep.c b/sys/arch/amd64/amd64/vmm_machdep.c
index c84fbb3273c..7e87ccc7dd9 100644
--- a/sys/arch/amd64/amd64/vmm_machdep.c
+++ b/sys/arch/amd64/amd64/vmm_machdep.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: vmm_machdep.c,v 1.33 2024/08/27 09:16:03 bluhm Exp $ */
+/* $OpenBSD: vmm_machdep.c,v 1.34 2024/09/03 13:36:19 dv Exp $ */
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -2253,7 +2253,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg_state *vrs)
 	uint32_t pinbased, procbased, procbased2, exit, entry;
 	uint32_t want1, want0;
 	uint64_t ctrlval, cr3, msr_misc_enable;
-	uint16_t ctrl, vpid;
+	uint16_t ctrl;
 	struct vmx_msr_store *msr_store;
 
 	rw_assert_wrlock(&vcpu->vc_lock);
@@ -2516,30 +2516,12 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg_state *vrs)
 	    IA32_VMX_ACTIVATE_SECONDARY_CONTROLS, 1)) {
 		if (vcpu_vmx_check_cap(vcpu, IA32_VMX_PROCBASED2_CTLS,
 		    IA32_VMX_ENABLE_VPID, 1)) {
-
-			/* We may sleep during allocation, so reload VMCS. */
-			vcpu->vc_last_pcpu = curcpu();
-			ret = vmm_alloc_vpid(&vpid);
-			if (vcpu_reload_vmcs_vmx(vcpu)) {
-				printf("%s: failed to reload vmcs\n", __func__);
-				ret = EINVAL;
-				goto exit;
-			}
-			if (ret) {
-				DPRINTF("%s: could not allocate VPID\n",
-				    __func__);
-				ret = EINVAL;
-				goto exit;
-			}
-
-			if (vmwrite(VMCS_GUEST_VPID, vpid)) {
+			if (vmwrite(VMCS_GUEST_VPID, vcpu->vc_vpid)) {
 				DPRINTF("%s: error setting guest VPID\n",
 				    __func__);
 				ret = EINVAL;
 				goto exit;
 			}
-
-			vcpu->vc_vpid = vpid;
 		}
 	}
 
@@ -2832,13 +2814,19 @@ vcpu_init_vmx(struct vcpu *vcpu)
 	uint32_t cr0, cr4;
 	int ret = 0;
 
+	/* Allocate a VPID early to avoid km_alloc if we're out of VPIDs. */
+	if (vmm_alloc_vpid(&vcpu->vc_vpid))
+		return (ENOMEM);
+
 	/* Allocate VMCS VA */
 	vcpu->vc_control_va = (vaddr_t)km_alloc(PAGE_SIZE, &kv_page, &kp_zero,
 	    &kd_waitok);
 	vcpu->vc_vmx_vmcs_state = VMCS_CLEARED;
 
-	if (!vcpu->vc_control_va)
-		return (ENOMEM);
+	if (!vcpu->vc_control_va) {
+		ret = ENOMEM;
+		goto exit;
+	}
 
 	/* Compute VMCS PA */
 	if (!pmap_extract(pmap_kernel(), vcpu->vc_control_va,
@@ -3091,15 +3079,20 @@ vcpu_reset_regs(struct vcpu *vcpu, struct vcpu_reg_state *vrs)
 int
 vcpu_init_svm(struct vcpu *vcpu, struct vm_create_params *vcp)
 {
-	uint16_t asid;
 	int ret = 0;
 
+	/* Allocate an ASID early to avoid km_alloc if we're out of ASIDs. */
+	if (vmm_alloc_vpid(&vcpu->vc_vpid))
+		return (ENOMEM);
+
 	/* Allocate VMCB VA */
 	vcpu->vc_control_va = (vaddr_t)km_alloc(PAGE_SIZE, &kv_page, &kp_zero,
 	    &kd_waitok);
 
-	if (!vcpu->vc_control_va)
-		return (ENOMEM);
+	if (!vcpu->vc_control_va) {
+		ret = ENOMEM;
+		goto exit;
+	}
 
 	/* Compute VMCB PA */
 	if (!pmap_extract(pmap_kernel(), vcpu->vc_control_va,
@@ -3173,14 +3166,6 @@ vcpu_init_svm(struct vcpu *vcpu, struct vm_create_params *vcp)
 	    (uint64_t)vcpu->vc_svm_ioio_va,
 	    (uint64_t)vcpu->vc_svm_ioio_pa);
 
-	/* Guest VCPU ASID */
-	if (vmm_alloc_vpid(&asid)) {
-		DPRINTF("%s: could not allocate asid\n", __func__);
-		ret = EINVAL;
-		goto exit;
-	}
-	vcpu->vc_vpid = asid;
-
 	/* Shall we enable SEV? */
 	vcpu->vc_sev = vcp->vcp_sev;
 
@@ -3260,8 +3245,7 @@ vcpu_deinit_vmx(struct vcpu *vcpu)
 	}
 #endif
 
-	if (vcpu->vc_vmx_vpid_enabled)
-		vmm_free_vpid(vcpu->vc_vpid);
+	vmm_free_vpid(vcpu->vc_vpid);
 }
 
 /*