Re: [PATCH v2 2/2] KVM: VMX: Fix stale PID-pointer table entry left after vCPU free

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Persistent Use-After-Free of AVIC backing page due to dangling pointer in `avic_physical_id_table` when SVM vCPU creation fails.
- [High] Transient Use-After-Free of vcpu memory due to missing hardware cache invalidation (VPID) and synchronization during VMX vCPU destruction.
--

--- Patch [2]: [PATCH v2 2/2] KVM: VMX: Fix stale PID-pointer table entry left after vCPU free ---
commit a1d19d2d3c1c1e5aad00c7f77f6736e2bfb89509
Author: Dmytro Maluka <dmaluka@chromium.org>

KVM: VMX: Fix stale PID-pointer table entry left after vCPU free

vCPU creation in kvm_vm_ioctl_create_vcpu() may fail after
kvm_arch_vcpu_create() -> vmx_vcpu_create() already succeeded. In such
case kvm_vm_ioctl_create_vcpu() destroys the newly created vCPU in the
failure path. However, that leaves a side effect: the IPIv pid_table
entry remains configured with this vCPU's pi_desc address.

Link: https://lore.kernel.org/kvm/al4rNqpBYy8FGKPw@blrnaveerao1/

> Note that the same issue exists for SVM AVIC as well [1], to be fixed.

[Severity: Critical]
This is a pre-existing issue, but does SVM vCPU destruction leave a dangling
pointer in avic_physical_id_table?

During KVM_CREATE_VCPU, avic_init_backing_page() sets
avic_physical_id_table[id] to point to the newly allocated AVIC backing page
and marks it valid.

If vCPU creation later fails in kvm_vm_ioctl_create_vcpu(), svm_vcpu_free() is
called but fails to clear this physical ID table entry. kvm_arch_vcpu_destroy()
then calls kvm_free_lapic(), which uses free_page() to release the backing page.

The avic_physical_id_table retains a valid physical pointer to this freed
page. If a guest sends an IPI to this freed vCPU ID, could the SVM AVIC
hardware use the stale table entry to perform memory writes on the freed page,
causing host kernel memory corruption?

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 6b96f7aea20bd..ebf26ea84a50b 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7572,6 +7572,9 @@ void vmx_vcpu_free(struct kvm_vcpu *vcpu)
>  	nested_vmx_free_vcpu(vcpu);
>  	free_loaded_vmcs(vmx->loaded_vmcs);
>  	free_page((unsigned long)vmx->ve_info);
> +
> +	if (vmx_can_use_ipiv(vcpu))
> +		WRITE_ONCE(to_kvm_vmx(vcpu->kvm)->pid_table[vcpu->vcpu_id], 0);

[Severity: High]
This isn't a bug introduced by this patch, but does clearing the pid_table
entry alone prevent hardware from writing to the freed pi_desc?

According to the Intel SDM (Section 29.1.5), the processor caches PID-pointer
table entries in the VPID.

Because KVM immediately frees the vcpu memory into kvm_vcpu_cache after
kvm_arch_vcpu_destroy() without issuing a cross-vCPU flush (e.g., via
kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH)) to invalidate the VPID
caches of other running vCPUs and synchronize with in-flight IPI deliveries,
can other vCPUs still send hardware IPIs using the cached pi_desc pointer?

Could this result in the IPI hardware writing to the freed vcpu memory and
corrupting another newly created vCPU when the memory is reused from
kvm_vcpu_cache?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729170621.308809-1-dmaluka@chromium.org?part=2