Re: [PATCH 3/6] irqchip/gic-v3: validate MADT records before use

[Marc Zyngier <maz@kernel.org>]

On Fri, 16 Oct 2026 15:13:54 +0100,
Jiale Yao <yaojiale02@163.com> wrote:
> 
> The GICv3 ACPI paths cast generic MADT headers to distributor,
> redistributor, and CPU interface records before confirming that the
> corresponding fixed-size structures are present. Truncated records can
> therefore cause out-of-bounds field reads while probing or setting up
> virtualization data.
>
> Check each typed MADT record before its first field access. Also reject
> a short distributor record in the version validator, which runs before
> the probe callback.
> 
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 6e1fa5b247fc..0588d6d83b31 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -2299,6 +2299,9 @@ gic_acpi_parse_madt_redist(union acpi_subtable_headers *header,
>  			(struct acpi_madt_generic_redistributor *)header;
>  	void __iomem *redist_base;
>  
> +	if (BAD_MADT_ENTRY(redist, end))
> +		return -EINVAL;
> +
>  	redist_base = ioremap(redist->base_address, redist->length);
>  	if (!redist_base) {
>  		pr_err("Couldn't map GICR region @%llx\n", redist->base_address);
> @@ -2325,6 +2328,9 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,
>  	u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
>  	void __iomem *redist_base;
>  
> +	if (BAD_MADT_ENTRY(gicc, end))
> +		return -EINVAL;
> +
>  	/* Neither enabled or online capable means it doesn't exist, skip it */
>  	if (!(gicc->flags & (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
>  		return 0;
> @@ -2391,6 +2397,9 @@ static int __init gic_acpi_match_gicc(union acpi_subtable_headers *header,
>  	struct acpi_madt_generic_interrupt *gicc =
>  				(struct acpi_madt_generic_interrupt *)header;
>  
> +	if (BAD_MADT_ENTRY(gicc, end))
> +		return -EINVAL;
> +
>  	/*
>  	 * If GICC is enabled and has valid gicr base address, then it means
>  	 * GICR base is presented via GICC. The redistributor is only known to
> @@ -2437,6 +2446,9 @@ static bool __init acpi_validate_gic_table(struct acpi_subtable_header *header,
>  	int count;
>  
>  	dist = (struct acpi_madt_generic_distributor *)header;
> +	if (header->length < sizeof(*dist))
> +		return false;
> +

Why is this open-coded instead of using the checking macro?

>  	if (dist->version != ape->driver_data)
>  		return false;
>  
> @@ -2457,6 +2469,9 @@ static int __init gic_acpi_parse_virt_madt_gicc(union acpi_subtable_headers *hea
>  	int maint_irq_mode;
>  	static int first_madt = true;
>  
> +	if (BAD_MADT_ENTRY(gicc, end))
> +		return -EINVAL;
> +

Why isn't this using BAD_MADT_GICC_ENTRY()?

>  	if (!(gicc->flags &
>  	      (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
>  		return 0;
> @@ -2547,6 +2562,9 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
>  
>  	/* Get distributor base address */
>  	dist = (struct acpi_madt_generic_distributor *)header;
> +	if (BAD_MADT_ENTRY(dist, end))
> +		return -EINVAL;
> +

Why does this need to be re-checked if the validation phased has succeeded?

>  	acpi_data.dist_base = ioremap(dist->base_address,
>  				      ACPI_GICV3_DIST_MEM_SIZE);
>  	if (!acpi_data.dist_base) {

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.