[PATCH] ACPI: validate MADT IOAPIC entry bounds

Pengpeng Hou posted 1 patch 1 week, 3 days ago
drivers/acpi/processor_core.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
[PATCH] ACPI: validate MADT IOAPIC entry bounds
Posted by Pengpeng Hou 1 week, 3 days ago
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 <pengpeng@iscas.ac.cn>
---
 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);
-- 
2.43.0
Re: [PATCH] ACPI: validate MADT IOAPIC entry bounds
Posted by Rafael J. Wysocki (Intel) 4 days, 3 hours ago
On Wed, Jul 15, 2026 at 10:33 AM Pengpeng Hou <pengpeng@iscas.ac.cn> 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 <pengpeng@iscas.ac.cn>
> ---
>  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!