Re: [PATCH] KVM: nVMX: Hide shadow VMCS right after VMCLEAR

[Sean Christopherson <seanjc@google.com>]

On Tue, Jul 21, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> > index 6137e5307d0f6..d63cfdc1740e1 100644
> > --- a/arch/x86/kvm/vmx/nested.c
> > +++ b/arch/x86/kvm/vmx/nested.c
> [ ... ]
> > @@ -350,9 +351,15 @@ static void free_nested(struct kvm_vcpu *vcpu)
> >  	vmx->nested.current_vmptr = INVALID_GPA;
> >  	if (enable_shadow_vmcs) {
> >  		vmx_disable_shadow_vmcs(vmx);
> > -		vmcs_clear(vmx->vmcs01.shadow_vmcs);
> > -		free_vmcs(vmx->vmcs01.shadow_vmcs);
> > +
> > +		/*
> > +		 * Keep the pointer visible until after VMCLEAR, so migration
> > +		 * can clear an active shadow VMCS on the old CPU.
> > +		 */
> > +		shadow_vmcs = vmx->vmcs01.shadow_vmcs;
> > +		vmcs_clear(shadow_vmcs);
> >  		vmx->vmcs01.shadow_vmcs = NULL;
> 
> [Severity: High]
> Does this sequence need a compiler barrier (such as barrier() or WRITE_ONCE())
> to prevent the compiler from hoisting the NULL assignment above vmcs_clear()?
> 
> Because vmcs_clear() relies on an inline assembly block without a "memory"
> clobber and has no direct data dependency with vmx->vmcs01.shadow_vmcs, the
> compiler could legally reorder the store. 
> 
> If a preemption occurs right between the hoisted assignment and the VMCLEAR,
> and the vCPU migrates, vcpu_load() on the new CPU would IPI the old CPU to
> clear the loaded VMCS. The old CPU would see the NULL pointer and skip
> flushing, potentially leaving an active pointer to freed memory in its
> hardware cache.

FWIW, I had a pretty strong preference for Paolo's off-list suggestion of
explicitly disabling preemption, even before seeing Sashiko's response.  Every
other path the VMCLEARs the shadow VMCS runs with preemption disabled, IMO it's
more intuitive to follow suit here.  And even with the comment, it took a good
5-10 minutes of staring to understand what exactly is going wrong, though that's
probably a me problem.

> > +		free_vmcs(shadow_vmcs);
> >  	}
> >  	kfree(vmx->nested.cached_vmcs12);
> >  	vmx->nested.cached_vmcs12 = NULL;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260721102827.313208-1-pbonzini@redhat.com?part=1