[PATCH] KVM: nSVM: Reset the virtual VMLOAD/VMSAVE bit in vmcb02 on every nested VMRUN

[PATCH] KVM: nSVM: Reset the virtual VMLOAD/VMSAVE bit in vmcb02 on every nested VMRUN

[Amirmohammad Eftekhar]

The SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE (VLS) bit in
vmcb02->control.misc_ctl2 is OR-set in nested_vmcb02_prepare_control()
when L2 is allowed to use virtual VMLOAD/VMSAVE, but it is never reset.
vmcb02 is a persistent per-vCPU structure reused across nested VMRUNs,
so once one nested entry enables VLS the bit leaks into every later
entry -- including entries where vmcb12 has VLS cleared and L1 has asked
KVM to intercept VMLOAD/VMSAVE.  Per AMD's APM, virtual VMLOAD/VMSAVE is
honored over the VMLOAD/VMSAVE intercepts, so with the stale bit set
L2's VMLOAD/VMSAVE are virtualized and run without a #VMEXIT, and the
intercept L1 explicitly requested is silently dropped.

misc_ctl is reset from vmcb01 a few lines earlier; misc_ctl2's VLS bit
has no equivalent reset.  The only writes to misc_ctl2 are this OR-set,
the vmcb01 init OR-set, and the independent V_LBR (bit 0) set/clear, so
nothing clears bit 1 between entries.  nested_vmcb02_recalc_intercepts()
sets INTERCEPT_VMLOAD/VMSAVE when nested_vmcb_needs_vls_intercept() is
true and otherwise only WARNs on the reverse condition; it never catches
the real hazard of VLS on while the intercept is also set.

Reset the VLS bit before the conditional set, mirroring misc_ctl, and
add a forward WARN so a future regression trips immediately.

This is distinct from CVE-2021-3656 (single-VMRUN under-sanitization of
a VLS bit L1 set in vmcb12 without being granted the feature); here L1
is granted VLS and uses it legitimately, and the bit goes stale across
VMRUNs.

Signed-off-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
---
 arch/x86/kvm/svm/nested.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index af0ef6784..65e61b796 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -197,6 +197,14 @@ void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm)
 	} else {
 		WARN_ON_ONCE(!(vmcb02->control.misc_ctl2 & SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE));
 	}
+
+	/*
+	 * If KVM is intercepting VMLOAD/VMSAVE, virtual VMLOAD/VMSAVE must be
+	 * off, or hardware would honor the virtualization and render the
+	 * intercept inert.
+	 */
+	WARN_ON_ONCE(vmcb_is_intercept(&vmcb02->control, INTERCEPT_VMLOAD) &&
+		     (vmcb02->control.misc_ctl2 & SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE));
 }
 
 /*
@@ -994,6 +1002,13 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
 
 	/* SVM_MISC2_ENABLE_V_LBR is controlled by svm_update_lbrv() */
 
+	/*
+	 * vmcb02 is reused across nested VMRUNs and the VLS bit below is only
+	 * OR-set, so clear it first; a stale bit would let L2 run VMLOAD/VMSAVE
+	 * unintercepted even on a VMRUN that asks KVM to intercept them.
+	 */
+	vmcb02->control.misc_ctl2 &= ~SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE;
+
 	if (!nested_vmcb_needs_vls_intercept(svm))
 		vmcb02->control.misc_ctl2 |= SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE;
 
-- 
2.54.0 (Apple Git-157)

Re: [PATCH] KVM: nSVM: Reset the virtual VMLOAD/VMSAVE bit in vmcb02 on every nested VMRUN

[Sean Christopherson]

+Yosry and Jim

