[PATCH v2 0/3] KVM: SVM: Fix a (very) unlikely UAF for GA Log IRQs

[PATCH v2 0/3] KVM: SVM: Fix a (very) unlikely UAF for GA Log IRQs

[Sean Christopherson]

Fix a potential UAF due to freeing vCPUs while they're still reachable through
the global hashed list used to handle GA Log notifications.

v2:
 - Defer adding the VM to the GA Log list until a vCPU is created (KVM only
   needs GA Lot notifications to wake blocking vCPUs), so that removing the
   VM from the list can be done during pre-destroy without having to handle
   the scenario where VM creation fails at a later stage. [Sashiko, Xiao]
 - Drop the stable@ tag, as it's not clear to me that the bug can actually
   occur in the wild, whereas the changes themselves are somewhat risky.

v1: https://lore.kernel.org/all/20260625220933.3357733-1-seanjc@google.com

Sean Christopherson (3):
  KVM: SVM: Make kvm_x86_ops.vcpu_precreate() hook fully AVIC specific
  KVM: SVM: Do all per-VM AVIC initialization during vCPU precreation
    phase
  KVM: SVM: Remove VM from the GA Log notifier list before VM
    destruction

 arch/x86/kvm/svm/avic.c | 96 +++++++++++++++++++++++++++--------------
 arch/x86/kvm/svm/svm.c  | 16 ++-----
 arch/x86/kvm/svm/svm.h  |  3 +-
 3 files changed, 69 insertions(+), 46 deletions(-)


base-commit: a204badd8432f93b7e862e7dac6db0fe3d65f370
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 1/3] KVM: SVM: Make kvm_x86_ops.vcpu_precreate() hook fully AVIC specific

[Sean Christopherson]

In anticipation of deferring all per-VM AVIC initialization until a vCPU
is first created, move SVM's kvm_x86_ops.vcpu_precreate() hook into avic.c
as avic_vcpu_pre_create() and nullify the hook if AVIC is disabled (and
WARN if the hook is somehow invoked without AVIC enabled).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 13 +++++++++----
 arch/x86/kvm/svm/svm.c  |  8 ++------
 arch/x86/kvm/svm/svm.h  |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 58e493a80cb0..4a0a2dbd1687 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -293,13 +293,10 @@ static int avic_get_physical_id_table_order(struct kvm *kvm)
 	return get_order((__avic_get_max_physical_id(kvm, NULL) + 1) * sizeof(u64));
 }
 
-int avic_alloc_physical_id_table(struct kvm *kvm)
+static int avic_alloc_physical_id_table(struct kvm *kvm)
 {
 	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
 
-	if (!irqchip_in_kernel(kvm) || !enable_apicv)
-		return 0;
-
 	if (kvm_svm->avic_physical_id_table)
 		return 0;
 
@@ -311,6 +308,14 @@ int avic_alloc_physical_id_table(struct kvm *kvm)
 	return 0;
 }
 
+int avic_vcpu_precreate(struct kvm *kvm)
+{
+	if (!irqchip_in_kernel(kvm) || WARN_ON_ONCE(!enable_apicv))
+		return 0;
+
+	return avic_alloc_physical_id_table(kvm);
+}
+
 void avic_vm_destroy(struct kvm *kvm)
 {
 	unsigned long flags;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index ef69a51ab27f..a7d141f7e76c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1306,11 +1306,6 @@ void svm_switch_vmcb(struct vcpu_svm *svm, struct kvm_vmcb_info *target_vmcb)
 	svm->vmcb = target_vmcb->ptr;
 }
 
