linux-kernel - Re: [PATCH 1/3] KVM: nSVM: Use intuitive local variables in recalc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aYOCAH8zLLXllou7@google.com>
Date: Wed, 4 Feb 2026 09:29:36 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Kevin Cheng <chengkev@...gle.com>, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] KVM: nSVM: Use intuitive local variables in recalc_intercepts()

On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> recalc_intercepts() currently uses c, h, g as local variables for the
> control area of the current VMCB, vmcb01, and (cached) vmcb12.
> 
> The current VMCB should always be vmcb02 when recalc_intercepts() is
> executed in guest mode. Use vmcb01/vmcb02 local variables instead to
> make it clear the function is updating intercepts in vmcb02 based on the
> intercepts in vmcb01 and (cached) vmcb12.
> 
> Add a WARNING() if the current VMCB is not in fact vmcb02.

This belongs in a separate patch.

> No functional change intended.
> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@...ux.dev>
> ---
>  arch/x86/kvm/svm/nested.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index f295a41ec659..2dda52221fd8 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -125,8 +125,7 @@ static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
>  
>  void recalc_intercepts(struct vcpu_svm *svm)
>  {
> -	struct vmcb_control_area *c, *h;
> -	struct vmcb_ctrl_area_cached *g;
> +	struct vmcb *vmcb01, *vmcb02;
>  	unsigned int i;
>  
>  	vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
> @@ -134,14 +133,14 @@ void recalc_intercepts(struct vcpu_svm *svm)
>  	if (!is_guest_mode(&svm->vcpu))
>  		return;
>  
> -	c = &svm->vmcb->control;
> -	h = &svm->vmcb01.ptr->control;
> -	g = &svm->nested.ctl;
> +	vmcb01 = svm->vmcb01.ptr;
> +	vmcb02 = svm->nested.vmcb02.ptr;
> +	WARN_ON_ONCE(svm->vmcb != vmcb02);

If we're going to bother with a WARN, then this code should definitely bail,
because configuring vmcb01 using the nested logic is all but guaranteed to break
L1 in weird ways.

>  	for (i = 0; i < MAX_INTERCEPT; i++)
> -		c->intercepts[i] = h->intercepts[i];
> +		vmcb02->control.intercepts[i] = vmcb01->control.intercepts[i];
>  
> -	if (g->int_ctl & V_INTR_MASKING_MASK) {
> +	if (svm->nested.ctl.int_ctl & V_INTR_MASKING_MASK) {

I vote to keep a pointer to the cached control as vmcb12_ctrl.  Coming from a
nVMX-focused background, I can never remember what svm->nested.ctl holds.  For
me, this is waaaay more intuivite:

	if (vmcb12_ctrl->int_ctl & V_INTR_MASKING_MASK) {

>  	for (i = 0; i < MAX_INTERCEPT; i++)
> -		c->intercepts[i] |= g->intercepts[i];
> +		vmcb02->control.intercepts[i] |= svm->nested.ctl.intercepts[i];

And even more so here:

	for (i = 0; i < MAX_INTERCEPT; i++)
		vmcb02->control.intercepts[i] |= vmcb12_ctrl->intercepts[i];

>  
>  	/* If SMI is not intercepted, ignore guest SMI intercept as well  */
>  	if (!intercept_smi)
> -		vmcb_clr_intercept(c, INTERCEPT_SMI);
> +		vmcb_clr_intercept(&vmcb02->control, INTERCEPT_SMI);
>  
>  	if (nested_vmcb_needs_vls_intercept(svm)) {
>  		/*
> @@ -177,10 +176,10 @@ void recalc_intercepts(struct vcpu_svm *svm)
>  		 * we must intercept these instructions to correctly
>  		 * emulate them in case L1 doesn't intercept them.
>  		 */
> -		vmcb_set_intercept(c, INTERCEPT_VMLOAD);
> -		vmcb_set_intercept(c, INTERCEPT_VMSAVE);
> +		vmcb_set_intercept(&vmcb02->control, INTERCEPT_VMLOAD);
> +		vmcb_set_intercept(&vmcb02->control, INTERCEPT_VMSAVE);
>  	} else {
> -		WARN_ON(!(c->virt_ext & VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK));
> +		WARN_ON(!(vmcb02->control.virt_ext & VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK));

Opportunistically switch this to WARN_ON_ONCE.  Any "unguarded" WARN in KVM
(outside of e.g. __init code) is just asking for a self-DoS.

>  	}
>  }
>  
> -- 
> 2.52.0.457.g6b5491de43-goog
>