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

Marc Zyngier <[email protected]> Fri, 07 Aug 2026 16:59:21 +0100
Newsgroups dev.linux.lists.kvmarm,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
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.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.