Re: [PATCH] ACPI: validate MADT IOAPIC entry bounds

On Wed, Jul 15, 2026 at 10:33 AM Pengpeng Hou <[email protected]> wrote:
>
> The IOAPIC hotplug lookup parses both MADT and _MAT records directly.
> The MADT walk previously used a subtable's declared length to advance the
> cursor after only locating a generic header.  The _MAT path likewise passed
> a generic header to the IOAPIC helper.
>
> Validate that a current record has a complete generic header, that its
> declared length is contained in the available record range, and that a
> typed IOAPIC record contains the full fixed IOAPIC body before reading its
> fields.  Use the same relation for both MADT and _MAT provider paths.
>
> Fixes: ecf5636dcd59 ("ACPI: Add interfaces to parse IOAPIC ID for IOAPIC hotplug")
> Signed-off-by: Pengpeng Hou <[email protected]>
> ---
>  drivers/acpi/processor_core.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index a4498357bd16..3bf076c150fa 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -336,11 +336,26 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
>  EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>
>  #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
> -static int get_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
> +static bool madt_entry_is_valid(struct acpi_subtable_header *entry,
> +                               unsigned long end)
> +{
> +       unsigned long start = (unsigned long)entry;
> +
> +       if (start >= end || end - start < sizeof(*entry))
> +               return false;
> +
> +       return entry->length >= sizeof(*entry) && entry->length <= end - start;
> +}
> +
> +static int get_ioapic_id(struct acpi_subtable_header *entry,
> +                        const unsigned long end, u32 gsi_base,
>                          u64 *phys_addr, int *ioapic_id)
>  {
>         struct acpi_madt_io_apic *ioapic = (struct acpi_madt_io_apic *)entry;
>
> +       if (!madt_entry_is_valid(entry, end) || BAD_MADT_ENTRY(ioapic, end))
> +               return 0;
> +
>         if (ioapic->global_irq_base != gsi_base)
>                 return 0;
>
> @@ -361,17 +376,19 @@ static int parse_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
>                 return apic_id;
>
>         entry = (unsigned long)madt;
> +       if (madt->header.length < sizeof(*madt))
> +               return apic_id;
>         madt_end = entry + madt->header.length;
>
>         /* Parse all entries looking for a match. */
>         entry += sizeof(struct acpi_table_madt);
> -       while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
> +       while (madt_entry_is_valid((struct acpi_subtable_header *)entry,
> +                                  madt_end)) {
>                 hdr = (struct acpi_subtable_header *)entry;
>                 if (hdr->type == ACPI_MADT_TYPE_IO_APIC &&
> -                   get_ioapic_id(hdr, gsi_base, phys_addr, &apic_id))
> +                   get_ioapic_id(hdr, madt_end, gsi_base, phys_addr, &apic_id))
>                         break;
> -               else
> -                       entry += hdr->length;
> +               entry += hdr->length;
>         }
>
>         return apic_id;
> @@ -398,7 +415,9 @@ static int parse_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
>
>         header = (struct acpi_subtable_header *)obj->buffer.pointer;
>         if (header->type == ACPI_MADT_TYPE_IO_APIC)
> -               get_ioapic_id(header, gsi_base, phys_addr, &apic_id);
> +               get_ioapic_id(header,
> +                             (unsigned long)header + obj->buffer.length,
> +                             gsi_base, phys_addr, &apic_id);
>
>  exit:
>         kfree(buffer.pointer);
> --

Applied as 7.3 material, thanks!