Re: [PATCH] KVM: nVMX: Don't flush shadow VMCS12 to guest memory during vCPU teardown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Live migration during SMM with an active nested guest overwrites L2 state (e.g. RIP/RSP) with stale data from the hardware shadow VMCS.
- [High] Live migration during SMM with nested shadow VMCS enabled causes a Triple Fault on the destination.
- [High] During vCPU teardown, __nested_vmx_vmexit() unconditionally loads L1 host state, triggering a cross-process memory read via load_pdptrs() with a borrowed active_mm.
--

commit 15d3183acc0011b1c1063bab25d832a346f7a45c
Author: Jim Mattson <jmattson@google.com>

KVM: nVMX: Don't flush shadow VMCS12 to guest memory during vCPU teardown

When a vCPU is destroyed while L2 is active, KVM synthesizes a nested VM-Exit.
During this process exit, the vCPU destruction runs with a borrowed lazy TLB 
active_mm, so skipping the shadow VMCS12 flush prevents __copy_to_user() 
from corrupting an unrelated task's memory.

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 151873407abd3..33dc8a07019f8 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -5121,21 +5121,28 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
>  	if (likely(!vmx->fail)) {
>  		sync_vmcs02_to_vmcs12(vcpu, vmcs12);
>  
> -		if (vm_exit_reason != -1)
> +		if (vm_exit_reason != -1) {
>  			prepare_vmcs12(vcpu, vmcs12, vm_exit_reason,
>  				       exit_intr_info, exit_qualification,
>  				       exit_insn_len);
>  
> -		/*
> -		 * Must happen outside of sync_vmcs02_to_vmcs12() as it will
> -		 * also be used to capture vmcs12 cache as part of
> -		 * capturing nVMX state for snapshot (migration).
> -		 *
> -		 * Otherwise, this flush will dirty guest memory at a
> -		 * point it is already assumed by user-space to be
> -		 * immutable.
> -		 */
> -		nested_flush_cached_shadow_vmcs12(vcpu, vmcs12);
> +			/*
> +			 * Must happen outside of sync_vmcs02_to_vmcs12() as it will
> +			 * also be used to capture vmcs12 cache as part of
> +			 * capturing nVMX state for snapshot (migration).
> +			 *
> +			 * Otherwise, this flush will dirty guest memory at a
> +			 * point it is already assumed by user-space to be
> +			 * immutable.
> +			 *
> +			 * Do not flush when vm_exit_reason == -1 (e.g. forced exit
> +			 * on vCPU teardown or SMM entry). On teardown, KVM may run
> +			 * with current->mm == NULL on a borrowed lazy TLB mm,
> +			 * where writing guest memory would corrupt an unrelated
> +			 * task. Nothing can observe the flush in that case anyway.
> +			 */
> +			nested_flush_cached_shadow_vmcs12(vcpu, vmcs12);
> +		}
>  	} else {
>  		/*
>  		 * The only expected VM-instruction error is "VM entry with

[Severity: High]
This is a pre-existing issue, but since this patch handles vCPU teardown with
a borrowed active_mm, does the later call to load_vmcs12_host_state() in
__nested_vmx_vmexit() have a similar issue?

During vCPU teardown, __nested_vmx_vmexit() unconditionally loads L1 host
state:

arch/x86/kvm/vmx/nested.c:__nested_vmx_vmexit() {
    ...
    load_vmcs12_host_state(vcpu, vmcs12);
    ...
}

If the L1 host state uses PAE paging, nested_vmx_load_cr3() triggers the
PDPTR load:

arch/x86/kvm/vmx/nested.c:nested_vmx_load_cr3() {
    ...
    if (reload_pdptrs && !nested_ept && is_pae_paging(vcpu) &&
        CC(!load_pdptrs(vcpu, cr3))) {
    ...
}

This executes a memory read using copy_from_user() in the context of the
borrowed active_mm:

arch/x86/kvm/regs.c:load_pdptrs() {
    ...
    ret = kvm_vcpu_read_guest_page(vcpu, gpa_to_gfn(real_gpa), pdpte,
                                   cr3 & GENMASK(11, 5), sizeof(pdpte));
    ...
}

Could this cross-process memory read cause destructive side-effects if it hits
a memory-mapped I/O (MMIO) region in the unrelated task?


[Severity: Critical]
This is a pre-existing issue, but does skipping the shadow VMCS12 flush on
SMM entry (where vm_exit_reason == -1) cause data corruption during live
migration?

In __nested_vmx_vmexit(), need_vmcs12_to_shadow_sync is prevented from being
set on SMM entry because of the vm_exit_reason check:

arch/x86/kvm/vmx/nested.c:__nested_vmx_vmexit() {
    ...
    if ((vm_exit_reason != -1) &&
        (enable_shadow_vmcs || nested_vmx_is_evmptr12_valid(vmx)))
        vmx->nested.need_vmcs12_to_shadow_sync = true;
    ...
}

If live migration is initiated while in SMM, vmx_get_nested_state() treats
the hardware shadow VMCS as authoritative:

arch/x86/kvm/vmx/nested.c:vmx_get_nested_state() {
    ...
    else if (enable_shadow_vmcs)
        copy_shadow_to_vmcs12(vmx);
    ...
}

Since need_vmcs12_to_shadow_sync was not set, doesn't this overwrite the
correctly synced vmcs12 with stale data from the hardware shadow VMCS,
resulting in guest state rollback (e.g., RIP/RSP)?


[Severity: High]
This isn't a bug introduced by this patch, but is there a Triple Fault risk
during live migration in SMM with nested shadow VMCS enabled?

Because is_guest_mode is false in SMM, the shadow_vmcs12 is omitted from the
migration stream:

arch/x86/kvm/vmx/nested.c:vmx_get_nested_state() {
    ...
    if (is_guest_mode(vcpu) &&
        nested_cpu_has_shadow_vmcs(vmcs12) &&
        vmcs12->vmcs_link_pointer != INVALID_GPA)
        kvm_state.size += sizeof(user_vmx_nested_state->shadow_vmcs12);
    ...
}

On the destination, vmx_set_nested_state() skips restoring shadow_vmcs12 and
returns early:

arch/x86/kvm/vmx/nested.c:vmx_set_nested_state() {
    ...
    if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE))
        return 0;
    ...
}

When the guest leaves SMM, vmx_leave_smm() checks the revision ID of the
zeroed shadow_vmcs12:

arch/x86/kvm/vmx/vmx.c:vmx_leave_smm() {
    ...
    /* Triple fault if the state is invalid.  */
    if (nested_vmx_check_restored_vmcs12(vcpu) < 0)
        return 1;
    ...
}

Could this zeroed buffer fail the revision ID check and induce a Triple Fault?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908132838.2116068-1-jmattson@google.com?part=1