Re: [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure

On Thu, 20 Aug 2026 at 14:03, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] vgic_v4_init() truncates its_vm.nr_vpes on vgic_v4_request_vpe_irq() failure, causing a resource leak of previously allocated VPE IRQs and SGIs during teardown.

https://lore.kernel.org/all/CA+EHjTzf4C6bdwTGWNN1HeapFWEFWm-AJtxi5gQKHekRS2n8Aw@mail.gmail.com/

/fuad

> --
>
> --- Patch [2]: [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure ---
> commit 09380ca1832d1b65e5229cfbb8e1f8ab95b65846
> Author: Fuad Tabba <[email protected]>
>
> irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure
>
> This commit fixes a resource leak in its_alloc_vcpu_irqs() where SGI
> domains and vPE irqs were not properly freed if the allocation loop
> failed midway.
>
> > diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> > index 754839e409f88..f707a3cb281aa 100644
> > --- a/drivers/irqchip/irq-gic-v4.c
> > +++ b/drivers/irqchip/irq-gic-v4.c
>
> [ ... ]
>
> > @@ -189,11 +191,14 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
> >               vm->vpes[i]->irq = vpe_base_irq + i;
> >               ret = its_alloc_vcpu_sgis(vm->vpes[i], i);
> >               if (ret)
> > -                     goto err;
> > +                     goto err_free_irqs;
> >       }
> >
> >       return 0;
> >
> > +err_free_irqs:
> > +     its_free_sgi_irqs(vm);
> > +     irq_domain_free_irqs(vpe_base_irq, vm->nr_vpes);
> >  err:
>
> [Severity: High]
> This is a pre-existing issue, but does the teardown path in
> its_free_vcpu_irqs() fully clean up resources if its_vm.nr_vpes gets
> truncated?
>
> If vgic_v4_init() in arch/arm64/kvm/vgic/vgic-v4.c fails to request an IRQ
> via vgic_v4_request_vpe_irq(), it explicitly truncates nr_vpes to avoid
> calling free_irq() on unrequested IRQs:
>
> arch/arm64/kvm/vgic/vgic-v4.c:vgic_v4_init() {
>     ...
>     /* Trick: adjust the number of vpes so we know how many to nuke on teardown... */
>     dist->its_vm.nr_vpes = i;
>     break;
>     ...
> }
>
> When cleanup subsequently calls its_free_vcpu_irqs(), it relies on this
> truncated nr_vpes value:
>
> drivers/irqchip/irq-gic-v4.c:its_free_vcpu_irqs() {
>     ...
>     its_free_sgi_irqs(vm);
>     irq_domain_free_irqs(vm->vpes[0]->irq, vm->nr_vpes);
>     ...
> }
>
> Will this sequence leave the previously allocated IRQ domains, fwnodes, and
> SGI domains orphaned for the vCPUs beyond the failure index, since they are
> no longer tracked by the truncated nr_vpes count?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2