[PATCH 2/2] Clean up RSDT Table Creation (v2)
This patch is also based on the patch by Vincent Minet. It corrects the size calculation of the RSDT, and checks for overflow of MAX_RSDT_ENTRIES, assuming that the external table entry count is contained within MAX_RSDT_ENTRIES. I moved the for() loop to the end of the code that adds table_offset_entry entries so I could add the check for overflow - || (nb_rsdt_entries > MAX_RSDT_ENTRIES) This is not ideal. An ideal fix would require a rewrite of the rsdt build code, which I can do later and submit to qemu. Signed-off-by: Beth Kon diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c index cdae363..7db91d8 100755 --- a/kvm/bios/rombios32.c +++ b/kvm/bios/rombios32.c @@ -1602,7 +1602,7 @@ void acpi_bios_init(void) uint32_t hpet_addr; #endif uint32_t base_addr, rsdt_addr, fadt_addr, addr, facs_addr, dsdt_addr, ssdt_addr; -uint32_t acpi_tables_size, madt_addr, madt_size, rsdt_size, madt_end; +uint32_t acpi_tables_size, madt_addr, madt_size, rsdt_size, madt_end, rsdt_end; uint32_t srat_addr,srat_size; uint16_t i, external_tables; int nb_numa_nodes; @@ -1628,7 +1628,7 @@ void acpi_bios_init(void) addr = base_addr = ram_size - ACPI_DATA_SIZE; rsdt_addr = addr; rsdt = (void *)(addr); -rsdt_size = sizeof(*rsdt) + external_tables * 4; +rsdt_size = sizeof(*rsdt); addr += rsdt_size; fadt_addr = addr; @@ -1872,16 +1872,6 @@ void acpi_bios_init(void) "HPET", sizeof(*hpet), 1); #endif -acpi_additional_tables(); /* resets cfg to required entry */ -for(i = 0; i < external_tables; i++) { -uint16_t len; -if(acpi_load_table(i, addr, &len) < 0) -BX_PANIC("Failed to load ACPI table from QEMU\n"); -rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr); -addr += len; -if(addr >= ram_size) -BX_PANIC("ACPI table overflow\n"); -} #endif /* RSDT */ @@ -1894,9 +1884,19 @@ void acpi_bios_init(void) // rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(hpet_addr); if (nb_numa_nodes > 0) rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(srat_addr); +acpi_additional_tables(); /* resets cfg to required entry */ +for(i = 0; i < external_tables; i++) { +uint16_t len; +if(acpi_load_table(i, addr, &len) < 0) +BX_PANIC("Failed to load ACPI table from QEMU\n"); +rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr); +addr += len; +if ((addr >= ram_size) || (nb_rsdt_entries > MAX_RSDT_ENTRIES)) +BX_PANIC("ACPI table overflow\n"); +} #endif -rsdt_size -= MAX_RSDT_ENTRIES * 4; -rsdt_size += nb_rsdt_entries * 4; +rsdt_end = (uint32_t)(&rsdt->table_offset_entry[nb_rsdt_entries]); +rsdt_size = rsdt_end - rsdt_addr; acpi_build_table_header((struct acpi_table_header *)rsdt, "RSDT", rsdt_size, 1); -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Clean up RSDT Table Creation
Beth Kon wrote: This patch is also based on the patch by Vincent Minet. It corrects the size calculation of the RSDT, and checks for overflow of MAX_RSDT_ENTRIES, assuming that the external table entry count is contained within MAX_RSDT_ENTRIES. Signed-off-by: Beth Kon diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c index 7f62e4f..ac8f9c5 100755 --- a/kvm/bios/rombios32.c +++ b/kvm/bios/rombios32.c @@ -1626,7 +1626,7 @@ void acpi_bios_init(void) addr = base_addr = ram_size - ACPI_DATA_SIZE; rsdt_addr = addr; rsdt = (void *)(addr); -rsdt_size = sizeof(*rsdt) + external_tables * 4; +rsdt_size = sizeof(*rsdt); addr += rsdt_size; fadt_addr = addr; @@ -1873,16 +1873,6 @@ void acpi_bios_init(void) "HPET", sizeof(*hpet), 1); #endif -acpi_additional_tables(); /* resets cfg to required entry */ -for(i = 0; i < external_tables; i++) { -uint16_t len; -if(acpi_load_table(i, addr, &len) < 0) -BX_PANIC("Failed to load ACPI table from QEMU\n"); -rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr); -addr += len; -if(addr >= ram_size) -BX_PANIC("ACPI table overflow\n"); -} #endif /* RSDT */ @@ -1895,6 +1885,19 @@ void acpi_bios_init(void) // rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(hpet_addr); if (nb_numa_nodes > 0) rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(srat_addr); +acpi_additional_tables(); /* resets cfg to required entry */ +/* external_tables load must occur last to + * properly check for MAX_RSDT_ENTRIES overflow. + */ +for(i = 0; i < external_tables; i++) { +uint16_t len; +if(acpi_load_table(i, addr, &len) < 0) +BX_PANIC("Failed to load ACPI table from QEMU\n"); +rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr); +addr += len; +if((addr >= ram_size) || (nb_rsdt_entries > MAX_RSDT_ENTRIES)) +BX_PANIC("ACPI table overflow\n"); +} #endif rsdt_size -= MAX_RSDT_ENTRIES * 4; rsdt_size += nb_rsdt_entries * 4; Same comment - instead of calculating the size incrementally, set rsdt_end = &rsdt->table_offset_entry[nb_rsdt_entries] and calculate the size from that. btw, why did you move the code? -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Clean up RSDT Table Creation
This patch is also based on the patch by Vincent Minet. It corrects the size calculation of the RSDT, and checks for overflow of MAX_RSDT_ENTRIES, assuming that the external table entry count is contained within MAX_RSDT_ENTRIES. Signed-off-by: Beth Kon diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c index 7f62e4f..ac8f9c5 100755 --- a/kvm/bios/rombios32.c +++ b/kvm/bios/rombios32.c @@ -1626,7 +1626,7 @@ void acpi_bios_init(void) addr = base_addr = ram_size - ACPI_DATA_SIZE; rsdt_addr = addr; rsdt = (void *)(addr); -rsdt_size = sizeof(*rsdt) + external_tables * 4; +rsdt_size = sizeof(*rsdt); addr += rsdt_size; fadt_addr = addr; @@ -1873,16 +1873,6 @@ void acpi_bios_init(void) "HPET", sizeof(*hpet), 1); #endif -acpi_additional_tables(); /* resets cfg to required entry */ -for(i = 0; i < external_tables; i++) { -uint16_t len; -if(acpi_load_table(i, addr, &len) < 0) -BX_PANIC("Failed to load ACPI table from QEMU\n"); -rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr); -addr += len; -if(addr >= ram_size) -BX_PANIC("ACPI table overflow\n"); -} #endif /* RSDT */ @@ -1895,6 +1885,19 @@ void acpi_bios_init(void) // rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(hpet_addr); if (nb_numa_nodes > 0) rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(srat_addr); +acpi_additional_tables(); /* resets cfg to required entry */ +/* external_tables load must occur last to + * properly check for MAX_RSDT_ENTRIES overflow. + */ +for(i = 0; i < external_tables; i++) { +uint16_t len; +if(acpi_load_table(i, addr, &len) < 0) +BX_PANIC("Failed to load ACPI table from QEMU\n"); +rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr); +addr += len; +if((addr >= ram_size) || (nb_rsdt_entries > MAX_RSDT_ENTRIES)) +BX_PANIC("ACPI table overflow\n"); +} #endif rsdt_size -= MAX_RSDT_ENTRIES * 4; rsdt_size += nb_rsdt_entries * 4; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html