Re: [PATCH v2 01/20] ACPI: irqchip: validate MADT entry bounds before probing

[Lorenzo Pieralisi <lpieralisi@kernel.org>]

On Thu, Oct 29, 2026 at 11:52:32AM +0800, Jiale Yao wrote:
> IRQ chip probing invokes validation and probe callbacks after the common
> table walker has only established that a generic subtable header is
> readable. A malformed length can leave callback field accesses outside
> the MADT.
> 
> Validate the generic header length and ensure the complete declared
> entry is contained in the table before invoking IRQ chip callbacks.
> 
> Fixes: e647b532275b ("ACPI: Add early device probing infrastructure")
> Cc: stable@vger.kernel.org

I don't think this patch fixes anything (actually you don't know, it
might break kernels that are currently booting).

Remove the stable tag (on the whole series, though others might have a
different opinion). If this gets merged, goes to stable and then
we discover it triggers a regression (on existing platforms whose
ACPI tables are broken but the system is working) it would be very
annoying.

> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  drivers/acpi/scan.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 0b17a7e51956..10e8d736f44b 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2896,6 +2896,13 @@ static DEFINE_MUTEX(acpi_probe_mutex);
>  static int __init acpi_match_madt(union acpi_subtable_headers *header,
>  				  const unsigned long end)
>  {
> +	unsigned long start = (unsigned long)header;
> +
> +	if (start >= end || end - start < sizeof(header->common) ||
> +	    header->common.length < sizeof(header->common) ||
> +	    header->common.length > end - start)
> +		return -EINVAL;

This is a check that AFAICS could be done in acpi_parse_entries_array() once
for all (where you also know header->common is what you need checking, btw). 

Actually, what's missing there to avoid peppering the kernel with
BAD_MADT_ENTRY checks is the entry size (that we could add with a LUT).

The downsize is that by centralizing it, if anything breaks (which is
possible) we have to revert it and therefore remove the check for all
consumers.

I don't know. Rafael, all, thoughts on this ?

Thanks,
Lorenzo

> +
>  	if (!ape->subtable_valid || ape->subtable_valid(&header->common, ape))
>  		if (!ape->probe_subtbl(header, end))
>  			acpi_probe_count++;
> -- 
> 2.34.1
>