[PATCH] acpi: acpidump: handle invalid MADT table data (LP: #1278422)

Keng-Yu Lin kengyu at canonical.com
Thu Feb 13 09:06:12 UTC 2014


On Mon, Feb 10, 2014 at 8:53 PM, Colin King <colin.king at canonical.com> wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> acpidump can dump out extraneous sub tables if the MADT is longer
> than the number of subtables provided and/or sub table type and
> lengths are wrong.  Add some sanity checking.  Also, re-work the
> way sub table info is dumped out and add some missing fields.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>  src/acpi/acpidump/acpidump.c | 59 +++++++++++++++++++++++++++-----------------
>  1 file changed, 36 insertions(+), 23 deletions(-)
>
> diff --git a/src/acpi/acpidump/acpidump.c b/src/acpi/acpidump/acpidump.c
> index 636a97c..e9dccde 100644
> --- a/src/acpi/acpidump/acpidump.c
> +++ b/src/acpi/acpidump/acpidump.c
> @@ -766,7 +766,7 @@ static void acpidump_xsdt(fwts_framework *fw, const fwts_acpi_table_info *table)
>  static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>  {
>         uint8_t *data = (uint8_t *)table->data;
> -       size_t offset = 0, length = table->length;
> +       size_t offset = 0;
>         int i = 0;
>
>         static const fwts_acpidump_field fields[] = {
> @@ -776,21 +776,47 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                 FIELD_END
>         };
>
> -       acpi_dump_table_fields(fw, data, fields, 0, length);
> +       static const char *types[] = {
> +               "Processor Local APIC",         /* 0 */
> +               "I/O APIC",                     /* 1 */
> +               "Interrupt Source Override",    /* 2 */
> +               "Non-maskable Interrupt Source (NMI)", /* 3 */
> +               "Local APIC NMI",               /* 4 */
> +               "Local APIC Address Override",  /* 5 */
> +               "I/O SAPIC",                    /* 6 */
> +               "Local SAPIC",                  /* 7 */
> +               "Platform Interrupt Sources",   /* 8 */
> +               "Processor Local x2APIC",       /* 9 */
> +               "Local x2APIC NMI",             /* 10 */
> +               "GIC",                          /* 11 */
> +               "GIC Distributor",              /* 12 */
> +               "OEM Defined"                   /* 13 */
> +       };
> +       static const fwts_acpidump_field fields_sub_table_header[] = {
> +               FIELD_STRS("  Type", fwts_acpi_madt_sub_table_header, type, types, 13),
> +               FIELD_UINT("  Length", fwts_acpi_madt_sub_table_header, length),
> +               FIELD_END
> +       };
> +
> +       acpi_dump_table_fields(fw, data, fields, 0, table->length);
>
> +       offset += sizeof(fwts_acpi_table_madt);
>         data += sizeof(fwts_acpi_table_madt);
> -       length -= sizeof(fwts_acpi_table_madt);
>
> -       while (length > (int)sizeof(fwts_acpi_madt_sub_table_header)) {
> +
> +       while (offset < table->length) {
>                 int skip = 0;
>                 fwts_acpi_madt_sub_table_header *hdr = (fwts_acpi_madt_sub_table_header *)data;
>
> +               /* Check for unknown type or illegal size */
> +               if (hdr->type > 12 || hdr->length < 6)
> +                       break;
> +
> +               fwts_log_info_verbatum(fw, "APIC Structure #%d:", ++i);
> +               __acpi_dump_table_fields(fw, data, fields_sub_table_header, offset);
> +
>                 data += sizeof(fwts_acpi_madt_sub_table_header);
> -               length -= sizeof(fwts_acpi_madt_sub_table_header);
>                 offset += sizeof(fwts_acpi_madt_sub_table_header);
> -
> -               fwts_log_nl(fw);
> -               fwts_log_info_verbatum(fw, "APIC Structure #%d:", ++i);
>
>                 switch (hdr->type) {
>                 case 0: {
> @@ -801,7 +827,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_BITF("   Enabled",    fwts_acpi_madt_processor_local_apic, flags, 1, 0),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Processor Local APIC:");
>                                 __acpi_dump_table_fields(fw, data, fields_processor_local_apic, offset);
>                                 skip = sizeof(fwts_acpi_madt_processor_local_apic);
>                         }
> @@ -813,7 +838,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  Global IRQ Base", fwts_acpi_madt_io_apic, global_irq_base),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " I/O APIC:");
>                                 __acpi_dump_table_fields(fw, data, fields_io_apic, offset);
>                                 skip = sizeof(fwts_acpi_madt_io_apic);
>                         }
> @@ -824,9 +848,10 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  Source",          fwts_acpi_madt_interrupt_override, source),
>                                         FIELD_UINT("  Gbl Sys Int",     fwts_acpi_madt_interrupt_override, gsi),
>                                         FIELD_UINT("  Flags",           fwts_acpi_madt_interrupt_override, flags),
> +                                       FIELD_BITF("   Polarity",       fwts_acpi_madt_interrupt_override, flags, 2, 0),
> +                                       FIELD_BITF("   Trigger Mode",   fwts_acpi_madt_interrupt_override, flags, 2, 2),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Interrupt Source Override:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_interrupt_override, offset);
>                                 skip = sizeof(fwts_acpi_madt_interrupt_override);
>                         }
> @@ -837,7 +862,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  Gbl Sys Int",     fwts_acpi_madt_nmi, gsi),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Non-maskable Interrupt Source (NMI):");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_nmi, offset);
>                                 skip = sizeof(fwts_acpi_madt_nmi);
>                         }
> @@ -849,7 +873,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  Local APIC LINT", fwts_acpi_madt_local_apic_nmi, local_apic_lint),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Local APIC NMI:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_local_apic_nmi, offset);
>                                 skip = sizeof(fwts_acpi_madt_local_apic_nmi);
>                         }
> @@ -859,7 +882,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  Local APIC Addr", fwts_acpi_madt_local_apic_addr_override, address),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Local APIC Address Override:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_local_apic_addr_override, offset);
>                                 skip = sizeof(fwts_acpi_madt_local_apic_addr_override);
>                         }
> @@ -871,7 +893,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  I/O SAPIC Addr",  fwts_acpi_madt_io_sapic, address),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " I/O SAPIC:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_io_sapic, offset);
>                                 skip = sizeof(fwts_acpi_madt_io_sapic);
>                         }
> @@ -887,7 +908,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  UID String",      fwts_acpi_madt_local_sapic, uid_string),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Local SAPIC:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_local_sapic, offset);
>                                 skip = (sizeof(fwts_acpi_madt_local_sapic) +
>                                         strlen(local_sapic->uid_string) + 1);
> @@ -904,7 +924,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  PIS Flags",       fwts_acpi_madt_platform_int_source, pis_flags),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Platform Interrupt Sources:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_local_sapic, offset);
>                                 skip = (sizeof(fwts_acpi_madt_platform_int_source));
>                         }
> @@ -916,7 +935,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  Processor UID",   fwts_acpi_madt_local_x2apic, processor_uid),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Processor Local x2APIC:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_local_x2apic, offset);
>                                 skip = (sizeof(fwts_acpi_madt_local_x2apic));
>                         }
> @@ -928,7 +946,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  LINT#",           fwts_acpi_madt_local_x2apic_nmi, local_x2apic_lint),
>                                         FIELD_END
>                                 };
> -                               fwts_log_info_verbatum(fw, " Local x2APIC NMI:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_local_x2apic_nmi, offset);
>                                 skip = (sizeof(fwts_acpi_madt_local_x2apic_nmi));
>                         }
> @@ -944,7 +961,6 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  Parked Address",  fwts_acpi_madt_gic, parked_address),
>                                         FIELD_UINT("  Phys. Base. Addr",fwts_acpi_madt_gic, physical_base_address),
>                                 };
> -                               fwts_log_info_verbatum(fw, " GIC:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_gic, offset);
>                                 skip = (sizeof(fwts_acpi_madt_gic));
>                         }
> @@ -957,19 +973,16 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>                                         FIELD_UINT("  Sys Vector Base", fwts_acpi_madt_gicd, system_vector_base),
>                                         FIELD_UINT("  Reserved",        fwts_acpi_madt_gicd, reserved2),
>                                 };
> -                               fwts_log_info_verbatum(fw, " GIC Distributor:");
>                                 __acpi_dump_table_fields(fw, data, fields_madt_gicd, offset);
>                                 skip = (sizeof(fwts_acpi_madt_gicd));
>                         }
>                         break;
>                 default:
> -                       fwts_log_info_verbatum(fw, " Reserved for OEM use:");
>                         skip = 0;
>                         break;
>                 }
>                 data   += skip;
>                 offset += skip;
> -               length -= skip;
>         }
>  }
>
> --
> 1.9.rc1
>

Acked-by: Keng-Yu Lin <kengyu at canonical.com>



More information about the fwts-devel mailing list