[PATCH v2 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable

[PATCH v2 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable

[zjamg]

From: Yuchao Zhang <ndaugoing@gmail.com>

Hi Marc, Oliver, and KVM/arm64 maintainers,

By code inspection of commit 6da5e537f5af ("KVM: arm64: vgic: Pick EOIcount
deactivations from AP-list tail"), a race condition exists when a remote
vCPU disables LPIs while the target vCPU has an in-flight LPI in a List
Register (LR).

Specifically:
- vgic_flush_pending_lpis() unconditionally unlinks all LPIs from ap_list
  without checking whether the interrupt is in an LR (irq->on_lr).
- If the LPI in the LR happened to be the last one populated, the per-CPU
  pointer *host_data_ptr(last_lr_irq) on the target vCPU is left dangling.
- When the target vCPU exits guest mode, vgic_v3_fold_lr_state() starts
  traversing ap_list via list_for_each_entry_continue() from this unlinked,
  poisoned (or freed) last_lr_irq, leading to UAF or an immediate panic
  when locking irq->irq_lock.

Solution & Scope:
This patch prevents unlinking LPIs that are currently on an LR in
vgic_flush_pending_lpis(), ensures *host_data_ptr(last_lr_irq) is cleared
after folding, skips the ap_list walk when eoicount is zero, and prevents
the fold from resurrecting the pending state of an edge LPI once the
redistributor has LPIs disabled.

Note: this closes the primary race (the last_lr_irq node itself is no
longer unlinkable while in-flight), but the fold traversal can still
race with a remote flush unlinking a subsequent non-LR node in the
ap_list tail. Fully closing that window needs the fold side to take
references before dropping locks (in the spirit of the prune-side fix
in commit 7258770e5814 ("KVM: arm64: vgic: Handle race between
interrupt affinity change and LPI disabling")) and is left as a
follow-up.

Changes in v2:
- Added the fold-side guard: vgic_v3_fold_lr() no longer preserves the
  pending bit of an edge LPI folded while the redistributor has LPIs
  disabled. Without this, a flushed in-flight LPI is resurrected from
  the LR pending bit and re-injected while GICR_CTLR.EnableLPIs is 0
  (spurious LPI delivery to the guest), since the injection path has no
  lpis_enabled gate. Thanks to the Sashiko AI review for pointing this
  out.

Yuchao Zhang (1):
  KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable

 arch/arm64/kvm/vgic/vgic-v2.c |  3 +++
 arch/arm64/kvm/vgic/vgic-v3.c | 14 ++++++++++++--
 arch/arm64/kvm/vgic/vgic.c    | 10 +++++++---
 3 files changed, 22 insertions(+), 5 deletions(-)

--
2.53.0

[PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable

[zjamg]

From: Yuchao Zhang <ndaugoing@gmail.com>

By code inspection, a race condition and potential Use-After-Free/crash
exists between remote LPI disabling and local LR folding when EOImode==0.

Commit 6da5e537f5af ("KVM: arm64: vgic: Pick EOIcount deactivations from
AP-list tail") introduced tracking the last interrupt placed in a List
Register in per-CPU host data (*host_data_ptr(last_lr_irq)) and traverses
the remaining ap_list via list_for_each_entry_continue() in
vgic_v3_fold_lr_state().

However, an interrupt loaded into an LR can be an LPI. While vCPU-B is
running the guest, another vCPU-A can write to vCPU-B's redistributor
GICR_CTLR to clear EnableLPIs, which dispatches vgic_flush_pending_lpis().

vgic_flush_pending_lpis() unconditionally removes all LPIs from ap_list
without checking whether the interrupt is currently in an LR
(irq->on_lr):
- It calls list_del(&irq->ap_list), setting ap_list.next to LIST_POISON1,
  and drops the AP-list reference.
- If the LPI is unmapped or its translation cache was invalidated, the
  vgic_irq refcount drops to zero and the object is freed via RCU.
- Meanwhile, vCPU-B's per-CPU *host_data_ptr(last_lr_irq) cannot be
  cleared by remote vCPUs and is left dangling.

When vCPU-B subsequently exits the guest:
1. vgic_v3_fold_lr_state() resumes using the unlinked last_lr_irq.
2. list_for_each_entry_continue() unconditionally evaluates
   list_next_entry(irq, ap_list) during loop initialization, accessing
   LIST_POISON1 (or freed memory).
3. If eoicount > 0, it attempts guard(raw_spinlock)(&irq->irq_lock) on
   the poisoned address, leading to an immediate host kernel panic.

Fix this by:
1. In vgic_flush_pending_lpis(), do not remove LPIs that are currently
   in-flight in an LR (irq->on_lr == true). They will be naturally pruned
   by the owning vCPU's vgic_prune_ap_list() after LR folding.
2. In vgic_fold_state(), clear *host_data_ptr(last_lr_irq) after folding
   so that no stale pointer survives past guest execution.
3. In vgic_v3_fold_lr_state() and vgic_v2_fold_lr_state(), bail out
   immediately if eoicount is zero, avoiding unnecessary list_next_entry()
   evaluation.
4. In vgic_v3_fold_lr(), do not resurrect the pending state of an edge
   LPI folded while the redistributor has LPIs disabled.
   vgic_flush_pending_lpis() cannot remotely clear the LRs of a running
   vCPU, so such an LPI would otherwise be requeued and re-injected
   while GICR_CTLR.EnableLPIs is 0, violating the flush semantics (and
   the architecture, which gives no expectation of the pending state
   being retained across a disable).

Note: this closes the primary race (the last_lr_irq node itself is no
longer unlinkable while in-flight), but the fold traversal can still
race with a remote flush unlinking a subsequent non-LR node in the
ap_list tail. Fully closing that window needs the fold side to take
references before dropping locks (in the spirit of the prune-side fix
in commit 7258770e5814 ("KVM: arm64: vgic: Handle race between
interrupt affinity change and LPI disabling")) and is left as a
follow-up.

Fixes: 6da5e537f5af ("KVM: arm64: vgic: Pick EOIcount deactivations from AP-list tail")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
 arch/arm64/kvm/vgic/vgic-v2.c |  3 +++
 arch/arm64/kvm/vgic/vgic-v3.c | 14 ++++++++++++--
 arch/arm64/kvm/vgic/vgic.c    | 10 +++++++---
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index 7182f63fc938..7b6cd05ce32d 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -122,6 +122,9 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 	for (int lr = 0; lr < vgic_cpu->vgic_v2.used_lrs; lr++)
 		vgic_v2_fold_lr(vcpu, cpuif->vgic_lr[lr]);
 
+	if (!eoicount)
+		return;
+
 	/* See the GICv3 equivalent for the EOIcount handling rationale */
 	list_for_each_entry_continue(irq, &vgic_cpu->ap_list_head, ap_list) {
 		u32 lr;
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 726e20a1da6e..dd1314f4543d 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -96,9 +96,16 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)
 		deactivated = irq->active && !(val & ICH_LR_ACTIVE_BIT);
 		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
 
-		/* Edge is the only case where we preserve the pending bit */
+		/*
+		 * Edge is the only case where we preserve the pending bit.
+		 * Do not resurrect the pending state of LPIs once the
+		 * redistributor has them disabled: vgic_flush_pending_lpis()
+		 * has discarded them, and the LRs of a running vCPU cannot
+		 * be remotely cleared.
+		 */
 		if (irq->config == VGIC_CONFIG_EDGE &&
-		    (val & ICH_LR_PENDING_BIT))
+		    (val & ICH_LR_PENDING_BIT) &&
+		    (irq->intid < VGIC_MIN_LPI || vgic_lpis_enabled(vcpu)))
 			irq->pending_latch = true;
 
 		/*
@@ -155,6 +162,9 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 	for (int lr = 0; lr < cpuif->used_lrs; lr++)
 		vgic_v3_fold_lr(vcpu, cpuif->vgic_lr[lr]);
 
+	if (!eoicount)
+		return;
+
 	/*
 	 * EOIMode=0: use EOIcount to emulate deactivation. We are
 	 * guaranteed to deactivate in reverse order of the activation, so
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index b25303d9919f..0d7d75c3ac93 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -205,10 +205,12 @@ void vgic_flush_pending_lpis(struct kvm_vcpu *vcpu)
 		if (irq_is_lpi(vcpu->kvm, irq->intid)) {
 			raw_spin_lock(&irq->irq_lock);
 			irq->pending_latch = false;
-			list_del(&irq->ap_list);
-			irq->vcpu = NULL;
+			if (!irq->on_lr) {
+				list_del(&irq->ap_list);
+				irq->vcpu = NULL;
+				deleted |= vgic_put_irq_norelease(vcpu->kvm, irq);
+			}
 			raw_spin_unlock(&irq->irq_lock);
-			deleted |= vgic_put_irq_norelease(vcpu->kvm, irq);
 		}
 	}
 
@@ -873,6 +875,8 @@ static void vgic_fold_state(struct kvm_vcpu *vcpu)
 		vgic_v2_fold_lr_state(vcpu);
 	else
 		vgic_v3_fold_lr_state(vcpu);
+
+	*host_data_ptr(last_lr_irq) = NULL;
 }
 
 /* Requires the irq_lock to be held. */
-- 
2.53.0

Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable

[Fuad Tabba]

Hi Yuchao,

On Fri, 18 Sep 2026 12:02:14 +0800, Yuchao Zhang <ndaugoing@gmail.com> wrote:

[...]

> Note: this closes the primary race (the last_lr_irq node itself is no
> longer unlinkable while in-flight), but the fold traversal can still
> race with a remote flush unlinking a subsequent non-LR node in the
> ap_list tail. Fully closing that window needs the fold side to take
> references before dropping locks (in the spirit of the prune-side fix
> in commit 7258770e5814 ("KVM: arm64: vgic: Handle race between
> interrupt affinity change and LPI disabling")) and is left as a
> follow-up.

This is the same race Hyunwoo reported back in June [1]. You might
want to have a look at that thread first: Oliver and Marc's view there
was that the fix is to take the ap_list_lock in
vgic_v3_fold_lr_state() [2][3], and Hyunwoo posted a draft of that
[4].

Cheers,
/fuad

[1] https://lore.kernel.org/r/aiHrGM1f8czcUby4@v4bel
[2] https://lore.kernel.org/r/aiJi5a3JJ-TbWL-s@kernel.org
[3] https://lore.kernel.org/r/87a4t99z9n.wl-maz@kernel.org
[4] https://lore.kernel.org/r/aiXvwGD1hS6vwLEd@v4bel

Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable

[Yuchao Zhang]

Hi Fuad,

Thanks a lot for pointing me to that thread! I was not aware of
Hyunwoo's earlier report and the discussion with Oliver and Marc.

I'll read through the thread and their rationale on the ap_list_lock
approach. I'm happy to defer to Hyunwoo's effort to avoid duplicate
work.

Thanks again for the pointer!

Best regards,
Yuchao

Fuad Tabba <fuad.tabba@linux.dev> 于2026年9月18日周五 15:22写道：
>
> Hi Yuchao,
>
> On Fri, 18 Sep 2026 12:02:14 +0800, Yuchao Zhang <ndaugoing@gmail.com> wrote:
>
> [...]
>
> > Note: this closes the primary race (the last_lr_irq node itself is no
> > longer unlinkable while in-flight), but the fold traversal can still
> > race with a remote flush unlinking a subsequent non-LR node in the
> > ap_list tail. Fully closing that window needs the fold side to take
> > references before dropping locks (in the spirit of the prune-side fix
> > in commit 7258770e5814 ("KVM: arm64: vgic: Handle race between
> > interrupt affinity change and LPI disabling")) and is left as a
> > follow-up.
>
> This is the same race Hyunwoo reported back in June [1]. You might
> want to have a look at that thread first: Oliver and Marc's view there
> was that the fix is to take the ap_list_lock in
> vgic_v3_fold_lr_state() [2][3], and Hyunwoo posted a draft of that
> [4].
>
> Cheers,
> /fuad
>
> [1] https://lore.kernel.org/r/aiHrGM1f8czcUby4@v4bel
> [2] https://lore.kernel.org/r/aiJi5a3JJ-TbWL-s@kernel.org
> [3] https://lore.kernel.org/r/87a4t99z9n.wl-maz@kernel.org
> [4] https://lore.kernel.org/r/aiXvwGD1hS6vwLEd@v4bel

Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable

[Fuad Tabba]

Hi Yuchao,

On Fri, 18 Sept 2026 at 12:51, Yuchao Zhang <ndaugoing@gmail.com> wrote:
>
> Hi Fuad,
>
> Thanks a lot for pointing me to that thread! I was not aware of
> Hyunwoo's earlier report and the discussion with Oliver and Marc.
>
> I'll read through the thread and their rationale on the ap_list_lock
> approach. I'm happy to defer to Hyunwoo's effort to avoid duplicate
> work.

I'm not sure you should defer to their effort. It doesn't seem like
Hyunwoo has done any work on this for a while. I just wanted to point
you to the existing discussion.

Cheers,
/fuad

> Thanks again for the pointer!
>
> Best regards,
> Yuchao
>
> Fuad Tabba <fuad.tabba@linux.dev> 于2026年9月18日周五 15:22写道：
> >
> > Hi Yuchao,
> >
> > On Fri, 18 Sep 2026 12:02:14 +0800, Yuchao Zhang <ndaugoing@gmail.com> wrote:
> >
> > [...]
> >
> > > Note: this closes the primary race (the last_lr_irq node itself is no
> > > longer unlinkable while in-flight), but the fold traversal can still
> > > race with a remote flush unlinking a subsequent non-LR node in the
> > > ap_list tail. Fully closing that window needs the fold side to take
> > > references before dropping locks (in the spirit of the prune-side fix
> > > in commit 7258770e5814 ("KVM: arm64: vgic: Handle race between
> > > interrupt affinity change and LPI disabling")) and is left as a
> > > follow-up.
> >
> > This is the same race Hyunwoo reported back in June [1]. You might
> > want to have a look at that thread first: Oliver and Marc's view there
> > was that the fix is to take the ap_list_lock in
> > vgic_v3_fold_lr_state() [2][3], and Hyunwoo posted a draft of that
> > [4].
> >
> > Cheers,
> > /fuad
> >
> > [1] https://lore.kernel.org/r/aiHrGM1f8czcUby4@v4bel
> > [2] https://lore.kernel.org/r/aiJi5a3JJ-TbWL-s@kernel.org
> > [3] https://lore.kernel.org/r/87a4t99z9n.wl-maz@kernel.org
> > [4] https://lore.kernel.org/r/aiXvwGD1hS6vwLEd@v4bel

Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable

[Oliver Upton]

On Fri, Sep 18, 2026 at 12:58:17PM +0100, Fuad Tabba wrote:
> Hi Yuchao,
> 
> On Fri, 18 Sept 2026 at 12:51, Yuchao Zhang <ndaugoing@gmail.com> wrote:
> >
> > Hi Fuad,
> >
> > Thanks a lot for pointing me to that thread! I was not aware of
> > Hyunwoo's earlier report and the discussion with Oliver and Marc.
> >
> > I'll read through the thread and their rationale on the ap_list_lock
> > approach. I'm happy to defer to Hyunwoo's effort to avoid duplicate
> > work.
> 
> I'm not sure you should defer to their effort. It doesn't seem like
> Hyunwoo has done any work on this for a while. I just wanted to point
> you to the existing discussion.

Yuchao if you have cycles I would definitely appreciate it if you can
pursue a fix. My view hasn't changed since before: let's make that
traversal of the ap_list is done under the ap_list_lock, as this is not
intended to be walked lock-free.

Taking a step back, the whole cross-vCPU LPI disabling always leaves me
feeling ill... Really when RWP=0 becomes visible from another vCPU we
need to guarantee that the LPIs have been actually retired, meaning we
can't have one sitting in an LR. Even with the locking fix I think we
miss this.

Given how unlikely it is for well-behaved software to disable LPIs
remotely in the first place, I wonder if we should just halt the VM
similar to how we handle accesses to the active state. That's a really
big hammer but we've had a lot of bugs in this department and I'm
somewhat biased towards an obviously correct solution.

We wouldn't need to do this for a vCPU disabling LPIs on its own
redistributor since we've already exited the guest.

I'll think on it a bit more.

Oliver

[PATCH v3 0/1] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay

[Yuchao Zhang]

Hi Marc, Oliver, Fuad, and KVM/arm64 maintainers,

Following up on the discussion around the remote LPI disable vs LR fold
race [1], this series addresses the issue at its root: the last_lr_irq
cursor introduced in commit 6da5e537f5af ("KVM: arm64: vgic: Pick EOIcount
deactivations from AP-list tail").

Problem:
vgic_v3_fold_lr_state() / vgic_v2_fold_lr_state() walk the overflow tail
of the ap_list starting from *host_data_ptr(last_lr_irq) without holding
ap_list_lock. Caching this raw pointer across the entire guest execution
leaves it vulnerable to concurrent modification: when a remote vCPU
disables LPIs via GICR_CTLR, vgic_flush_pending_lpis() unlinks the node
with list_del() and drops its reference, leaving last_lr_irq pointing to
a poisoned or freed object. When the vCPU exits, the walk dereferences
corrupted memory, causing a kernel panic or UAF.

Oliver and Marc suggested taking ap_list_lock in
vgic_v3_fold_lr_state() [2][3]. I tried that approach first, but it
runs into the following lock-order problems:
1. kvm_notify_acked_irq() grabs regular spinlocks and can re-enter
   vgic_queue_irq_unlock() (which takes ap_list_lock), causing deadlock.
2. vgic_put_irq() is a no-op for SPI/PPI, but for LPIs it calls
   refcount_dec_and_lock_irqsave() which acquires dist->lpi_xa.xa_lock
   when dropping the last reference. That lock sits above ap_list_lock
   in the lock ordering, so calling vgic_put_irq() under ap_list_lock
   causes lock inversion.
Addressing these under a global fold lock requires deferring all EOI'ed
SPI notifications to a stack bitmap and deferring LPI releases with
vgic_put_irq_norelease(), penalizing the fast path for all exits even
though folding hardware LRs does not touch ap_list at all. It also still
requires pinning and clearing the per-CPU last_lr_irq pointer.

Changes since v2:
- Replaced the skip-unlink approach of v2 with dropping the last_lr_irq
  cursor entirely and serializing only the overflow EOI replay under
  ap_list_lock (per Oliver and Marc's suggestion [2][3]).

The cleaner approach in this patch:
1. Drop the fragile last_lr_irq per-CPU cursor entirely.
2. The common fast path (folding hardware LRs) runs natively without
   ap_list_lock. We record the INTIDs of the used LRs in a small stack
   array (VGIC_V3_MAX_LRS / VGIC_V2_MAX_LRS entries).
3. If eoicount == 0 (the vast majority of guest exits), clear
   cpuif->used_lrs = 0 and return immediately without taking ap_list_lock.
4. If unlikely(eoicount > 0), acquire ap_list_lock only to scan the ap_list
   and pin (via vgic_get_irq_ref) up to eoicount active interrupts that
   were not in hardware LRs. The scan is a linear walk over at most 16/64
   LR INTIDs per candidate, on the rare eoicount > 0 path - bounded and
   acceptable. lr_intids[] only records INTIDs from the used_lrs range;
   the extraction mask mirrors vgic_fold_lr() exactly
   (ICH_LR_VIRTUAL_ID_MASK for GICv3, GICH_LR_VIRTUALID for GICv2),
   so no stale or invalid slot can produce a false match.
5. Drop ap_list_lock immediately, and then replay their deactivations
   outside the lock, naturally eliminating both eventfd re-entrancy and
   lpi_xa lock inversions without changing any function signatures.
   vgic_fold_lr() has no error path, so cpuif->used_lrs = 0 is always
   reached after a complete fold, with no risk of partial cleanup.

Note on EOIcount hardware limits:
ICH_HCR_EL2.EOIcount (GICv3) and GICH_HCR.EOICount (GICv2) are both
5-bit fields, giving a maximum value of 31. The targets[32] stack array
and min_t(u32, eoicount, ARRAY_SIZE(targets)) bound together ensure no
overflow even if hardware writes an unexpected value.

Note on EOIcount source:
For GICv3, eoicount is read from cpuif->vgic_hcr, which is populated by
__vgic_v3_save_state right before ICH_HCR_EL2 is cleared in hardware.
For GICv2, vgic_v2_save_state reads GICH_HCR via MMIO into the same
cpuif->vgic_hcr field (when LRENPIE is set) before writing 0 to GICH_HCR.
In both cases the software copy is the only valid source; reading the
hardware register after save would return 0.

Note on scope:
This series fixes the use-after-free in the ap_list traversal caused by
last_lr_irq. It does not address the separate concern raised by Oliver
in [2] about a pending LPI still sitting in an LR when RWP=0 becomes
visible to another vCPU; that may require a stronger approach (e.g.
halting the VM) and I am happy to follow up separately.

[1] https://lore.kernel.org/r/aiHrGM1f8czcUby4@v4bel
[2] https://lore.kernel.org/r/aiJi5a3JJ-TbWL-s@kernel.org
[3] https://lore.kernel.org/r/87a4t99z9n.wl-maz@kernel.org

Yuchao Zhang (1):
  KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay

 arch/arm64/include/asm/kvm_host.h |  3 --
 arch/arm64/kvm/vgic/vgic-v2.c     | 64 +++++++++++++++++++++++--------
 arch/arm64/kvm/vgic/vgic-v3.c     | 74 ++++++++++++++++++++++++++-----
 arch/arm64/kvm/vgic/vgic.c        |  9 +---
 arch/arm64/kvm/vgic/vgic.h        | 14 ++++++++
 5 files changed, 114 insertions(+), 50 deletions(-)

-- 
2.53.0

[PATCH v3] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay

[Yuchao Zhang]

Commit 6da5e537f5af ("KVM: arm64: vgic: Pick EOIcount deactivations from
AP-list tail") introduced tracking the last interrupt placed in a List
Register in per-CPU host data (*host_data_ptr(last_lr_irq)) to resume the
EOIcount-based deactivation walk in the overflow tail of the ap_list.

However, caching this raw pointer in per-CPU host data spans the entire
guest execution without holding ap_list_lock or a reference count.
A concurrent vCPU can disable LPIs by writing to GICR_CTLR.EnableLPIs,
triggering vgic_flush_pending_lpis() which removes LPIs with list_del()
and drops their reference.  When the victim vCPU exits, the lock-free
list_for_each_entry_continue() starting from last_lr_irq dereferences a
poisoned next pointer or freed object, causing a host panic or UAF.

Rather than taking ap_list_lock across the entire fold path (which forces
deferring kvm_notify_acked_irq() to avoid eventfd deadlocks and deferring
vgic_put_irq() to avoid lock inversion with lpi_xa), solve this by
dropping the fragile last_lr_irq cursor entirely:

1. In vgic_flush_lr_state(), do not track last_lr_irq.
2. In vgic_v3_fold_lr_state() / vgic_v2_fold_lr_state(), fold the hardware
   LRs natively without ap_list_lock.  Record the INTIDs of the used LRs in
   a small stack array (at most 16/64 entries).
3. If eoicount == 0 (the vast majority of exits), clear cpuif->used_lrs and
   return immediately without taking ap_list_lock on the fast path.
4. If unlikely(eoicount > 0), acquire ap_list_lock only to scan the ap_list
   and pin up to eoicount active interrupts that were not in hardware LRs.
   The scan is a linear walk over at most 16/64 LR INTIDs per candidate, on
   the rare eoicount > 0 path - bounded and acceptable.
   Then drop ap_list_lock before replaying their deactivations, naturally
   avoiding both eventfd re-entrancy and lpi_xa lock inversions.

Fixes: 6da5e537f5af ("KVM: arm64: vgic: Pick EOIcount deactivations from AP-list tail")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
 arch/arm64/include/asm/kvm_host.h |  3 --
 arch/arm64/kvm/vgic/vgic-v2.c     | 64 +++++++++++++++++++-------
 arch/arm64/kvm/vgic/vgic-v3.c     | 74 +++++++++++++++++++++----------
 arch/arm64/kvm/vgic/vgic.c        |  9 +---
 arch/arm64/kvm/vgic/vgic.h        | 14 ++++++
 5 files changed, 114 insertions(+), 50 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 27fe0cd5b2d7..c4355ae23e98 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -800,9 +800,6 @@ struct kvm_host_data {
 	unsigned int debug_brps;
 	unsigned int debug_wrps;
 
-	/* Last vgic_irq part of the AP list recorded in an LR */
-	struct vgic_irq *last_lr_irq;
-
 	/* PPI state tracking for GICv5-based guests */
 	struct {
 		DECLARE_BITMAP(pendr, VGIC_V5_NR_PRIVATE_IRQS);
diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index 7182f63fc938..4a0ad5bd1cf6 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -115,37 +115,71 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_v2_cpu_if *cpuif = &vgic_cpu->vgic_v2;
 	u32 eoicount = FIELD_GET(GICH_HCR_EOICOUNT, cpuif->vgic_hcr);
-	struct vgic_irq *irq = *host_data_ptr(last_lr_irq);
+	struct vgic_irq *targets[32];
+	u32 lr_intids[VGIC_V2_MAX_LRS];
+	int nr_lrs = min_t(int, vgic_cpu->vgic_v2.used_lrs, ARRAY_SIZE(lr_intids));
+	u32 max_targets;
+	int nr_targets = 0;
 
 	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
 
-	for (int lr = 0; lr < vgic_cpu->vgic_v2.used_lrs; lr++)
-		vgic_v2_fold_lr(vcpu, cpuif->vgic_lr[lr]);
+	if (!vgic_cpu->vgic_v2.used_lrs && !eoicount)
+		return;
 
-	/* See the GICv3 equivalent for the EOIcount handling rationale */
-	list_for_each_entry_continue(irq, &vgic_cpu->ap_list_head, ap_list) {
-		u32 lr;
+	for (int lr = 0; lr < nr_lrs; lr++) {
+		u32 val = cpuif->vgic_lr[lr];
+
+		lr_intids[lr] = val & GICH_LR_VIRTUALID;
+		vgic_v2_fold_lr(vcpu, val);
+	}
+
+	cpuif->used_lrs = 0;
+
+	if (likely(!eoicount))
+		return;
+
+	max_targets = min_t(u32, eoicount, ARRAY_SIZE(targets));
+
+	/*
+	 * EOIMode=0: replay deactivations for overflow active interrupts.
+	 * Walk ap_list under ap_list_lock and pin candidate interrupts so we
+	 * can process them outside the lock without risking lock inversion.
+	 */
+	scoped_guard(raw_spinlock, &vgic_cpu->ap_list_lock) {
+		struct vgic_irq *irq;
 
-		if (!eoicount) {
-			break;
-		} else {
-			guard(raw_spinlock)(&irq->irq_lock);
+		list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
+			if (nr_targets == max_targets)
+				break;
 
-			if (!(likely(vgic_target_oracle(irq) == vcpu) &&
-			      irq->active))
+			if (intid_in_lrs(irq->intid, lr_intids, nr_lrs))
 				continue;
 
+			scoped_guard(raw_spinlock, &irq->irq_lock) {
+				if (likely(vgic_target_oracle(irq) == vcpu) &&
+				    irq->active) {
+					vgic_get_irq_ref(irq);
+					targets[nr_targets++] = irq;
+				}
+			}
+		}
+	}
+
+	for (int i = 0; i < nr_targets; i++) {
+		struct vgic_irq *irq = targets[i];
+		u32 lr;
+
+		scoped_guard(raw_spinlock, &irq->irq_lock) {
 			lr = vgic_v2_compute_lr(vcpu, irq) & ~GICH_LR_ACTIVE_BIT;
 		}
 
 		if (lr & GICH_LR_HW)
 			writel_relaxed(FIELD_GET(GICH_LR_PHYSID_CPUID, lr),
 				       kvm_vgic_global_state.gicc_base + GIC_CPU_DEACTIVATE);
+
 		vgic_v2_fold_lr(vcpu, lr);
-		eoicount--;
+		vgic_put_irq(vcpu->kvm, irq);
 	}
-
-	cpuif->used_lrs = 0;
 }
 
 void vgic_v2_deactivate(struct kvm_vcpu *vcpu, u32 val)
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 726e20a1da6e..7e573860f3e1 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -148,37 +148,65 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_v3_cpu_if *cpuif = &vgic_cpu->vgic_v3;
 	u32 eoicount = FIELD_GET(ICH_HCR_EL2_EOIcount, cpuif->vgic_hcr);
-	struct vgic_irq *irq = *host_data_ptr(last_lr_irq);
+	struct vgic_irq *targets[32];
+	u32 lr_intids[VGIC_V3_MAX_LRS];
+	int nr_lrs = min_t(int, cpuif->used_lrs, ARRAY_SIZE(lr_intids));
+	u32 max_targets;
+	int nr_targets = 0;
 
 	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
 
-	for (int lr = 0; lr < cpuif->used_lrs; lr++)
-		vgic_v3_fold_lr(vcpu, cpuif->vgic_lr[lr]);
+	if (!cpuif->used_lrs && !eoicount)
+		return;
+
+	for (int lr = 0; lr < nr_lrs; lr++) {
+		u64 val = cpuif->vgic_lr[lr];
+
+		if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+			lr_intids[lr] = val & ICH_LR_VIRTUAL_ID_MASK;
+		else
+			lr_intids[lr] = val & GICH_LR_VIRTUALID;
+
+		vgic_v3_fold_lr(vcpu, val);
+	}
+
+	cpuif->used_lrs = 0;
+
+	if (likely(!eoicount))
+		return;
+
+	max_targets = min_t(u32, eoicount, ARRAY_SIZE(targets));
 
 	/*
-	 * EOIMode=0: use EOIcount to emulate deactivation. We are
-	 * guaranteed to deactivate in reverse order of the activation, so
-	 * just pick one active interrupt after the other in the tail part
-	 * of the ap_list, past the LRs, and replay the deactivation as if
-	 * the CPU was doing it. We also rely on priority drop to have taken
-	 * place, and the list to be sorted by priority.
+	 * EOIMode=0: replay deactivations for overflow active interrupts.
+	 * Walk ap_list under ap_list_lock and pin candidate interrupts so we
+	 * can process them outside the lock without risking lock inversion.
 	 */
-	list_for_each_entry_continue(irq, &vgic_cpu->ap_list_head, ap_list) {
-		u64 lr;
+	scoped_guard(raw_spinlock, &vgic_cpu->ap_list_lock) {
+		struct vgic_irq *irq;
 
-		/*
-		 * I would have loved to write this using a scoped_guard(),
-		 * but using 'continue' here is a total train wreck.
-		 */
-		if (!eoicount) {
-			break;
-		} else {
-			guard(raw_spinlock)(&irq->irq_lock);
+		list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
+			if (nr_targets == max_targets)
+				break;
 
-			if (!(likely(vgic_target_oracle(irq) == vcpu) &&
-			      irq->active))
+			if (intid_in_lrs(irq->intid, lr_intids, nr_lrs))
 				continue;
 
+			scoped_guard(raw_spinlock, &irq->irq_lock) {
+				if (likely(vgic_target_oracle(irq) == vcpu) &&
+				    irq->active) {
+					vgic_get_irq_ref(irq);
+					targets[nr_targets++] = irq;
+				}
+			}
+		}
+	}
+
+	for (int i = 0; i < nr_targets; i++) {
+		struct vgic_irq *irq = targets[i];
+		u64 lr;
+
+		scoped_guard(raw_spinlock, &irq->irq_lock) {
 			lr = vgic_v3_compute_lr(vcpu, irq) & ~ICH_LR_ACTIVE_BIT;
 		}
 
@@ -186,10 +214,8 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 			vgic_v3_deactivate_phys(FIELD_GET(ICH_LR_PHYS_ID_MASK, lr));
 
 		vgic_v3_fold_lr(vcpu, lr);
-		eoicount--;
+		vgic_put_irq(vcpu->kvm, irq);
 	}
-
-	cpuif->used_lrs = 0;
 }
 
 void vgic_v3_deactivate(struct kvm_vcpu *vcpu, u64 val)
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index b25303d9919f..966948fd3ddd 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -866,9 +866,6 @@ static void vgic_fold_state(struct kvm_vcpu *vcpu)
 		return;
 	}
 
-	if (!*host_data_ptr(last_lr_irq))
-		return;
-
 	if (kvm_vgic_global_state.type == VGIC_V2)
 		vgic_v2_fold_lr_state(vcpu);
 	else
@@ -1015,14 +1012,10 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 	if (irqs_outside_lrs(&als))
 		vgic_sort_ap_list(vcpu);
 
-	*host_data_ptr(last_lr_irq) = NULL;
-
 	list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
 		scoped_guard(raw_spinlock,  &irq->irq_lock) {
-			if (likely(vgic_target_oracle(irq) == vcpu)) {
+			if (likely(vgic_target_oracle(irq) == vcpu))
 				vgic_populate_lr(vcpu, irq, count++);
-				*host_data_ptr(last_lr_irq) = irq;
-			}
 		}
 
 		if (count == kvm_vgic_global_state.nr_lr)
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index b71d486ae514..35a07b320be4 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -334,6 +334,20 @@ static inline void vgic_get_irq_ref(struct vgic_irq *irq)
 	WARN_ON_ONCE(!vgic_try_get_irq_ref(irq));
 }
 
+/*
+ * Linear scan over at most VGIC_V3_MAX_LRS / VGIC_V2_MAX_LRS entries.
+ * Only called on the rare eoicount > 0 path, so the O(n) cost is
+ * acceptable and avoids the complexity of a bitmap.
+ */
+static inline bool intid_in_lrs(u32 intid, const u32 *lr_intids, int nr_lrs)
+{
+	for (int i = 0; i < nr_lrs; i++) {
+		if (lr_intids[i] == intid)
+			return true;
+	}
+	return false;
+}
+
 void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu);
 void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr);
 void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr);
-- 
2.53.0

Re: [PATCH v3 0/1] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay

[Marc Zyngier]

On Sun, 20 Sep 2026 02:49:10 +0100,
Yuchao Zhang <ndaugoing@gmail.com> wrote:
> 
> Hi Marc, Oliver, Fuad, and KVM/arm64 maintainers,
> 
> Following up on the discussion around the remote LPI disable vs LR fold
> race [1], this series addresses the issue at its root: the last_lr_irq
> cursor introduced in commit 6da5e537f5af ("KVM: arm64: vgic: Pick EOIcount
> deactivations from AP-list tail").
> 
> Problem:
> vgic_v3_fold_lr_state() / vgic_v2_fold_lr_state() walk the overflow tail
> of the ap_list starting from *host_data_ptr(last_lr_irq) without holding
> ap_list_lock. Caching this raw pointer across the entire guest execution
> leaves it vulnerable to concurrent modification: when a remote vCPU
> disables LPIs via GICR_CTLR, vgic_flush_pending_lpis() unlinks the node
> with list_del() and drops its reference, leaving last_lr_irq pointing to
> a poisoned or freed object. When the vCPU exits, the walk dereferences
> corrupted memory, causing a kernel panic or UAF.

Why can't this be solved by simply taking a reference on the object
pointed to by last_lr_irq?

> 
> Oliver and Marc suggested taking ap_list_lock in
> vgic_v3_fold_lr_state() [2][3]. I tried that approach first, but it
> runs into the following lock-order problems:
> 1. kvm_notify_acked_irq() grabs regular spinlocks and can re-enter
>    vgic_queue_irq_unlock() (which takes ap_list_lock), causing deadlock.
> 2. vgic_put_irq() is a no-op for SPI/PPI, but for LPIs it calls
>    refcount_dec_and_lock_irqsave() which acquires dist->lpi_xa.xa_lock
>    when dropping the last reference. That lock sits above ap_list_lock
>    in the lock ordering, so calling vgic_put_irq() under ap_list_lock
>    causes lock inversion.

Which is why we have vgic_put_irq_norelease() and
vgic_release_deleted_lpis(), which allow deferring the release until
we're in a suitable context. But that's beside the point.

> Addressing these under a global fold lock requires deferring all EOI'ed
> SPI notifications to a stack bitmap and deferring LPI releases with

Which stack bitmap?

> vgic_put_irq_norelease(), penalizing the fast path for all exits even
> though folding hardware LRs does not touch ap_list at all. It also still
> requires pinning and clearing the per-CPU last_lr_irq pointer.

An uncontended atomic access is hardly an overhead, is it? Where is
the overhead? And I don't understand what you're saying about the LRs
not affecting the ap_list... They *always* do.

> 
> Changes since v2:
> - Replaced the skip-unlink approach of v2 with dropping the last_lr_irq
>   cursor entirely and serializing only the overflow EOI replay under
>   ap_list_lock (per Oliver and Marc's suggestion [2][3]).
> 
> The cleaner approach in this patch:
> 1. Drop the fragile last_lr_irq per-CPU cursor entirely.
> 2. The common fast path (folding hardware LRs) runs natively without
>    ap_list_lock. We record the INTIDs of the used LRs in a small stack
>    array (VGIC_V3_MAX_LRS / VGIC_V2_MAX_LRS entries).

Why is v2 even under consideration? LPIs are strictly v3 (ignoring
v5 here), and non-LPIs are statically allocated, meaning they can't
vanish under your feet.

> 3. If eoicount == 0 (the vast majority of guest exits), clear
>    cpuif->used_lrs = 0 and return immediately without taking ap_list_lock.
> 4. If unlikely(eoicount > 0), acquire ap_list_lock only to scan the ap_list
>    and pin (via vgic_get_irq_ref) up to eoicount active interrupts that
>    were not in hardware LRs. The scan is a linear walk over at most 16/64
>    LR INTIDs per candidate, on the rare eoicount > 0 path - bounded and

What makes you think this is acceptable? It really isn't. The point is
that this is not limited to 16 entries. That's the whole point of
EOIcount, which spans up to 31 simultaneously active priorities.

I have no idea what you describe is achieving, TBH. And looking at the
patch, I see a quadratic behaviour, which doesn't strike me as low
overhead...

>    acceptable. lr_intids[] only records INTIDs from the used_lrs range;
>    the extraction mask mirrors vgic_fold_lr() exactly
>    (ICH_LR_VIRTUAL_ID_MASK for GICv3, GICH_LR_VIRTUALID for GICv2),
>    so no stale or invalid slot can produce a false match.
> 5. Drop ap_list_lock immediately, and then replay their deactivations
>    outside the lock, naturally eliminating both eventfd re-entrancy and
>    lpi_xa lock inversions without changing any function signatures.
>    vgic_fold_lr() has no error path, so cpuif->used_lrs = 0 is always
>    reached after a complete fold, with no risk of partial cleanup.
> 
> Note on EOIcount hardware limits:
> ICH_HCR_EL2.EOIcount (GICv3) and GICH_HCR.EOICount (GICv2) are both
> 5-bit fields, giving a maximum value of 31. The targets[32] stack array
> and min_t(u32, eoicount, ARRAY_SIZE(targets)) bound together ensure no
> overflow even if hardware writes an unexpected value.
> 
> Note on EOIcount source:
> For GICv3, eoicount is read from cpuif->vgic_hcr, which is populated by
> __vgic_v3_save_state right before ICH_HCR_EL2 is cleared in hardware.
> For GICv2, vgic_v2_save_state reads GICH_HCR via MMIO into the same
> cpuif->vgic_hcr field (when LRENPIE is set) before writing 0 to GICH_HCR.
> In both cases the software copy is the only valid source; reading the
> hardware register after save would return 0.
> 
> Note on scope:
> This series fixes the use-after-free in the ap_list traversal caused by
> last_lr_irq. It does not address the separate concern raised by Oliver
> in [2] about a pending LPI still sitting in an LR when RWP=0 becomes
> visible to another vCPU; that may require a stronger approach (e.g.
> halting the VM) and I am happy to follow up separately.

This has all the hallmarks of an AI gone wild.

The problem is correctly described (last_lr_irq doesn't hold a
reference on the irq), but the proposed solution is completely
ignoring it, and implements... something else.

I've hacked something at [1], which:

- changes the behaviour of last_lr_irq to only be non-NULL when the
  LRs are full.

- take a reference on the irq flagged as last_lr_irq, and drop this
  reference in vgic_prune_ap_list(), contributing to the LPIs being
  freed once the ap_list_lock is dropped.

- stop the world when a vcpu disable LPIs. We could do slightly
  better, but it isn't worth the hassle for something that *never*
  happens.

I only boot tested a small nested guest (L1 + L2) with lockdep on my
laptop, and nothing caught fire. Please give it a go.

You also seem to have a reproducer for this, it'd be good if you could
turn it into a selftest.

Thanks,

	M.

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/vgic-last_lr_irq-fixes

-- 
Jazz isn't dead. It just smells funny.

Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable

[Marc Zyngier]

On Fri, 18 Sep 2026 20:15:56 +0100,
Oliver Upton <oupton@kernel.org> wrote:
> 
> On Fri, Sep 18, 2026 at 12:58:17PM +0100, Fuad Tabba wrote:
> > Hi Yuchao,
> > 
> > On Fri, 18 Sept 2026 at 12:51, Yuchao Zhang <ndaugoing@gmail.com> wrote:
> > >
> > > Hi Fuad,
> > >
> > > Thanks a lot for pointing me to that thread! I was not aware of
> > > Hyunwoo's earlier report and the discussion with Oliver and Marc.
> > >
> > > I'll read through the thread and their rationale on the ap_list_lock
> > > approach. I'm happy to defer to Hyunwoo's effort to avoid duplicate
> > > work.
> > 
> > I'm not sure you should defer to their effort. It doesn't seem like
> > Hyunwoo has done any work on this for a while. I just wanted to point
> > you to the existing discussion.
> 
> Yuchao if you have cycles I would definitely appreciate it if you can
> pursue a fix. My view hasn't changed since before: let's make that
> traversal of the ap_list is done under the ap_list_lock, as this is not
> intended to be walked lock-free.
> 
> Taking a step back, the whole cross-vCPU LPI disabling always leaves me
> feeling ill... Really when RWP=0 becomes visible from another vCPU we
> need to guarantee that the LPIs have been actually retired, meaning we
> can't have one sitting in an LR. Even with the locking fix I think we
> miss this.
> 
> Given how unlikely it is for well-behaved software to disable LPIs
> remotely in the first place, I wonder if we should just halt the VM
> similar to how we handle accesses to the active state. That's a really
> big hammer but we've had a lot of bugs in this department and I'm
> somewhat biased towards an obviously correct solution.

Irrespective of the whole ap_list_lock issue, I think this is the only
valid option. Messing with the ap_list of another vcpu while it is
running can never result in something that actually works. There's a
hack doing that in my tree.

> We wouldn't need to do this for a vCPU disabling LPIs on its own
> redistributor since we've already exited the guest.

In general, we could stop the target vcpu only. But this is making
things more complex, and I quite like the idea of a large
hammer... ;-)

	M.

-- 
Jazz isn't dead. It just smells funny.