Re: [PATCH v2 1/4] KVM: arm64: vgic-its: Free the caches when GITS_BASER changes

Sashiko, this one is real.

On Fri, 21 Aug 2026 at 07:57, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Pre-existing race condition in ITS MMIO handler allows MSI injection while ITS is disabled due to a stale translation cache entry.
> --
>
> --- Patch [1]: [PATCH v2 1/4] KVM: arm64: vgic-its: Free the caches when GITS_BASER changes ---
> commit 98c89973da07ed23e9bd8348bfd2cf61333c4e66
> Author: Fuad Tabba <[email protected]>
>
> KVM: arm64: vgic-its: Free the caches when GITS_BASER changes
>
> A guest that disables the ITS and changes GITS_BASER<n> with VALID still set
> keeps the mapped devices and collections. This patch frees the list and
> drops the translation cache whenever the stored value changes.
>
> Link: https://lore.kernel.org/all/[email protected]/
>
> > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> > index 9e782a4fea7e5..ab89b0138efd0 100644
> > --- a/arch/arm64/kvm/vgic/vgic-its.c
> > +++ b/arch/arm64/kvm/vgic/vgic-its.c
>
> [ ... ]
>
> > @@ -1702,6 +1705,8 @@ static void vgic_mmio_write_its_baser(struct kvm *kvm,
> >                       vgic_its_free_collection_list(kvm, its);
> >                       break;
> >               }
> > +             /* A concurrent injection may have cached a translation. */
> > +             vgic_its_invalidate_cache(its);
>
> [Severity: High]
> This is a pre-existing issue, but the commit message explicitly notes that
> an injection can still cache an entry after the ITS is disabled.
>
> Is it possible for a subsequent MSI to trigger an unexpected LPI injection?

Yes, and it violates the spec: with GITS_CTLR.Enabled == 0 writes to
GITS_TRANSLATER are ignored, so nothing should be injected.

It was reported in June [1] and is being worked on.

Cheers,
/fuad

[1] https://lore.kernel.org/all/ah6Lsi4MfKUU6wBR@v4bel/

>
> If Thread 1 (e.g. irqfd) calls vgic_its_inject_msi(), takes its_lock, and
> enters vgic_its_resolve_lpi() in arch/arm64/kvm/vgic/vgic-its.c, it reads
> its->enabled as true.
>
> If Thread 2 (guest vCPU) concurrently disables the ITS via
> vgic_mmio_write_its_ctlr(), it acquires cmd_lock (but not its_lock), sets
> its->enabled to false, and clears the cache.
>
> Thread 1 could then resume, finish resolving the LPI, and call
> vgic_its_cache_translation() to insert the entry into the cache, ignoring the
> fact that its->enabled is now false.
>
> A subsequent MSI would then hit vgic_its_inject_cached_translation(), which
> does a fast-path lookup without checking its->enabled, successfully injecting
> the MSI.
>
> Could this violate the GICv3 specification by delivering LPIs while the ITS
> is explicitly disabled, potentially crashing a guest OS during kexec?
>
> >               mutex_unlock(&its->its_lock);
> >       }
> >  }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1