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

[sashiko-bot@kernel.org]

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 <Sascha.Bischoff@arm.com>

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=2