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

[Sean Christopherson <seanjc@google.com>]

+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?