[PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation

[PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation

[Dmytro Maluka]

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. As a result,
when another vCPU sends an IPI to the APIC ID of this failed-to-create
vCPU, it will cause HW to write to this (freed!) pi_desc memory. [*]

An easy fix would be to clear the pid_table entry in vmx_vcpu_free().
However that would be still problematic, for the following reason:
userspace may try to create a vCPU with the same vcpu_id as an existing
one; vmx_vcpu_create() will succeed, and only after that
kvm_vm_ioctl_create_vcpu() will check for the duplicate vcpu_id and
fail with -EEXIST and then free the vCPU in the failure path. So in this
failure path, vmx_vcpu_free() would clear the pid_table entry for that
already existing good vCPU, i.e. effectively disable IPIv for that vCPU.

So instead fix the issue by moving the pid_table entry setup from
.vcpu_create() to the newly introduced .vcpu_postcreate() callback
called at the end of kvm_vm_ioctl_create_vcpu(), when we are sure that
the vCPU creation succeeded.

[*] Although, since this memory is freed into the kvm_vcpu_cache kmem
    cache which is only used for allocating kvm_vcpus, _maybe_ this
    memory will only be reused for pi_desc of another vCPU, not for
    anything else. So _maybe_ this will only result in delivering the
    IPI to a wrong vCPU (possibly of another VM) in the worst case, not
    in a random corruption of kernel memory.

Fixes: d588bb9be1da ("KVM: VMX: enable IPI virtualization")
Signed-off-by: Dmytro Maluka <dmaluka@chromium.org>
---
 arch/x86/include/asm/kvm-x86-ops.h |  1 +
 arch/x86/include/asm/kvm_host.h    |  1 +
 arch/x86/kvm/vmx/main.c            |  9 +++++++++
 arch/x86/kvm/vmx/vmx.c             | 11 +++++++----
 arch/x86/kvm/vmx/x86_ops.h         |  1 +
 arch/x86/kvm/x86.c                 |  1 +
 6 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 83dc5086138b..aecbe9c54004 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -25,6 +25,7 @@ KVM_X86_OP_OPTIONAL(vm_destroy)
 KVM_X86_OP_OPTIONAL(vm_pre_destroy)
 KVM_X86_OP_OPTIONAL_RET0(vcpu_precreate)
 KVM_X86_OP(vcpu_create)
+KVM_X86_OP_OPTIONAL(vcpu_postcreate)
 KVM_X86_OP(vcpu_free)
 KVM_X86_OP(vcpu_reset)
 KVM_X86_OP(prepare_switch_to_guest)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 5f6c1ce9673b..3ed9d8fc3b96 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1803,6 +1803,7 @@ struct kvm_x86_ops {
 	/* Create, but do not attach this VCPU */
 	int (*vcpu_precreate)(struct kvm *kvm);
 	int (*vcpu_create)(struct kvm_vcpu *vcpu);
+	void (*vcpu_postcreate)(struct kvm_vcpu *vcpu);
 	void (*vcpu_free)(struct kvm_vcpu *vcpu);
 	void (*vcpu_reset)(struct kvm_vcpu *vcpu, bool init_event);
 
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index 83d9921277ea..bdf5ba3155f6 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -78,6 +78,14 @@ static int vt_vcpu_create(struct kvm_vcpu *vcpu)
 	return vmx_vcpu_create(vcpu);
 }
 
+static void vt_vcpu_postcreate(struct kvm_vcpu *vcpu)
+{
+	if (is_td_vcpu(vcpu))
+		return;
+
+	vmx_vcpu_postcreate(vcpu);
+}
+
 static void vt_vcpu_free(struct kvm_vcpu *vcpu)
 {
 	if (is_td_vcpu(vcpu)) {
@@ -898,6 +906,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
 
 	.vcpu_precreate = vt_op(vcpu_precreate),
 	.vcpu_create = vt_op(vcpu_create),
+	.vcpu_postcreate = vt_op(vcpu_postcreate),
 	.vcpu_free = vt_op(vcpu_free),
 	.vcpu_reset = vt_op(vcpu_reset),
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index cc75feec05da..ae6cef9c13e0 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7752,10 +7752,6 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu)
 		vmx->ve_info = page_to_virt(page);
 	}
 
-	if (vmx_can_use_ipiv(vcpu))
-		WRITE_ONCE(to_kvm_vmx(vcpu->kvm)->pid_table[vcpu->vcpu_id],
-			   __pa(&vmx->vt.pi_desc) | PID_TABLE_ENTRY_VALID);
-
 	return 0;
 
 free_vmcs:
@@ -7767,6 +7763,13 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu)
 	return err;
 }
 
