lkml.org 
[lkml]   [2004]   [Jan]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRFC: ACPI table overflow handling
    From
    Hi,

    We ran into a problem with the current ACPI NUMA code overflowing an
    array if the number of CPUs found in the SRAT was greater than NR_CPUS.

    Looking at the acpi_table_parse_madt() I noticed that it doesn't do
    anything to handle overflows, instead it relies on hacks in the
    call-back functions to come up with a check instead, the various checks
    in place are somewhat inconsistent in their implementation.

    I could just hack the NUMA srat_num_cpus handling code to have a limit
    as IMHO it is a lot cleaner to improve the acpi_table_parse_madt() API
    by adding a max_entries argument and then have acpi_table_parse_madt
    spit out a warning if it found too many entries.

    Patch attached, it was generated against 2.6.0-test11 but applies fine
    with a little fuzz to 2.6.1-rc2 (my apologies for not having fixed
    x86_64 yet, it should be trivial to apply the i386 changes to the x86_64
    tree as well).

    I'd appreciate comments/oppinions/suggestions?

    Thanks,
    Jes

    diff -urN -X /usr/people/jes/exclude-linux --exclude=io --exclude=sn --exclude='qla1280.[ch]' orig/linux-2.6.0-test11-ia64/arch/i386/kernel/acpi/boot.c linux-2.6.0-test11-ia64/arch/i386/kernel/acpi/boot.c
    --- orig/linux-2.6.0-test11-ia64/arch/i386/kernel/acpi/boot.c Wed Nov 26 12:45:28 2003
    +++ linux-2.6.0-test11-ia64/arch/i386/kernel/acpi/boot.c Thu Jan 8 05:11:49 2004
    @@ -44,8 +44,8 @@
    extern int acpi_irq;
    extern int acpi_ht;

    -int acpi_lapic = 0;
    -int acpi_ioapic = 0;
    +int acpi_lapic;
    +int acpi_ioapic;

    /* --------------------------------------------------------------------------
    Boot-time Configuration
    @@ -418,7 +418,7 @@
    * and (optionally) overriden by a LAPIC_ADDR_OVR entry (64-bit value).
    */

    - result = acpi_table_parse_madt(ACPI_MADT_LAPIC_ADDR_OVR, acpi_parse_lapic_addr_ovr);
    + result = acpi_table_parse_madt(ACPI_MADT_LAPIC_ADDR_OVR, acpi_parse_lapic_addr_ovr, 65536);
    if (result < 0) {
    printk(KERN_ERR PREFIX "Error parsing LAPIC address override entry\n");
    return result;
    @@ -426,7 +426,8 @@

    mp_register_lapic_address(acpi_lapic_addr);

    - result = acpi_table_parse_madt(ACPI_MADT_LAPIC, acpi_parse_lapic);
    + result = acpi_table_parse_madt(ACPI_MADT_LAPIC, acpi_parse_lapic,
    + MAX_APICS);
    if (!result) {
    printk(KERN_ERR PREFIX "No LAPIC entries present\n");
    /* TBD: Cleanup to allow fallback to MPS */
    @@ -438,7 +439,7 @@
    return result;
    }

    - result = acpi_table_parse_madt(ACPI_MADT_LAPIC_NMI, acpi_parse_lapic_nmi);
    + result = acpi_table_parse_madt(ACPI_MADT_LAPIC_NMI, acpi_parse_lapic_nmi, 65536);
    if (result < 0) {
    printk(KERN_ERR PREFIX "Error parsing LAPIC NMI entry\n");
    /* TBD: Cleanup to allow fallback to MPS */
    @@ -475,8 +476,8 @@
    return 1;
    }

    - result = acpi_table_parse_madt(ACPI_MADT_IOAPIC, acpi_parse_ioapic);
    - if (!result) {
    + result = acpi_table_parse_madt(ACPI_MADT_IOAPIC, acpi_parse_ioapic, MAX_IO_APICS);
    + if (!result) {
    printk(KERN_ERR PREFIX "No IOAPIC entries present\n");
    return -ENODEV;
    }
    @@ -488,14 +489,14 @@
    /* Build a default routing table for legacy (ISA) interrupts. */
    mp_config_acpi_legacy_irqs();

    - result = acpi_table_parse_madt(ACPI_MADT_INT_SRC_OVR, acpi_parse_int_src_ovr);
    + result = acpi_table_parse_madt(ACPI_MADT_INT_SRC_OVR, acpi_parse_int_src_ovr, NR_IRQ_VECTORS);
    if (result < 0) {
    printk(KERN_ERR PREFIX "Error parsing interrupt source overrides entry\n");
    /* TBD: Cleanup to allow fallback to MPS */
    return result;
    }

    - result = acpi_table_parse_madt(ACPI_MADT_NMI_SRC, acpi_parse_nmi_src);
    + result = acpi_table_parse_madt(ACPI_MADT_NMI_SRC, acpi_parse_nmi_src, NR_IRQ_VECTORS);
    if (result < 0) {
    printk(KERN_ERR PREFIX "Error parsing NMI SRC entry\n");
    /* TBD: Cleanup to allow fallback to MPS */
    diff -urN -X /usr/people/jes/exclude-linux --exclude=io --exclude=sn --exclude='qla1280.[ch]' orig/linux-2.6.0-test11-ia64/arch/ia64/kernel/acpi.c linux-2.6.0-test11-ia64/arch/ia64/kernel/acpi.c
    --- orig/linux-2.6.0-test11-ia64/arch/ia64/kernel/acpi.c Wed Nov 26 12:44:07 2003
    +++ linux-2.6.0-test11-ia64/arch/ia64/kernel/acpi.c Thu Jan 8 03:03:46 2004
    @@ -550,29 +550,34 @@

    /* Local APIC */

    - if (acpi_table_parse_madt(ACPI_MADT_LAPIC_ADDR_OVR, acpi_parse_lapic_addr_ovr) < 0)
    + /*
    + * For the callback functions that do not have problems with maximum
    + * table sizes internally, we use 65536 - please adjust if you ever
    + * build a system with more ;-)
    + */
    + if (acpi_table_parse_madt(ACPI_MADT_LAPIC_ADDR_OVR, acpi_parse_lapic_addr_ovr, 65536) < 0)
    printk(KERN_ERR PREFIX "Error parsing LAPIC address override entry\n");

    - if (acpi_table_parse_madt(ACPI_MADT_LSAPIC, acpi_parse_lsapic) < 1)
    + if (acpi_table_parse_madt(ACPI_MADT_LSAPIC, acpi_parse_lsapic, NR_CPUS) < 1)
    printk(KERN_ERR PREFIX "Error parsing MADT - no LAPIC entries\n");

    - if (acpi_table_parse_madt(ACPI_MADT_LAPIC_NMI, acpi_parse_lapic_nmi) < 0)
    + if (acpi_table_parse_madt(ACPI_MADT_LAPIC_NMI, acpi_parse_lapic_nmi, 65536) < 0)
    printk(KERN_ERR PREFIX "Error parsing LAPIC NMI entry\n");

    /* I/O APIC */

    - if (acpi_table_parse_madt(ACPI_MADT_IOSAPIC, acpi_parse_iosapic) < 1)
    + if (acpi_table_parse_madt(ACPI_MADT_IOSAPIC, acpi_parse_iosapic, 256) < 1)
    printk(KERN_ERR PREFIX "Error parsing MADT - no IOSAPIC entries\n");

    /* System-Level Interrupt Routing */

    - if (acpi_table_parse_madt(ACPI_MADT_PLAT_INT_SRC, acpi_parse_plat_int_src) < 0)
    + if (acpi_table_parse_madt(ACPI_MADT_PLAT_INT_SRC, acpi_parse_plat_int_src, ACPI_MAX_PLATFORM_INTERRUPTS) < 0)
    printk(KERN_ERR PREFIX "Error parsing platform interrupt source entry\n");

    - if (acpi_table_parse_madt(ACPI_MADT_INT_SRC_OVR, acpi_parse_int_src_ovr) < 0)
    + if (acpi_table_parse_madt(ACPI_MADT_INT_SRC_OVR, acpi_parse_int_src_ovr, 65536) < 0)
    printk(KERN_ERR PREFIX "Error parsing interrupt source overrides entry\n");

    - if (acpi_table_parse_madt(ACPI_MADT_NMI_SRC, acpi_parse_nmi_src) < 0)
    + if (acpi_table_parse_madt(ACPI_MADT_NMI_SRC, acpi_parse_nmi_src, 65536) < 0)
    printk(KERN_ERR PREFIX "Error parsing NMI SRC entry\n");
    skip_madt:

    diff -urN -X /usr/people/jes/exclude-linux --exclude=io --exclude=sn --exclude='qla1280.[ch]' orig/linux-2.6.0-test11-ia64/drivers/acpi/numa.c linux-2.6.0-test11-ia64/drivers/acpi/numa.c
    --- orig/linux-2.6.0-test11-ia64/drivers/acpi/numa.c Thu Dec 11 04:22:40 2003
    +++ linux-2.6.0-test11-ia64/drivers/acpi/numa.c Thu Jan 8 02:49:24 2004
    @@ -38,7 +38,7 @@
    #define Dprintk(x...)
    #endif

    -extern int __init acpi_table_parse_madt_family (enum acpi_table_id id, unsigned long madt_size, int entry_id, acpi_madt_entry_handler handler);
    +extern int __init acpi_table_parse_madt_family (enum acpi_table_id id, unsigned long madt_size, int entry_id, acpi_madt_entry_handler handler, int max_entries);

    void __init
    acpi_table_print_srat_entry (
    @@ -156,10 +156,11 @@
    int __init
    acpi_table_parse_srat (
    enum acpi_srat_entry_id id,
    - acpi_madt_entry_handler handler)
    + acpi_madt_entry_handler handler,
    + int max_entries)
    {
    return acpi_table_parse_madt_family(ACPI_SRAT, sizeof(struct acpi_table_srat),
    - id, handler);
    + id, handler, max_entries);
    }


    @@ -173,9 +174,11 @@

    if (result > 0) {
    result = acpi_table_parse_srat(ACPI_SRAT_PROCESSOR_AFFINITY,
    - acpi_parse_processor_affinity);
    + acpi_parse_processor_affinity,
    + NR_CPUS);
    result = acpi_table_parse_srat(ACPI_SRAT_MEMORY_AFFINITY,
    - acpi_parse_memory_affinity);
    + acpi_parse_memory_affinity,
    + NR_MEMBLKS);
    } else {
    /* FIXME */
    printk("Warning: acpi_table_parse(ACPI_SRAT) returned %d!\n",result);
    diff -urN -X /usr/people/jes/exclude-linux --exclude=io --exclude=sn --exclude='qla1280.[ch]' orig/linux-2.6.0-test11-ia64/drivers/acpi/tables.c linux-2.6.0-test11-ia64/drivers/acpi/tables.c
    --- orig/linux-2.6.0-test11-ia64/drivers/acpi/tables.c Thu Dec 11 04:22:40 2003
    +++ linux-2.6.0-test11-ia64/drivers/acpi/tables.c Thu Jan 8 04:46:09 2004
    @@ -295,13 +295,14 @@
    enum acpi_table_id id,
    unsigned long madt_size,
    int entry_id,
    - acpi_madt_entry_handler handler)
    + acpi_madt_entry_handler handler,
    + int max_entries)
    {
    void *madt = NULL;
    - acpi_table_entry_header *entry = NULL;
    - unsigned long count = 0;
    - unsigned long madt_end = 0;
    - unsigned int i = 0;
    + acpi_table_entry_header *entry;
    + unsigned int count = 0;
    + unsigned long madt_end;
    + unsigned int i;

    if (!handler)
    return -EINVAL;
    @@ -335,13 +336,17 @@
    ((unsigned long) madt + madt_size);

    while (((unsigned long) entry) < madt_end) {
    - if (entry->type == entry_id) {
    - count++;
    + if (entry->type == entry_id && count++ < max_entries)
    handler(entry);
    - }
    +
    entry = (acpi_table_entry_header *)
    ((unsigned long) entry + entry->length);
    }
    + if (count > max_entries) {
    + printk(KERN_WARNING PREFIX "[%s:0x%02x] ignored %i entries of "
    + "%i found\n", acpi_table_signatures[id], entry_id,
    + count - max_entries, count);
    + }

    return count;
    }
    @@ -350,10 +355,11 @@
    int __init
    acpi_table_parse_madt (
    enum acpi_madt_entry_id id,
    - acpi_madt_entry_handler handler)
    + acpi_madt_entry_handler handler,
    + int max_entries)
    {
    return acpi_table_parse_madt_family(ACPI_APIC, sizeof(struct acpi_table_madt),
    - id, handler);
    + id, handler, max_entries);
    }


    @@ -578,4 +584,3 @@

    return 0;
    }
    -
    diff -urN -X /usr/people/jes/exclude-linux --exclude=io --exclude=sn --exclude='qla1280.[ch]' orig/linux-2.6.0-test11-ia64/include/linux/acpi.h linux-2.6.0-test11-ia64/include/linux/acpi.h
    --- orig/linux-2.6.0-test11-ia64/include/linux/acpi.h Wed Nov 26 12:42:43 2003
    +++ linux-2.6.0-test11-ia64/include/linux/acpi.h Thu Jan 8 03:08:06 2004
    @@ -355,8 +355,8 @@
    int acpi_table_init (void);
    int acpi_table_parse (enum acpi_table_id id, acpi_table_handler handler);
    int acpi_get_table_header_early (enum acpi_table_id id, struct acpi_table_header **header);
    -int acpi_table_parse_madt (enum acpi_madt_entry_id id, acpi_madt_entry_handler handler);
    -int acpi_table_parse_srat (enum acpi_srat_entry_id id, acpi_madt_entry_handler handler);
    +int acpi_table_parse_madt (enum acpi_madt_entry_id id, acpi_madt_entry_handler handler, int max_entries);
    +int acpi_table_parse_srat (enum acpi_srat_entry_id id, acpi_madt_entry_handler handler, int max_entries);
    void acpi_table_print (struct acpi_table_header *header, unsigned long phys_addr);
    void acpi_table_print_madt_entry (acpi_table_entry_header *madt);
    void acpi_table_print_srat_entry (acpi_table_entry_header *srat);
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2005-03-22 13:59    [W:0.033 / U:183.308 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site