-static int svm_vcpu_precreate(struct kvm *kvm)
-{
-	return avic_alloc_physical_id_table(kvm);
-}
-
 static int svm_vcpu_create(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm;
@@ -5333,7 +5328,7 @@ struct kvm_x86_ops svm_x86_ops __initdata = {
 	.emergency_disable_virtualization_cpu = svm_emergency_disable_virtualization_cpu,
 	.has_emulated_msr = svm_has_emulated_msr,
 
-	.vcpu_precreate = svm_vcpu_precreate,
+	.vcpu_precreate = avic_vcpu_precreate,
 	.vcpu_create = svm_vcpu_create,
 	.vcpu_free = svm_vcpu_free,
 	.vcpu_reset = svm_vcpu_reset,
@@ -5712,6 +5707,7 @@ static __init int svm_hardware_setup(void)
 	enable_apicv = avic_hardware_setup();
 	if (!enable_apicv) {
 		enable_ipiv = false;
+		svm_x86_ops.vcpu_precreate = NULL;
 		svm_x86_ops.vcpu_blocking = NULL;
 		svm_x86_ops.vcpu_unblocking = NULL;
 		svm_x86_ops.vcpu_get_apicv_inhibit_reasons = NULL;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 716be21fba33..3c5459374969 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -945,7 +945,7 @@ extern struct kvm_x86_nested_ops svm_nested_ops;
 
 bool __init avic_hardware_setup(void);
 void avic_hardware_unsetup(void);
-int avic_alloc_physical_id_table(struct kvm *kvm);
+int avic_vcpu_precreate(struct kvm *kvm);
 void avic_vm_destroy(struct kvm *kvm);
 int avic_vm_init(struct kvm *kvm);
 void avic_init_vmcb(struct vcpu_svm *svm, struct vmcb *vmcb);
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 2/3] KVM: SVM: Do all per-VM AVIC initialization during vCPU precreation phase

[Sean Christopherson]

Move all per-VM AVIC initialization from VM creation to vCPU pre-creation,
i.e. defer allocating the logical ID table and adding the VM to the GA Log
list until vCPUs are created.  This will allow removing the VM from the GA
Log list before vCPUs are destroyed without needing yet another kvm_x86_ops
hook (.vm_pre_destroy() is very intentionally called if and only if VM
creation fully succeeds).

As a bonus, this re-unites physical and logic table allocation, and avoids
allocating a logical table in the unlikely scenario that userspace creates
a VM without an in-kernel local APIC.

Another bonus to hooking .vcpu_precreate() is that there is no need to
unwind on failure, as the VM has already been created, i.e. KVM will run
through all phases of VM destruction.  In fact, unwinding is undesirable,
as KVM tries to keep VM-wide behavior idempotent/sticky across creaton of
multiple vCPUs.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 94 +++++++++++++++++++++++++----------------
 arch/x86/kvm/svm/svm.c  |  6 ---
 2 files changed, 57 insertions(+), 43 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 4a0a2dbd1687..d71a2fed1a08 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -308,47 +308,32 @@ static int avic_alloc_physical_id_table(struct kvm *kvm)
 	return 0;
 }
 
-int avic_vcpu_precreate(struct kvm *kvm)
+static int avic_alloc_logical_id_table(struct kvm *kvm)
 {
-	if (!irqchip_in_kernel(kvm) || WARN_ON_ONCE(!enable_apicv))
-		return 0;
-
-	return avic_alloc_physical_id_table(kvm);
-}
-
-void avic_vm_destroy(struct kvm *kvm)
-{
-	unsigned long flags;
-	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
-
-	if (!enable_apicv)
-		return;
-
-	free_page((unsigned long)kvm_svm->avic_logical_id_table);
-	free_pages((unsigned long)kvm_svm->avic_physical_id_table,
-		   avic_get_physical_id_table_order(kvm));
-
-	spin_lock_irqsave(&svm_vm_data_hash_lock, flags);
-	hash_del(&kvm_svm->hnode);
-	spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
-}
-
-int avic_vm_init(struct kvm *kvm)
-{
-	unsigned long flags;
-	int err = -ENOMEM;
 	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
-	struct kvm_svm *k2;
-	u32 vm_id;
 
-	if (!enable_apicv)
+	if (kvm_svm->avic_logical_id_table)
 		return 0;
 
 	kvm_svm->avic_logical_id_table = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
 	if (!kvm_svm->avic_logical_id_table)
-		goto free_avic;
+		return -ENOMEM;
 
-	spin_lock_irqsave(&svm_vm_data_hash_lock, flags);
+	return 0;
+}
+
+static void avic_add_vm_to_ga_log_list(struct kvm *kvm)
+{
+	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
+	struct kvm_svm *k2;
+	u32 vm_id;
+
+	lockdep_assert_held(&kvm->lock);
+
+	if (kvm_svm->avic_vm_id)
+		return;
+
+	guard(spinlock_irqsave)(&svm_vm_data_hash_lock);
  again:
 	vm_id = next_vm_id = (next_vm_id + 1) & AVIC_VM_ID_MASK;
 	if (vm_id == 0) { /* id is 1-based, zero is not okay */
@@ -364,13 +349,48 @@ int avic_vm_init(struct kvm *kvm)
 	}
 	kvm_svm->avic_vm_id = vm_id;
 	hash_add(svm_vm_data_hash, &kvm_svm->hnode, kvm_svm->avic_vm_id);
-	spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
+}
 
+int avic_vcpu_precreate(struct kvm *kvm)
+{
+	int r;
+
+	if (!irqchip_in_kernel(kvm) || WARN_ON_ONCE(!enable_apicv))
+		return 0;
+
+	/*
+	 * Don't unwind on failure, all actions must be idempotent with respect
+	 * to creating multiple vCPUs, i.e. must persist until the VM is destroyed.
+	 */
+	r = avic_alloc_physical_id_table(kvm);
+	if (r)
+		return r;
+
+	r = avic_alloc_logical_id_table(kvm);
+	if (r)
+		return r;
+
+	avic_add_vm_to_ga_log_list(kvm);
 	return 0;
+}
 
-free_avic:
-	avic_vm_destroy(kvm);
-	return err;
+void avic_vm_destroy(struct kvm *kvm)
+{
+	unsigned long flags;
+	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
+
+	if (!enable_apicv)
+		return;
+
+	free_page((unsigned long)kvm_svm->avic_logical_id_table);
+	free_pages((unsigned long)kvm_svm->avic_physical_id_table,
+		   avic_get_physical_id_table_order(kvm));
+
+	if (kvm_svm->avic_vm_id) {
+		spin_lock_irqsave(&svm_vm_data_hash_lock, flags);
+		hash_del(&kvm_svm->hnode);
+		spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
+	}
 }
 
 static phys_addr_t avic_get_backing_page_address(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index a7d141f7e76c..7f3a815d737f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5297,12 +5297,6 @@ static int svm_vm_init(struct kvm *kvm)
 	if (!pause_filter_count || !pause_filter_thresh)
 		kvm_disable_exits(kvm, KVM_X86_DISABLE_EXITS_PAUSE);
 
-	if (enable_apicv) {
-		int ret = avic_vm_init(kvm);
-		if (ret)
-			return ret;
-	}
-
 	svm_srso_vm_init();
 	return 0;
 }
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 3/3] KVM: SVM: Remove VM from the GA Log notifier list before VM destruction

