[PATCH] KVM: nVMX: Restore L2's DEBUGCTL/BNDCFGS/CET from vmcs12 on RSM and nested state restore

[Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>]

nested_vmx_enter_non_root_mode() snapshots the guest's DEBUGCTL, BNDCFGS
and CET state (S_CET/SSP/SSP_TBL) into vmx->nested.pre_vmenter_* so that
prepare_vmcs02() can preserve into vmcs02 any of those fields that VM
entry leaves unchanged, i.e. whose VM_ENTRY_LOAD_* control is clear.  The
snapshot reads the currently loaded VMCS.

That is correct for VMLAUNCH/VMRESUME: the active VMCS is vmcs01, and
when VM entry does not load a field L2 does inherit L1's value for it.
It is wrong for the two from_vmentry == false callers -- RSM back into L2
(vmx_leave_smm()) and KVM_SET_NESTED_STATE -- because there the active
VMCS is still vmcs01, holding L1's live state, while the state L2 must
resume with lives in vmcs12.  On RSM the pre-SMI values were put in
vmcs12 when the SMI forced the L2 -> L1 exit (CET via
sync_vmcs02_to_vmcs12(), DEBUGCTL/BNDCFGS tracked in vmcs12 as L2 updates
them); on nested state restore vmcs12 is the state userspace supplied.

So after an SMI taken while L2 is running, followed by RSM -- a sequence
entirely under the guest's control -- or after a restore, L2 resumes with
L1's DEBUGCTL, BNDCFGS and CET instead of its own whenever L1 left the
corresponding VM_ENTRY_LOAD_* control clear.  Nothing rejects it: the
physical VM entry sees a legal vmcs02 (the stale values are valid, just
not L2's), so the state is silently wrong rather than faulted.

Source the snapshot from vmcs12 on the !from_vmentry paths.

Signed-off-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
---
Notes (not for the commit message):

 - Fidelity/correctness bug: the victim is L2 and the entity that both
   owns L1 and drove the SMI is L1, so this is L1 corrupting its own L2,
   not a privilege-boundary crossing -- not memory-safety and not
   guest -> host.  It is a concrete instance of the existing in-tree
   "L2's values may be corrupted on SMI+RSM" TODO near vmx_enter_smm().
 - The same snapshot bug applies to the KVM_SET_NESTED_STATE restore
   path, which this fixes too.
 - DEBUGCTL/BNDCFGS have read the active VMCS here for a long time; the
   CET arm is newer.  Happy to add whatever Fixes:/Cc: stable scope you
   prefer rather than guess the split.
 - Verified against mainline (v7.3-rc6) and kvm-x86/next; please confirm
   it still applies on your tree, I will rebase if needed.

 arch/x86/kvm/vmx/nested.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 151873407ab..c51dcb1cb8d 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3650,19 +3650,38 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,

 	kvm_service_local_tlb_flush_requests(vcpu);

+	/*
+	 * Snapshot the state that VM-Enter leaves unchanged when the matching
+	 * VM_ENTRY_LOAD_* control is clear, so it can be restored into vmcs02.
+	 * On VM-Enter the active VMCS is vmcs01 and its guest state is what L2
+	 * inherits.  On the !from_vmentry paths (RSM back into L2, and
+	 * KVM_SET_NESTED_STATE) the active VMCS is still vmcs01, i.e. holds
+	 * L1's state, whereas L2's state lives in vmcs12 -- read it from there
+	 * so that resuming L2 doesn't clobber its DEBUGCTL/BNDCFGS/CET with
+	 * L1's values.
+	 */
 	if (!vcpu->arch.nested_run_pending ||
 	    !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS))
-		vmx->nested.pre_vmenter_debugctl = vmx_guest_debugctl_read();
+		vmx->nested.pre_vmenter_debugctl = from_vmentry ?
+			vmx_guest_debugctl_read() : vmcs12->guest_ia32_debugctl;
 	if (kvm_mpx_supported() &&
 	    (!vcpu->arch.nested_run_pending ||
 	     !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)))
-		vmx->nested.pre_vmenter_bndcfgs = vmcs_read64(GUEST_BNDCFGS);
+		vmx->nested.pre_vmenter_bndcfgs = from_vmentry ?
+			vmcs_read64(GUEST_BNDCFGS) : vmcs12->guest_bndcfgs;

 	if (!vcpu->arch.nested_run_pending ||
-	    !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_CET_STATE))
-		vmcs_read_cet_state(vcpu, &vmx->nested.pre_vmenter_s_cet,
-				    &vmx->nested.pre_vmenter_ssp,
-				    &vmx->nested.pre_vmenter_ssp_tbl);
+	    !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_CET_STATE)) {
+		if (from_vmentry) {
+			vmcs_read_cet_state(vcpu, &vmx->nested.pre_vmenter_s_cet,
+					    &vmx->nested.pre_vmenter_ssp,
+					    &vmx->nested.pre_vmenter_ssp_tbl);
+		} else {
+			vmx->nested.pre_vmenter_s_cet = vmcs12->guest_s_cet;
+			vmx->nested.pre_vmenter_ssp = vmcs12->guest_ssp;
+			vmx->nested.pre_vmenter_ssp_tbl = vmcs12->guest_ssp_tbl;
+		}
+	}

 	/*
 	 * Stash L1's CR3, so that in the event of a "late" VM-Fail, i.e. a
--
2.54.0