Re: [PATCH] KVM: nVMX: Put vmcs12 pages if nested VM-Enter fails due to invalid guest state

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch resolves a memory leak for the nested VM-Exit path but completely misses an identical leak on the adjacent nested VM-Fail path when `nested_vmx_check_controls_late` fails.
--

--- Patch [1]: [PATCH] KVM: nVMX: Put vmcs12 pages if nested VM-Enter fails due to invalid guest state ---
commit 6b002f51033edcc9544a018d075246a30742f7ef
Author: Sean Christopherson <seanjc@google.com>

KVM: nVMX: Put vmcs12 pages if nested VM-Enter fails due to invalid guest state

This commit adds a missing call to nested_put_vmcs12_pages() when KVM
synthesizes a nested VM-Exit due to invalid guest state. This prevents
a page leak if L1 retries VMLAUNCH/VMRESUME.

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -3733,6 +3733,8 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
>  	if (!from_vmentry)
>  		return NVMX_VMENTRY_VMEXIT;
>  
> +	nested_put_vmcs12_pages(vcpu);
> +
>  	load_vmcs12_host_state(vcpu, vmcs12);

[Severity: High]
This isn't a bug introduced by this patch, but does the fix miss an identical
leak on the adjacent nested VM-Fail path?

Earlier in nested_vmx_enter_non_root_mode(), pages are mapped by
nested_get_vmcs12_pages(). Immediately following that, if
nested_vmx_check_controls_late() fails, it returns NVMX_VMENTRY_VMFAIL:

arch/x86/kvm/vmx/nested.c:nested_vmx_enter_non_root_mode() {
	...
	if (from_vmentry) {
		if (unlikely(!nested_get_vmcs12_pages(vcpu))) {
			vmx_switch_vmcs(vcpu, &vmx->vmcs01);
			return NVMX_VMENTRY_KVM_INTERNAL_ERROR;
		}

		if (nested_vmx_check_controls_late(vcpu, vmcs12)) {
			vmx_switch_vmcs(vcpu, &vmx->vmcs01);
			return NVMX_VMENTRY_VMFAIL;
		}
	...
}

This early return bypasses the newly added nested_put_vmcs12_pages() cleanup
since it doesn't jump to the vmentry_fail_vmexit label.

If an L1 guest triggers VMLAUNCH with an invalid state, such as an invalid
tpr_threshold vs vtpr combination, and then retries the operation, does
__kvm_vcpu_map() unconditionally overwrite the map pointers without
releasing the old ones?

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

Could this sequence allow an untrusted L1 guest to exhaust host memory by
repeatedly leaking page references?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716074636.756689-1-pbonzini@redhat.com?part=1