[Sean Christopherson]

When a VM is being destroyed, delete it from the list used to process GA
Log interrupts before vCPUs are freed, otherwise avic_ga_log_notifier()
could theoretically hit a use-after-free if a GA Log notification arrives
for a vCPU after the last reference to the VM has been put.

Note, in practice, it's likely all but impossible to trigger UAF, as all
all irqfds and thus all IRTEs are cleaned up by:

  kvm_irqfd_release()
  |
  |-> irqfd_deactivate()
      |
      |-> irqfd_shutdown()
          |
          |-> irq_bypass_unregister_consumer()

And kvm_irqfd_release() is guaranteed to run before the last reference to
the VM is put.  KVM also configures GA Log interrupts only when a vCPU is
blocking (older versions of KVM configre GA Log interrupts at all times,
but AVIC is off by default on those kernels).  Hitting UAF would require
tearing down a VM shortly after a vCPU stopped blocking, and with a very,
very delayed IRQ from hardware.

Opportunistically use guard() to avoid a local "flags" variable.

Fixes: 5881f73757cc ("svm: Introduce AMD IOMMU avic_ga_log_notifier")
Cc: Naveen N Rao (AMD) <naveen@kernel.org>
Cc: Xiao Wu <xiaowu.417@qq.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 19 ++++++++++++-------
 arch/x86/kvm/svm/svm.c  |  2 ++
 arch/x86/kvm/svm/svm.h  |  1 +
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index d71a2fed1a08..c5b1d294b15a 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -374,9 +374,20 @@ int avic_vcpu_precreate(struct kvm *kvm)
 	return 0;
 }
 