On Sat, Oct 17, 2026, Amirmohammad Eftekhar wrote:
> The SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE (VLS) bit in
> vmcb02->control.misc_ctl2 is OR-set in nested_vmcb02_prepare_control()
> when L2 is allowed to use virtual VMLOAD/VMSAVE, but it is never reset.
> vmcb02 is a persistent per-vCPU structure reused across nested VMRUNs,
> so once one nested entry enables VLS the bit leaks into every later
> entry -- including entries where vmcb12 has VLS cleared and L1 has asked
> KVM to intercept VMLOAD/VMSAVE.  Per AMD's APM, virtual VMLOAD/VMSAVE is
> honored over the VMLOAD/VMSAVE intercepts, so with the stale bit set
> L2's VMLOAD/VMSAVE are virtualized and run without a #VMEXIT, and the
> intercept L1 explicitly requested is silently dropped.
> 
> misc_ctl is reset from vmcb01 a few lines earlier; misc_ctl2's VLS bit
> has no equivalent reset.  The only writes to misc_ctl2 are this OR-set,
> the vmcb01 init OR-set, and the independent V_LBR (bit 0) set/clear, so
> nothing clears bit 1 between entries.  nested_vmcb02_recalc_intercepts()
> sets INTERCEPT_VMLOAD/VMSAVE when nested_vmcb_needs_vls_intercept() is
> true and otherwise only WARNs on the reverse condition; it never catches
> the real hazard of VLS on while the intercept is also set.
> 
> Reset the VLS bit before the conditional set, mirroring misc_ctl, and
> add a forward WARN so a future regression trips immediately.
> 
> This is distinct from CVE-2021-3656 (single-VMRUN under-sanitization of
> a VLS bit L1 set in vmcb12 without being granted the feature); here L1
> is granted VLS and uses it legitimately, and the bit goes stale across
> VMRUNs.
> 
> Signed-off-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
> ---
>  arch/x86/kvm/svm/nested.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index af0ef6784..65e61b796 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -197,6 +197,14 @@ void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm)
>  	} else {
>  		WARN_ON_ONCE(!(vmcb02->control.misc_ctl2 & SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE));
>  	}
> +
> +	/*
> +	 * If KVM is intercepting VMLOAD/VMSAVE, virtual VMLOAD/VMSAVE must be
> +	 * off, or hardware would honor the virtualization and render the
> +	 * intercept inert.
> +	 */
> +	WARN_ON_ONCE(vmcb_is_intercept(&vmcb02->control, INTERCEPT_VMLOAD) &&
> +		     (vmcb02->control.misc_ctl2 & SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE));
>  }
>  
>  /*
> @@ -994,6 +1002,13 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
>  
>  	/* SVM_MISC2_ENABLE_V_LBR is controlled by svm_update_lbrv() */
>  
> +	/*
> +	 * vmcb02 is reused across nested VMRUNs and the VLS bit below is only
> +	 * OR-set, so clear it first; a stale bit would let L2 run VMLOAD/VMSAVE
> +	 * unintercepted even on a VMRUN that asks KVM to intercept them.
> +	 */

This becomes self-explanatory if written as:

	if (nested_vmcb_needs_vls_intercept(svm))
		vmcb02->control.misc_ctl2 &= ~SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE;
	else
		vmcb02->control.misc_ctl2 |= SVM_MISC2_ENABLE_V_VMLOAD_VMSAVE;

and I definitely think we should un-invert the check to match nested_vmcb02_recalc_intercepts().

That said, not explicitly initializing all of misc_ctl2 is crazy.  AFAICT, LBR
virtualization is the only other bit in misc_ctl2, and it to is always computed
dynamically so I'd rather do this:

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 1503fee15baa..6ea35a3dc361 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -884,8 +884,6 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
 	}
 
 	/*
-	 * Copied from vmcb01.  msrpm_base can be overwritten later.
-	 *
 	 * SVM_MISC_ENABLE_NP in vmcb12 is only used for consistency checks.  If
 	 * L1 enables NPTs, KVM shadows L1's NPTs and uses those to run L2. If
 	 * L1 disables NPT, KVM runs L2 with the same NPTs used to run L1. For
@@ -893,6 +891,7 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
 	 * to L1 GPAs, so the same NPTs can be used for L1 and L2.
 	 */
 	vmcb02->control.misc_ctl = vmcb01->control.misc_ctl & (SVM_MISC_ENABLE_NP | SVM_MISC_ENABLE_GMET);
+	vmcb02->control.misc_ctl2 = 0;
 	vmcb02->control.iopm_base_pa = vmcb01->control.iopm_base_pa;
 	vmcb02->control.msrpm_base_pa = vmcb01->control.msrpm_base_pa;
 	vmcb_mark_dirty(vmcb02, VMCB_PERM_MAP);

Argh!  But nested_vmcb02_prepare_save() isn't called in the svm_set_nested_state()
case, so AFAICT svm_update_lbrv() is never called.  Though that seems like a
pre-existing?