* [PATCH v2 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable
@ 2026-09-18 4:02 zjamg
2026-09-18 4:02 ` [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable zjamg
2026-09-20 1:49 ` [PATCH v3 0/1] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay Yuchao Zhang
0 siblings, 2 replies; 8+ messages in thread
From: zjamg @ 2026-09-18 4:02 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton
Cc: James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, kvmarm, linux-arm-kernel, linux-kernel,
Yuchao Zhang
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable 2026-09-18 4:02 [PATCH v2 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable zjamg @ 2026-09-18 4:02 ` zjamg 2026-09-18 7:22 ` Fuad Tabba 2026-09-20 1:49 ` [PATCH v3 0/1] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay Yuchao Zhang 1 sibling, 1 reply; 8+ messages in thread From: zjamg @ 2026-09-18 4:02 UTC (permalink / raw) To: Marc Zyngier, Oliver Upton Cc: James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel, linux-kernel, Yuchao Zhang, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable 2026-09-18 4:02 ` [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable zjamg @ 2026-09-18 7:22 ` Fuad Tabba 2026-09-18 11:50 ` Yuchao Zhang 0 siblings, 1 reply; 8+ messages in thread From: Fuad Tabba @ 2026-09-18 7:22 UTC (permalink / raw) To: zjamg Cc: Marc Zyngier, Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable 2026-09-18 7:22 ` Fuad Tabba @ 2026-09-18 11:50 ` Yuchao Zhang 2026-09-18 11:58 ` Fuad Tabba 0 siblings, 1 reply; 8+ messages in thread From: Yuchao Zhang @ 2026-09-18 11:50 UTC (permalink / raw) To: Fuad Tabba Cc: Marc Zyngier, Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable 2026-09-18 11:50 ` Yuchao Zhang @ 2026-09-18 11:58 ` Fuad Tabba 2026-09-18 19:15 ` Oliver Upton 0 siblings, 1 reply; 8+ messages in thread From: Fuad Tabba @ 2026-09-18 11:58 UTC (permalink / raw) To: Yuchao Zhang Cc: Marc Zyngier, Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable 2026-09-18 11:58 ` Fuad Tabba @ 2026-09-18 19:15 ` Oliver Upton 0 siblings, 0 replies; 8+ messages in thread From: Oliver Upton @ 2026-09-18 19:15 UTC (permalink / raw) To: Fuad Tabba Cc: Yuchao Zhang, Marc Zyngier, Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 0/1] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay 2026-09-18 4:02 [PATCH v2 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable zjamg 2026-09-18 4:02 ` [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable zjamg @ 2026-09-20 1:49 ` Yuchao Zhang 2026-09-20 1:49 ` [PATCH v3] " Yuchao Zhang 1 sibling, 1 reply; 8+ messages in thread From: Yuchao Zhang @ 2026-09-20 1:49 UTC (permalink / raw) To: Marc Zyngier, Oliver Upton Cc: Fuad Tabba, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel, linux-kernel, 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay 2026-09-20 1:49 ` [PATCH v3 0/1] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay Yuchao Zhang @ 2026-09-20 1:49 ` Yuchao Zhang 0 siblings, 0 replies; 8+ messages in thread From: Yuchao Zhang @ 2026-09-20 1:49 UTC (permalink / raw) To: Marc Zyngier, Oliver Upton Cc: Fuad Tabba, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel, linux-kernel, Yuchao Zhang, stable 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-20 1:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-18 4:02 [PATCH v2 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable zjamg 2026-09-18 4:02 ` [PATCH v2] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable zjamg 2026-09-18 7:22 ` Fuad Tabba 2026-09-18 11:50 ` Yuchao Zhang 2026-09-18 11:58 ` Fuad Tabba 2026-09-18 19:15 ` Oliver Upton 2026-09-20 1:49 ` [PATCH v3 0/1] KVM: arm64: vgic: Drop last_lr_irq and serialize overflow EOI replay Yuchao Zhang 2026-09-20 1:49 ` [PATCH v3] " Yuchao Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®