Re: [PATCH v4 02/48] irqchip/gic-v5: Provide OF IRS config frame attrs to KVM

On Fri, 2026-07-24 at 11:10 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> New issues:
> - [High] Unchecked dereference of `irs_data` in
> `gic_of_setup_kvm_info()` causes a kernel panic if CPU 0 is
> unassociated with an IRS.
> 
> Pre-existing issues:
> - [High] Failure to clear `per_cpu_irs_data` on IRS initialization
> error leaves dangling pointers, causing a Use-After-Free.
> --
> 
> --- Patch [2]: [PATCH v4 02/48] irqchip/gic-v5: Provide OF IRS config
> frame attrs to KVM ---
> commit 6022367a12dfea4087fa1185ff131015d445a1aa
> Author: Sascha Bischoff <[email protected]>
> 
> irqchip/gic-v5: Provide OF IRS config frame attrs to KVM
> 
> This commit passes an IRS config frame pointer and coherency flag to
> KVM
> via struct gic_kvm_info for OF configurations. It adds a helper to
> retrieve
> the IRS chip data for CPU 0.
> 
> > diff --git a/drivers/irqchip/irq-gic-v5-irs.c
> > b/drivers/irqchip/irq-gic-v5-irs.c
> > index f3fce0b1e25d9..607e066821b52 100644
> > --- a/drivers/irqchip/irq-gic-v5-irs.c
> > +++ b/drivers/irqchip/irq-gic-v5-irs.c
> [ ... ]
> > @@ -50,6 +48,11 @@ static void irs_writeq_relaxed(struct
> > gicv5_irs_chip_data *irs_data,
> >  	writeq_relaxed(val, irs_data->irs_base + reg_offset);
> >  }
> >  
> > +struct gicv5_irs_chip_data *gicv5_irs_get_chip_data(void)
> > +{
> > +	return per_cpu(per_cpu_irs_data, 0);
> > +}
> 
> [Severity: High]
> This is a pre-existing issue, but does returning this per-cpu pointer
> risk a
> use-after-free if the IRS initialization failed earlier?
> 
> In gicv5_irs_of_init(), gicv5_irs_of_init_affinity() populates
> per_cpu(per_cpu_irs_data, cpu) with a newly allocated irs_data. If a
> subsequent step fails, the error path frees irs_data but does not
> clear
> the per-cpu pointers for the associated CPUs.
> 
> If CPU 0 belonged to a failed IRS, would this helper now return a
> dangling
> pointer that gets unconditionally dereferenced by
> gic_of_setup_kvm_info()
> during boot?
> 
> > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-
> > gic-v5.c
> > index c04523a438cee..e7a7aedcfaf78 100644
> > --- a/drivers/irqchip/irq-gic-v5.c
> > +++ b/drivers/irqchip/irq-gic-v5.c
> [ ... ]
> > @@ -1128,6 +1128,8 @@ static struct gic_kvm_info gic_v5_kvm_info
> > __initdata;
> >  
> >  static void __init gic_of_setup_kvm_info(struct device_node *node)
> >  {
> > +	struct gicv5_irs_chip_data *irs_data =
> > gicv5_irs_get_chip_data();
> > +
> >  	/*
> >  	 * If we don't have native GICv5 virtualisation support,
> > then
> >  	 * we also don't have FEAT_GCIE_LEGACY - the architecture
> > @@ -1140,6 +1142,9 @@ static void __init
> > gic_of_setup_kvm_info(struct device_node *node)
> >  
> >  	gic_v5_kvm_info.type = GIC_V5;
> >  
> > +	gic_v5_kvm_info.gicv5_irs.base = irs_data->irs_base;
> > +	gic_v5_kvm_info.gicv5_irs.non_coherent = !!(irs_data-
> > >flags & IRS_FLAGS_NON_COHERENT);
> 
> [Severity: High]
> Can irs_data be NULL here, leading to a kernel panic when
> dereferencing
> irs_data->irs_base and irs_data->flags?
> 
> If the system's Device Tree topology does not map logical CPU 0 to
> any IRS,
> gicv5_irs_get_chip_data() returns NULL. Would it be safer to check if
> irs_data is valid before assigning these fields?
> 

I will be posting a fixup for this as part of a separate series. On a
multi-IRS system we continue if one of the IRSes fails to init, in
which case we could have some CPUs unbound from an IRS at this point
albeit with stale pointers.

Thanks,
Sascha