Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable
@ 2026-09-18  2:46 zjamg
  2026-09-18  2:46 ` [PATCH 1/1] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable zjamg
  0 siblings, 1 reply; 3+ messages in thread
From: zjamg @ 2026-09-18  2:46 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton
  Cc: Fuad Tabba, Joey Gouly, Steffen Eiden, Suzuki K Poulose,
	Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm,
	linux-arm-kernel, kvm, 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, and skips the ap_list walk when eoicount is zero.

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.

Any feedback is greatly appreciated.

Thanks,
Yuchao Zhang

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 |  3 +++
 arch/arm64/kvm/vgic/vgic.c    | 10 +++++++---
 3 files changed, 13 insertions(+), 3 deletions(-)

-- 
2.53.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/1] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable
  2026-09-18  2:46 [PATCH 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable zjamg
@ 2026-09-18  2:46 ` zjamg
  2026-09-18  2:58   ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: zjamg @ 2026-09-18  2:46 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton
  Cc: Fuad Tabba, Joey Gouly, Steffen Eiden, Suzuki K Poulose,
	Zenghui Yu, Catalin Marinas, Will Deacon, kvmarm,
	linux-arm-kernel, kvm, 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.

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 |  3 +++
 arch/arm64/kvm/vgic/vgic.c    | 10 +++++++---
 3 files changed, 13 insertions(+), 3 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..c6eb5d9dca2d 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -155,6 +155,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 related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable
  2026-09-18  2:46 ` [PATCH 1/1] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable zjamg
@ 2026-09-18  2:58   ` sashiko-bot
  0 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-18  2:58 UTC (permalink / raw)
  To: zjamg; +Cc: Oliver Upton, Marc Zyngier, kvm, kvmarm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: arm64: vgic: LR folding resurrects flushed LPIs due to hardware pending state
--

commit b351a9acf6c1fda665727c19bb864d488c3f7d75
Author: Yuchao Zhang <ndaugoing@gmail.com>

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

This commit fixes a use-after-free crash by preventing the removal of
in-flight LPIs from the AP list during a remote flush. It leaves LPIs
that are currently in a list register on the list, relying on the target
vCPU to naturally prune them after folding.

> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> --- 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);
>  		}
>  	}

[Severity: High]
Does skipping the unlink for LR-resident LPIs here prevent them from being
naturally pruned later?

When a guest clears GICR_CTLR.EnableLPIs, all pending LPIs should be
discarded. By leaving in-flight LPIs on the AP list, we rely on the target
vCPU's fold logic to handle them. However, when the vCPU exits,
vgic_v3_fold_lr() reads the hardware List Register and resurrects the
pending state:

arch/arm64/kvm/vgic/vgic-v3.c:vgic_v3_fold_lr() {
        ...
        /* Edge is the only case where we preserve the pending bit */
        if (irq->config == VGIC_CONFIG_EDGE &&
            (val & ICH_LR_PENDING_BIT))
                irq->pending_latch = true;
        ...
}

Because vgic_v3_fold_lr() unconditionally sets irq->pending_latch to true
if the hardware LR has the pending bit set, the flushed LPI's pending state
is restored.

Can this cause the target oracle to evaluate the LPI as still pending,
bypassing the intended flush and keeping the LPI alive for re-injection?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918024620.83869-1-ndaugoing@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-18  2:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-18  2:46 [PATCH 0/1] KVM: arm64: vgic: fix UAF/crash on remote LPI disable zjamg
2026-09-18  2:46 ` [PATCH 1/1] KVM: arm64: vgic: Do not remove in-flight LPIs from AP list on disable zjamg
2026-09-18  2:58   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox