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

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.

Cheers,
/fuad





/fuad

> --
>
> --- Patch [2]: [PATCH v2 2/4] KVM: arm64: vgic: Don't leak the SPI array when init is retried ---
> commit 11b9c8f9971b11e30ffc049a56071fafe4c7df73
> Author: Fuad Tabba <[email protected]>
>
> KVM: arm64: vgic: Don't leak the SPI array when init is retried
>
> This patch fixes a memory leak where up to 960 struct vgic_irq could be leaked
> if kvm_vgic_setup_default_irq_routing() fails and userspace retries
> KVM_DEV_ARM_VGIC_CTRL_INIT. It resolves this by returning early from
> kvm_vgic_dist_init() if dist->spis is already allocated.
>
> > diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> > index 907057881b26a..d4cf143f3ae6b 100644
> > --- a/arch/arm64/kvm/vgic/vgic-init.c
> > +++ b/arch/arm64/kvm/vgic/vgic-init.c
> > @@ -210,6 +210,9 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
> >       struct kvm_vcpu *vcpu0 = kvm_get_vcpu(kvm, 0);
> >       int i;
> >
> > +     if (dist->spis)
> > +             return 0;
> > +
>
> [Severity: High]
> This is a pre-existing issue, but since the commit message mentions mirroring
> the early return behavior of vgic_v4_init(), does that original function
> have a stale state bug on retry?
>
> If vgic_init() fails after vgic_v4_init() successfully allocates
> dist->its_vm.vpes, vgic_initialized remains false. If userspace then creates
> an additional vCPU and retries KVM_DEV_ARM_VGIC_CTRL_INIT, vgic_v4_init()
> returns early on the second attempt without allocating a VPE for the newly
> created vCPU.
>
> When the new vCPU runs, vgic_v3_load() unconditionally calls vgic_v4_load().
> Because the VPE was never properly initialized for this new vCPU,
> vgic_v4_load() passes an uninitialized vpe->irq (0) to irq_set_affinity():
>
> arch/arm64/kvm/vgic/vgic-v4.c:vgic_v4_load() {
>         ...
>         err = irq_set_affinity(vpe->irq, cpumask_of(smp_processor_id()));
>         ...
> }
>
> This fails with -EINVAL and triggers a WARN_ON in vgic_v3_load():
>
> arch/arm64/kvm/vgic/vgic-v3.c:vgic_v3_load() {
>         ...
>         WARN_ON(vgic_v4_load(vcpu));
>         ...
> }
>
> Could an unprivileged VMM repeatedly trigger this host WARN_ON by
> deliberately failing the first initialization and adding a vCPU before
> retrying?
>
> Is this something that should be addressed alongside or before this leak fix?
>
> >       dist->active_spis = (atomic_t)ATOMIC_INIT(0);
> >       dist->spis = kzalloc_objs(struct vgic_irq, nr_spis, GFP_KERNEL_ACCOUNT);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2