Re: [PATCH v2 2/4] KVM: arm64: vgic: Don't leak the SPI array when init is retried

On Fri, 7 Aug 2026 at 16:58, Marc Zyngier <[email protected]> wrote:
>
> On Fri, 07 Aug 2026 13:15:52 +0100,
> Fuad Tabba <[email protected]> wrote:
> >
> > On Fri, 7 Aug 2026 at 11:56, <[email protected]> wrote:
> > >
> > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >
> > No, thank _you_ Sashiko!
> >
> > >
> > > Pre-existing issues:
> > > - [High] If `vgic_init()` fails after `vgic_v4_init()` succeeds, the VPE array is left allocated but `vgic_initialized` remains false. Userspace can then create additional VCPUs and retry `vgic_init()`. The second `vgic_v4_init()` returns early because the VPE array is already allocated, leaving the new VCPUs without initialized VPE IRQs.
> > ...
> > > When a new VCPU runs, `vgic_v4_load()` calls `irq_set_affinity()` with an uninitialized IRQ (0), which fails and triggers a host `WARN_ON`, causing a host-side denial of service.
> >
> > This is a separate fix. But you're right about the state.
> > kvm_arch_vcpu_precreate() only refuses vCPUs once vgic_initialized()
> > is true, so the retry admits one the VPE array does not cover.
> >
> > I don't think a VMM can drive it. kvm_vgic_setup_default_irq_routing()
> > is the only failure after vgic_v4_init(), and it builds its own
> > routing entries, so nothing short of a real -ENOMEM fails it.
> >
> > The fix I have in mind keeps vgic_v4_init() idempotent, as this patch
> > is: return early only while the vCPU count is unchanged, rebuild when
> > it grew. That needs its_free_vcpu_irqs() to clear vm->domain and
> > vm->fwnode first, which it does not today, so its_alloc_vcpu_irqs()
> > can reach its error path with a domain it has already removed.
> >
> > Let me think about it some more and I'll send something once others
> > have had a chance to digest this as well.
>
> I don't think we should grow anything opportunistically.
>
> The GICv4 code expects the number of vcpus to be fixed when first
> initialised, and if we must fail it one way or another, then
> everything should be torn down and reinitialised from scratch.
>
> I really don't want to have to think about the intermediate state.

Agreed. I'll drop the idempotent version I sketched and do teardown
and reinit instead.

Cheers,
/fuad

>
> Thanks,
>
>         M.
>
> --
> Jazz isn't dead. It just smells funny.