[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

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

[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The function prototype `avic_vm_init(struct kvm *kvm)` remains in `arch/x86/kvm/svm/svm.h` despite its implementation being removed.
--

commit e515c341adc5a84997456546a40c3f72c03c42b6
Author: Sean Christopherson <seanjc@google.com>

    KVM: SVM: Do all per-VM AVIC initialization during vCPU precreation phase
    
    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).

> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 4a0a2dbd1687f..d71a2fed1a08b 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)

[ ... ]

> -int avic_vm_init(struct kvm *kvm)
> +static void avic_add_vm_to_ga_log_list(struct kvm *kvm)
>  {

[Severity: Low]
Does the function prototype for avic_vm_init() in arch/x86/kvm/svm/svm.h also
need to be removed? The declaration appears to still be present even though
the implementation is removed here.

> -	unsigned long flags;
> -	int err = -ENOMEM;
>  	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);

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

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

[Sean Christopherson]

On Tue, Jun 30, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> > index 4a0a2dbd1687f..d71a2fed1a08b 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)
> 
> [ ... ]
> 
> > -int avic_vm_init(struct kvm *kvm)
> > +static void avic_add_vm_to_ga_log_list(struct kvm *kvm)
> >  {
> 
> [Severity: Low]
> Does the function prototype for avic_vm_init() in arch/x86/kvm/svm/svm.h also
> need to be removed? The declaration appears to still be present even though
> the implementation is removed here.

Doh.  Assuming no other issues, I'll fixup when applying.

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

[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 3/3] KVM: SVM: Remove VM from the GA Log notifier list before VM destruction

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fails to fully resolve the Use-After-Free race in `avic_ga_log_notifier()`. It leaves a Time-Of-Check to Time-Of-Use (TOCTOU) vulnerability where an already-found vCPU pointer can be dereferenced after it is freed.
--

commit a94e2dbde8bab766023c731fa2628928c6d8e750
Author: Sean Christopherson <seanjc@google.com>

KVM: SVM: Remove VM from the GA Log notifier list before VM destruction

This patch changes when a VM is deleted from the GA Log interrupt list. It
now removes the VM during avic_vm_pre_destroy() instead of waiting for
avic_vm_destroy(). This attempts to prevent a use-after-free if a GA Log
notification arrives for a vCPU after the last reference to the VM is put.

> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index d71a2fed1a08b..c5b1d294b15a2 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);
> +}

[Severity: High]
This isn't a bug introduced by this patch, but could there still be a
Time-Of-Check to Time-Of-Use race in avic_ga_log_notifier()?

If a notification arrives and avic_ga_log_notifier() finds the VM and its
vCPU pointer under svm_vm_data_hash_lock, it drops the lock before calling
kvm_vcpu_wake_up():

arch/x86/kvm/svm/avic.c:avic_ga_log_notifier() {
    ...
    spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);

    if (vcpu)
        kvm_vcpu_wake_up(vcpu);
}

If avic_ga_log_notifier() is preempted right after releasing the lock,
couldn't avic_vm_pre_destroy() still run concurrently, remove the VM from
the hash table here, and allow the remaining teardown process to free the
vCPUs?

Would this result in avic_ga_log_notifier() calling kvm_vcpu_wake_up()
on a freed vCPU pointer once it resumes execution?

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

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

[Sean Christopherson]

On Tue, Jun 30, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> > index d71a2fed1a08b..c5b1d294b15a2 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);
> > +}
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but could there still be a
> Time-Of-Check to Time-Of-Use race in avic_ga_log_notifier()?
> 
> If a notification arrives and avic_ga_log_notifier() finds the VM and its
> vCPU pointer under svm_vm_data_hash_lock, it drops the lock before calling
> kvm_vcpu_wake_up():
> 
> arch/x86/kvm/svm/avic.c:avic_ga_log_notifier() {
>     ...
>     spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
> 
>     if (vcpu)
>         kvm_vcpu_wake_up(vcpu);
> }
> 
> If avic_ga_log_notifier() is preempted right after releasing the lock,
> couldn't avic_vm_pre_destroy() still run concurrently, remove the VM from
> the hash table here, and allow the remaining teardown process to free the
> vCPUs?
> 
> Would this result in avic_ga_log_notifier() calling kvm_vcpu_wake_up()
> on a freed vCPU pointer once it resumes execution?

Ugh, yes, that's theoretically possible.  I *think* we can simply move the call
to kvm_vcpu_wake_up() inside the spinlock.  There is a potential deadlock that's
near this code[*], but I don't think svm_vm_data_hash_lock will have the same
problem as ir_list_lock?  Regardless, I'll tackle this one in a separate patch,
unless it looks like we'll have to completely redo the entire scheme.

[*] https://lore.kernel.org/all/aUmdSb3d7Z5REMLk@google.com

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

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

[Sean Christopherson]

On Tue, 30 Jun 2026 14:01:53 -0700, Sean Christopherson wrote:
> 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.
> 
> [...]

Applied to kvm-x86 svm, and I even remembered to drop the avic_vm_init()
declaration.

[1/3] KVM: SVM: Make kvm_x86_ops.vcpu_precreate() hook fully AVIC specific
      https://github.com/kvm-x86/linux/commit/8b47740b3ae0
[2/3] KVM: SVM: Do all per-VM AVIC initialization during vCPU precreation phase
      https://github.com/kvm-x86/linux/commit/4fa9a3767d1a
[3/3] KVM: SVM: Remove VM from the GA Log notifier list before VM destruction
      https://github.com/kvm-x86/linux/commit/78684b65fcc0

--
https://github.com/kvm-x86/linux/tree/next