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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing compiler barrier allows pointer nullification to be reordered before VMCLEAR, recreating the UAF.
--

--- Patch [1]: [PATCH] KVM: nVMX: Hide shadow VMCS right after VMCLEAR ---
commit 002acbf68f96812492b255bf63d17f12b9b5b24a
Author: Hyunwoo Kim <imv4bel@gmail.com>

KVM: nVMX: Hide shadow VMCS right after VMCLEAR

This commit updates free_nested() to keep the shadow VMCS pointer visible
until after VMCLEAR has completed. This change is intended to ensure that
if the vCPU migrates, the migration process can still clear an active shadow
VMCS on the old CPU.

> 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.

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