+void avic_vm_pre_destroy(struct kvm *kvm)
+{
+	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
+
+	if (WARN_ON_ONCE(!enable_apicv) || !kvm_svm->avic_vm_id)
+		return;
+
+	guard(spinlock_irqsave)(&svm_vm_data_hash_lock);
+
+	hash_del(&kvm_svm->hnode);
+}
+
 void avic_vm_destroy(struct kvm *kvm)
 {
-	unsigned long flags;
 	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
 
 	if (!enable_apicv)
@@ -385,12 +396,6 @@ void avic_vm_destroy(struct kvm *kvm)
 	free_page((unsigned long)kvm_svm->avic_logical_id_table);
 	free_pages((unsigned long)kvm_svm->avic_physical_id_table,
 		   avic_get_physical_id_table_order(kvm));
-
-	if (kvm_svm->avic_vm_id) {
-		spin_lock_irqsave(&svm_vm_data_hash_lock, flags);
-		hash_del(&kvm_svm->hnode);
-		spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
-	}
 }
 
 static phys_addr_t avic_get_backing_page_address(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7f3a815d737f..0e0dd9618750 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5329,6 +5329,7 @@ struct kvm_x86_ops svm_x86_ops __initdata = {
 
 	.vm_size = sizeof(struct kvm_svm),
 	.vm_init = svm_vm_init,
+	.vm_pre_destroy = avic_vm_pre_destroy,
 	.vm_destroy = svm_vm_destroy,
 
 	.prepare_switch_to_guest = svm_prepare_switch_to_guest,
@@ -5702,6 +5703,7 @@ static __init int svm_hardware_setup(void)
 	if (!enable_apicv) {
 		enable_ipiv = false;
 		svm_x86_ops.vcpu_precreate = NULL;
+		svm_x86_ops.vm_pre_destroy = NULL;
 		svm_x86_ops.vcpu_blocking = NULL;
 		svm_x86_ops.vcpu_unblocking = NULL;
 		svm_x86_ops.vcpu_get_apicv_inhibit_reasons = NULL;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 3c5459374969..616e45624f4c 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -946,6 +946,7 @@ extern struct kvm_x86_nested_ops svm_nested_ops;
 bool __init avic_hardware_setup(void);
 void avic_hardware_unsetup(void);
 int avic_vcpu_precreate(struct kvm *kvm);
+void avic_vm_pre_destroy(struct kvm *kvm);
 void avic_vm_destroy(struct kvm *kvm);
 int avic_vm_init(struct kvm *kvm);
 void avic_init_vmcb(struct vcpu_svm *svm, struct vmcb *vmcb);
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 1/3] KVM: SVM: Make kvm_x86_ops.vcpu_precreate() hook fully AVIC specific

[Naveen N Rao]

On Tue, Jun 30, 2026 at 02:01:54PM -0700, Sean Christopherson wrote:
> In anticipation of deferring all per-VM AVIC initialization until a vCPU
> is first created, move SVM's kvm_x86_ops.vcpu_precreate() hook into avic.c
> as avic_vcpu_pre_create() and nullify the hook if AVIC is disabled (and
     ^^^^^^^^^^^^^^^^^^^^ avic_vcpu_precreate()

> WARN if the hook is somehow invoked without AVIC enabled).
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 13 +++++++++----
>  arch/x86/kvm/svm/svm.c  |  8 ++------
>  arch/x86/kvm/svm/svm.h  |  2 +-
>  3 files changed, 12 insertions(+), 11 deletions(-)

Reviewed-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen

Re: [PATCH v2 2/3] KVM: SVM: Do all per-VM AVIC initialization during vCPU precreation phase

[Naveen N Rao]

On Tue, Jun 30, 2026 at 02:01:55PM -0700, Sean Christopherson wrote:
> Move all per-VM AVIC initialization from VM creation to vCPU pre-creation,
> i.e. defer allocating the logical ID table and adding the VM to the GA Log
> list until vCPUs are created.  This will allow removing the VM from the GA
> Log list before vCPUs are destroyed without needing yet another kvm_x86_ops
> hook (.vm_pre_destroy() is very intentionally called if and only if VM
> creation fully succeeds).
> 
> As a bonus, this re-unites physical and logic table allocation, and avoids
> allocating a logical table in the unlikely scenario that userspace creates
> a VM without an in-kernel local APIC.
> 
> Another bonus to hooking .vcpu_precreate() is that there is no need to
> unwind on failure, as the VM has already been created, i.e. KVM will run
> through all phases of VM destruction.  In fact, unwinding is undesirable,
> as KVM tries to keep VM-wide behavior idempotent/sticky across creaton of
> multiple vCPUs.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 94 +++++++++++++++++++++++++----------------
>  arch/x86/kvm/svm/svm.c  |  6 ---
>  2 files changed, 57 insertions(+), 43 deletions(-)

This takes the cleanup one step ahead by removing avic_vm_init(). Good!

With the prototype for avic_vm_init() removed:
Reviewed-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen

Re: [PATCH v2 3/3] KVM: SVM: Remove VM from the GA Log notifier list before VM destruction

[Naveen N Rao]

On Tue, Jun 30, 2026 at 02:01:56PM -0700, Sean Christopherson wrote:
> When a VM is being destroyed, delete it from the list used to process GA
> Log interrupts before vCPUs are freed, otherwise avic_ga_log_notifier()
> could theoretically hit a use-after-free if a GA Log notification arrives
> for a vCPU after the last reference to the VM has been put.
> 
> Note, in practice, it's likely all but impossible to trigger UAF, as all
> all irqfds and thus all IRTEs are cleaned up by:
> 
>   kvm_irqfd_release()
>   |
>   |-> irqfd_deactivate()
>       |
>       |-> irqfd_shutdown()
>           |
>           |-> irq_bypass_unregister_consumer()
> 
> And kvm_irqfd_release() is guaranteed to run before the last reference to
> the VM is put.  KVM also configures GA Log interrupts only when a vCPU is
> blocking (older versions of KVM configre GA Log interrupts at all times,
> but AVIC is off by default on those kernels).  Hitting UAF would require
> tearing down a VM shortly after a vCPU stopped blocking, and with a very,
> very delayed IRQ from hardware.
> 
> Opportunistically use guard() to avoid a local "flags" variable.
> 
> Fixes: 5881f73757cc ("svm: Introduce AMD IOMMU avic_ga_log_notifier")
> Cc: Naveen N Rao (AMD) <naveen@kernel.org>
> Cc: Xiao Wu <xiaowu.417@qq.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 19 ++++++++++++-------
>  arch/x86/kvm/svm/svm.c  |  2 ++
>  arch/x86/kvm/svm/svm.h  |  1 +
>  3 files changed, 15 insertions(+), 7 deletions(-)

LGTM.
Reviewed-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen