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

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.
--

--- 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