From: Oliver Upton <oupton@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: Marc Zyngier <maz@kernel.org>,
kvm@vger.kernel.org, kvmarm@lists.linux.dev
Subject: Re: [PATCH] KVM: arm64: Handle race between interrupt affinity change and LPI disabling
Date: Tue, 16 Jun 2026 11:51:25 -0700 [thread overview]
Message-ID: <ajGbLXf8Yc67cSI9@kernel.org> (raw)
In-Reply-To: <20260615183314.65A571F00A3A@smtp.kernel.org>
On Mon, Jun 15, 2026 at 06:33:14PM +0000, sashiko-bot@kernel.org wrote:
> > @@ -795,7 +796,11 @@ static void vgic_prune_ap_list(struct kvm_vcpu *vcpu)
> > continue;
> > }
> >
> > - /* This interrupt looks like it has to be migrated. */
> > + /*
> > + * This interrupt looks like it has to be migrated,
> > + * make sure it is kept alive while locks are dropped.
> > + */
> > + vgic_get_irq_ref(irq);
>
> [Severity: Critical]
> This isn't a bug introduced by this patch, but could the lock-dropping window
> here lead to a double list_del() and a use-after-free under certain race
> conditions?
>
> When vgic_prune_ap_list() drops the irq and ap_list locks shortly after this
> reference is taken, another thread could concurrently execute
> vgic_flush_pending_lpis() above and remove the irq from the list.
>
> If a third thread then concurrently injects the same LPI before the locks are
> reacquired, the irq_lock is taken and pending_latch is set to true again.
>
> When vgic_prune_ap_list() reacquires the locks further down in the function,
> the target_vcpu oracle check will pass because the interrupt is pending again:
>
> arch/arm64/kvm/vgic/vgic.c:vgic_prune_ap_list() {
> ...
> if (target_vcpu == vgic_target_oracle(irq)) {
> struct vgic_cpu *new_cpu = &target_vcpu->arch.vgic_cpu;
>
> list_del(&irq->ap_list);
> irq->vcpu = target_vcpu;
> ...
> }
>
> Because there is no check verifying that irq->vcpu is still equal to the
> source vcpu, does this unconditionally execute list_del() on a list entry
> that has already been removed and poisoned?
>
> Would this also bypass acquiring a new list reference, leaving the LPI on the
> new list with a deficient reference count if it avoids a crash?
Urgh... Marc, I think we need Hyunwoo's patch in addition to yours. Even
though this 3-way race seems improbable, it does appear possible :/
[*]: https://lore.kernel.org/kvmarm/aiHnI1mu6SGQrgnz@v4bel/
Thanks,
Oliver
prev parent reply other threads:[~2026-06-16 18:51 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-15 18:16 [PATCH] KVM: arm64: Handle race between interrupt affinity change and LPI disabling Marc Zyngier
2026-06-15 18:33 ` sashiko-bot
2026-06-16 18:51 ` Oliver Upton [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ajGbLXf8Yc67cSI9@kernel.org \
--to=oupton@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=maz@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.