Re: [PATCH 1/6] KVM: nVMX: Ensure KVM_REQ_GET_NESTED_STATE_PAGES is cleared on VM-Exit

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] An L1 guest can trigger a silent host memory leak (pinned pages and kmap space) by repeatedly executing VMLAUNCH with an invalid guest state.
--

--- Patch [1]: [PATCH 1/6] KVM: nVMX: Ensure KVM_REQ_GET_NESTED_STATE_PAGES is cleared on VM-Exit ---
commit 529c03d57fc94827c8dfbb4372dfae1313c5dd93
Author: Sean Christopherson <seanjc@google.com>

KVM: nVMX: Ensure KVM_REQ_GET_NESTED_STATE_PAGES is cleared on VM-Exit

This patch ensures KVM_REQ_GET_NESTED_STATE_PAGES is always checked and 
cleared when emulating a nested VM-Exit. This prevents double-mapping 
vmcs12 pages if KVM manages to bail from VM-Enter without processing the 
request and then emulates VMLAUNCH or VMRESUME.

Cc: stable@vger.kernel.org

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index b5460de4b1a72..f2518d67e7de5 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -5066,8 +5066,9 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
>  	/* trying to cancel vmlaunch/vmresume is a bug */
>  	kvm_warn_on_nested_run_pending(vcpu);
>  
> -#ifdef CONFIG_KVM_HYPERV
> +	/* Note, "checking" the request also clears the request. */
>  	if (kvm_check_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu)) {
> +#ifdef CONFIG_KVM_HYPERV

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential memory
leak related to how nested VM-Entry failures handle mapped pages.

If an L1 guest prepares a VMCS12 with valid addresses for mapped pages (like
virtual_apic_page_addr) but intentionally configures an invalid guest state 
(e.g., an invalid CR0 value), nested_vmx_enter_non_root_mode() maps the 
pages before validating the guest state:

arch/x86/kvm/vmx/nested.c:nested_vmx_enter_non_root_mode() {
    ...
    if (nested_vmx_check_guest_state(vcpu, vmcs12,
                                     &entry_failure_code)) {
        exit_reason.basic = EXIT_REASON_INVALID_STATE;
        vmcs12->exit_qualification = entry_failure_code;
        goto vmentry_fail_vmexit;
    }
    ...
}

The error path in vmentry_fail_vmexit synthesizes a VM-Exit back to L1 but 
returns without calling nested_put_vmcs12_pages() to unmap the resources:

arch/x86/kvm/vmx/nested.c:vmentry_fail_vmexit() {
    ...
    if (!from_vmentry)
        return NVMX_VMENTRY_VMEXIT;

    load_vmcs12_host_state(vcpu, vmcs12);
    vmcs12->vm_exit_reason = exit_reason.full;
    ...
    return NVMX_VMENTRY_VMEXIT;
}

If the L1 guest resumes and executes VMLAUNCH again, KVM calls
kvm_vcpu_map() on the already populated map structures:

virt/kvm/kvm_main.c:__kvm_vcpu_map() {
    ...
    map->pinned_page = NULL;
    map->page = NULL;
    map->hva = NULL;
    ...
}

This unconditionally clears the pointers without unmapping or verifying the
previous state, dropping the only KVM reference to the previously pinned
host page and kmap address.

Could a malicious L1 guest repeatedly trigger this in a tight loop to
exhaust host memory and kmap virtual space?

>  		/*
>  		 * KVM_REQ_GET_NESTED_STATE_PAGES is also used to map
>  		 * Enlightened VMCS after migration and we still need to

[ ... ]

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