Re: [PATCH v4 01/48] irqchip/gic-v5: Allow KVM setup without a maintenance IRQ

On Fri, 2026-07-24 at 11:21 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Medium] Exposing a latent memory leak of `gic_kvm_info` in KVM's
> initialization error path when a required maintenance IRQ is absent
> on GICv5 legacy-capable systems.
> --
> 
> --- Patch [1]: [PATCH v4 01/48] irqchip/gic-v5: Allow KVM setup
> without a maintenance IRQ ---
> commit 4a1c6e5108caaddf8c492f48ac4cb4568ab60c52
> Author: Sascha Bischoff <[email protected]>
> 
> irqchip/gic-v5: Allow KVM setup without a maintenance IRQ
> 
> This commit modifies the GICv5 initialization to no longer reject KVM
> setup
> solely due to a missing maintenance interrupt. Instead, it delegates
> this
> validation to KVM by passing a specific configuration flag based on
> legacy
> support capabilities.
> 
> > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-
> > gic-v5.c
> > index e9d1795235a66..c04523a438cee 100644
> > --- a/drivers/irqchip/irq-gic-v5.c
> > +++ b/drivers/irqchip/irq-gic-v5.c
> > @@ -1143,10 +1143,18 @@ static void __init
> > gic_of_setup_kvm_info(struct device_node *node)
> >  	/* GIC Virtual CPU interface maintenance interrupt */
> >  	gic_v5_kvm_info.no_maint_irq_mask = false;
> >  	gic_v5_kvm_info.maint_irq = irq_of_parse_and_map(node, 0);
> > -	if (!gic_v5_kvm_info.maint_irq) {
> > -		pr_warn("cannot find GICv5 virtual CPU interface
> > maintenance interrupt\n");
> > -		return;
> > -	}
> > +
> > +	/*
> > +	 * We require an MI if we have legacy support, but don't,
> > otherwise.
> > +	 * Given that there's an existing flag to convey that an
> > MI isn't
> > +	 * needed, we (ab)use it to tell KVM that the MI isn't
> > needed if we
> > +	 * don't support legacy.
> > +	 *
> > +	 * The check for ARM64_HAS_GICV5_LEGACY explicitly doesn't
> > use
> > +	 * cpus_have_final_cap() here as we run too early.
> > +	 */
> > +	if (!cpus_have_cap(ARM64_HAS_GICV5_LEGACY) &&
> > !gic_v5_kvm_info.maint_irq)
> > +		gic_v5_kvm_info.no_maint_irq_mask = true;
> >  
> >  	vgic_set_kvm_info(&gic_v5_kvm_info);
> 
> [Severity: Medium]
> By removing the early return here, does this expose a memory leak in
> KVM's
> initialization error path?
> 
> If a system has ARM64_HAS_GICV5_LEGACY support but the maintenance
> interrupt
> is missing, we now pass no_maint_irq_mask = false and maint_irq = 0
> to
> vgic_set_kvm_info() in drivers/irqchip/irq-gic-
> v5.c:gic_of_setup_kvm_info().
> 
> When this configuration reaches KVM:
> 
> arch/arm64/kvm/vgic/vgic-init.c:kvm_vgic_hyp_init() {
>     ...
>     if (has_mask && !gic_kvm_info->maint_irq) {
>         kvm_err("No vgic maintenance irq\n");
>         return -ENXIO;
>     }
>     ...
> }
> 
> It looks like kvm_vgic_hyp_init() returns -ENXIO without freeing the
> dynamically allocated gic_kvm_info structure, causing a memory leak.

We've previously discussed this exact issue as you raised it last time.
This is the way that the existing code works for a GICv3 system too.
For the time being this is fine. I intend on posting a follow up patch
at some point to clean things up, but that is not included with this
series as it simply does not belong here.

Thanks,
Sascha

> 
> >  }
>