+void vmx_vcpu_postcreate(struct kvm_vcpu *vcpu)
+{
+	if (vmx_can_use_ipiv(vcpu))
+		WRITE_ONCE(to_kvm_vmx(vcpu->kvm)->pid_table[vcpu->vcpu_id],
+			   __pa(&to_vmx(vcpu)->vt.pi_desc) | PID_TABLE_ENTRY_VALID);
+}
+
 #define L1TF_MSG_SMT "L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n"
 #define L1TF_MSG_L1D "L1TF CPU bug present and virtualization mitigation disabled, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n"
 
diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h
index 409858074246..defc2c3a37c8 100644
--- a/arch/x86/kvm/vmx/x86_ops.h
+++ b/arch/x86/kvm/vmx/x86_ops.h
@@ -21,6 +21,7 @@ int vmx_vm_init(struct kvm *kvm);
 void vmx_vm_destroy(struct kvm *kvm);
 int vmx_vcpu_precreate(struct kvm *kvm);
 int vmx_vcpu_create(struct kvm_vcpu *vcpu);
+void vmx_vcpu_postcreate(struct kvm_vcpu *vcpu);
 int vmx_vcpu_pre_run(struct kvm_vcpu *vcpu);
 fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags);
 void vmx_vcpu_free(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index afcac1042947..c9f512a30fec 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12873,6 +12873,7 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 		return;
 	vcpu_load(vcpu);
 	kvm_synchronize_tsc(vcpu, NULL);
+	kvm_x86_call(vcpu_postcreate)(vcpu);
 	vcpu_put(vcpu);
 
 	/* poll control enabled by default */
-- 
2.55.0.141.g00534a21ce-goog

Re: [PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation

[sashiko-bot]

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

Pre-existing issues:
- [Critical] The patch fixes a use-after-free in VMX IPI virtualization caused by KVM_CREATE_VCPU destroying a vCPU on duplicate ID, but fails to fix the exact same issue in AMD SVM AVIC.
--

--- Patch [1]: [PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation ---
commit c59610ef1e5c50c0016e2a7b4db23402a3147ffa
Author: Dmytro Maluka <dmaluka@chromium.org>

KVM: VMX: Postpone IPIv setup after successful vCPU creation

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.

> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index de709fb5bd76a..6a731cab9fb31 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -24,6 +24,7 @@ KVM_X86_OP_OPTIONAL(vm_destroy)
>  KVM_X86_OP_OPTIONAL(vm_pre_destroy)
>  KVM_X86_OP_OPTIONAL_RET0(vcpu_precreate)
>  KVM_X86_OP(vcpu_create)
> +KVM_X86_OP_OPTIONAL(vcpu_postcreate)

[Severity: Critical]
This isn't a bug introduced by this patch, but does this exact same
vulnerability exist in the AMD SVM AVIC code?

It looks like the SVM code suffers from the same use-after-free pattern
when userspace creates a duplicate vCPU ID.

In arch/x86/kvm/svm/avic.c:avic_init_backing_page(), the newly allocated
APIC backing page address is written into the VM-wide hardware IPI table
before the vCPU ID duplicate check:

    WRITE_ONCE(kvm_svm->avic_physical_id_table[id], new_entry);

If the vCPU ID check later fails in kvm_vm_ioctl_create_vcpu(), the
new vCPU is destroyed and kvm_free_lapic() frees the backing page:

    free_page((unsigned long)apic->regs);

However, the AVIC table is never updated to revert the overwrite.
This leaves the physical address of the freed page in the hardware table,
which could cause a hardware use-after-free if a guest subsequently sends
an IPI to that ID.

Could we also implement vcpu_postcreate for SVM to postpone the AVIC
table update, or do we need a separate fix for AMD?

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

Re: [PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation

[Dmytro Maluka]

On Thu, Jul 16, 2026 at 04:29:00PM +0000, sashiko-bot@kernel.org wrote:
> [Severity: Critical]
> This isn't a bug introduced by this patch, but does this exact same
> vulnerability exist in the AMD SVM AVIC code?
> 
> It looks like the SVM code suffers from the same use-after-free pattern
> when userspace creates a duplicate vCPU ID.

Seems plausible. Can someone with AMD expertise look into this?

> In arch/x86/kvm/svm/avic.c:avic_init_backing_page(), the newly allocated
> APIC backing page address is written into the VM-wide hardware IPI table
> before the vCPU ID duplicate check:
> 
>     WRITE_ONCE(kvm_svm->avic_physical_id_table[id], new_entry);
> 
> If the vCPU ID check later fails in kvm_vm_ioctl_create_vcpu(), the
> new vCPU is destroyed and kvm_free_lapic() frees the backing page:
> 
>     free_page((unsigned long)apic->regs);
> 
> However, the AVIC table is never updated to revert the overwrite.
> This leaves the physical address of the freed page in the hardware table,
> which could cause a hardware use-after-free if a guest subsequently sends
> an IPI to that ID.
> 
> Could we also implement vcpu_postcreate for SVM to postpone the AVIC
> table update, or do we need a separate fix for AMD?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260716160801.3155582-1-dmaluka@chromium.org?part=1

Re: [PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation

[Huang, Kai]

> 
> An easy fix would be to clear the pid_table entry in vmx_vcpu_free().
> However that would be still problematic, for the following reason:
> userspace may try to create a vCPU with the same vcpu_id as an existing
> one; vmx_vcpu_create() will succeed, and only after that
> kvm_vm_ioctl_create_vcpu() will check for the duplicate vcpu_id and
> fail with -EEXIST and then free the vCPU in the failure path. So in this
> failure path, vmx_vcpu_free() would clear the pid_table entry for that
> already existing good vCPU, i.e. effectively disable IPIv for that vCPU.

IMHO this seems a bit fragile?  If something similar to pid_table coming up in
the future, we could end up with a similar problem.

The "duplicated vcpu_id check" seems the ones that should happen as early as
possible.  Is it better to move the "duplicated vcpu_id check" earlier before
vmx_vcpu_create(), e.g., even before the kvm_arch_vcpu_precreate()?  In this
case we can do the pid_table cleanup in vmx_vcpu_free() I think.

The downside is this is common KVM code change for all archs, but it doesn't
seem this will bring problem to other archs.

Re: [PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation

[Dmytro Maluka]

On Fri, Jul 17, 2026 at 12:59:38PM +0000, Huang, Kai wrote:
> > 
> > An easy fix would be to clear the pid_table entry in vmx_vcpu_free().
> > However that would be still problematic, for the following reason:
> > userspace may try to create a vCPU with the same vcpu_id as an existing
> > one; vmx_vcpu_create() will succeed, and only after that
> > kvm_vm_ioctl_create_vcpu() will check for the duplicate vcpu_id and
> > fail with -EEXIST and then free the vCPU in the failure path. So in this
> > failure path, vmx_vcpu_free() would clear the pid_table entry for that
> > already existing good vCPU, i.e. effectively disable IPIv for that vCPU.
> 
> IMHO this seems a bit fragile?  If something similar to pid_table coming up in
> the future, we could end up with a similar problem.
> 
> The "duplicated vcpu_id check" seems the ones that should happen as early as
> possible.  Is it better to move the "duplicated vcpu_id check" earlier before
> vmx_vcpu_create(), e.g., even before the kvm_arch_vcpu_precreate()?  In this
> case we can do the pid_table cleanup in vmx_vcpu_free() I think.

Unfortunately it is not that simple. As I understand, the reason why the
"duplicated vcpu_id check" is done later is that it needs to be done
atomically together with inserting the vCPU into kvm->vcpu_array after
it, i.e. both the check and the insertion need to be done with kvm->lock
held (rather than releasing kvm->lock and taking it again between the
two operations).

IOW, if we just move the "duplicated vcpu_id check" earlier (before the
first unlock of kvm->lock), we have a race:

1. vCPU A is being created but not installed in kvm->vcpu_array yet.
2. vCPU B with the same vcpu_id is being created. It passes the
   duplicated vcpu_id check, since the check doesn't find vCPU A in
   kvm->vcpu_array.
3. vCPU A is installed in kvm->vcpu_array, vCPU creation succeeds.
4. vCPU B with the same vcpu_id is installed in kvm->vcpu_array, vCPU
   creation succeeds.

And we cannot just move the kvm->vcpu_array insertion earlier (before
the first unlock of kvm->lock), at least because the vCPU is not even
allocated at that point.

I guess we could track the used vcpu_ids separately in another xarray
(or a bitmap) which could be checked and updated by the early
"duplicated vcpu_id check" before the first unlock of kvm->lock. But do
we want to pay the memory price for that?