| Message ID | 20190515100813.16965-1-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [V2] fwts_acpica: don't add in RSDP or null table entries toXSDT or RSDT (LP: #1829167) | expand |
On 2019-05-15 3:08 a.m., Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > BugLink: https://bugs.launchpad.net/fwts/+bug/1829167 > > Adding in the RSDP table to the XSDT and RSDT is illegal. It also > breaks ACPICA as this tries to checksum the RSDP and as it's a non-standard > table the length field is offset differently from most tables causing > the checksum to segfault. Also don't in null tables, this does not make > sense. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > > V2: add in missing bug number > > --- > src/acpica/fwts_acpica.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c > index 75f8fe3d..65dad3c5 100644 > --- a/src/acpica/fwts_acpica.c > +++ b/src/acpica/fwts_acpica.c > @@ -1125,6 +1125,7 @@ int fwts_acpica_init(fwts_framework *fw) > return FWTS_ERROR; > if (table != NULL) { > uint64_t *entries; > + int j; > > fwts_acpica_XSDT = fwts_low_calloc(1, table->length); > if (fwts_acpica_XSDT == NULL) { > @@ -1135,18 +1136,19 @@ int fwts_acpica_init(fwts_framework *fw) > > n = (table->length - sizeof(ACPI_TABLE_HEADER)) / sizeof(uint64_t); > entries = (uint64_t*)(table->data + sizeof(ACPI_TABLE_HEADER)); > - for (i = 0; i < n; i++) { > + for (i = 0, j = 0; i < n; i++) { > fwts_acpi_table_info *tbl; > + > if (fwts_acpi_find_table_by_addr(fw, entries[i], &tbl) != FWTS_OK) > return FWTS_ERROR; > if (tbl) { > + if (strncmp(tbl->name, "RSDP", 4) == 0) > + continue; > if (strncmp(tbl->name, "FACP", 4) == 0) { > - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); > + fwts_acpica_XSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); > } else { > - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(tbl->data); > + fwts_acpica_XSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(tbl->data); > } > - } else { > - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(NULL); > } > } > fwts_acpica_XSDT->Header.Checksum = 0; > @@ -1160,6 +1162,7 @@ int fwts_acpica_init(fwts_framework *fw) > return FWTS_ERROR; > if (table) { > uint32_t *entries; > + int j; > > fwts_acpica_RSDT = fwts_low_calloc(1, table->length); > if (fwts_acpica_RSDT == NULL) { > @@ -1170,19 +1173,18 @@ int fwts_acpica_init(fwts_framework *fw) > > n = (table->length - sizeof(ACPI_TABLE_HEADER)) / sizeof(uint32_t); > entries = (uint32_t*)(table->data + sizeof(ACPI_TABLE_HEADER)); > - for (i = 0; i < n; i++) { > + for (i = 0, j = 0; i < n; i++) { > fwts_acpi_table_info *tbl; > if (fwts_acpi_find_table_by_addr(fw, entries[i], &tbl) != FWTS_OK) > return FWTS_ERROR; > if (tbl) { > + if (strncmp(tbl->name, "RSDP", 4) == 0) > + continue; > if (strncmp(tbl->name, "FACP", 4) == 0) { > - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); > - } > - else { > - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(tbl->data); > + fwts_acpica_RSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); > + } else { > + fwts_acpica_RSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(tbl->data); > } > - } else { > - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(NULL); > } > } > fwts_acpica_RSDT->Header.Checksum = 0; > Acked-by: Alex Hung <alex.hung@canonical.com>
On 5/15/19 6:08 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > BugLink: https://bugs.launchpad.net/fwts/+bug/1829167 > > Adding in the RSDP table to the XSDT and RSDT is illegal. It also > breaks ACPICA as this tries to checksum the RSDP and as it's a non-standard > table the length field is offset differently from most tables causing > the checksum to segfault. Also don't in null tables, this does not make > sense. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > > V2: add in missing bug number > > --- > src/acpica/fwts_acpica.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c > index 75f8fe3d..65dad3c5 100644 > --- a/src/acpica/fwts_acpica.c > +++ b/src/acpica/fwts_acpica.c > @@ -1125,6 +1125,7 @@ int fwts_acpica_init(fwts_framework *fw) > return FWTS_ERROR; > if (table != NULL) { > uint64_t *entries; > + int j; > > fwts_acpica_XSDT = fwts_low_calloc(1, table->length); > if (fwts_acpica_XSDT == NULL) { > @@ -1135,18 +1136,19 @@ int fwts_acpica_init(fwts_framework *fw) > > n = (table->length - sizeof(ACPI_TABLE_HEADER)) / sizeof(uint64_t); > entries = (uint64_t*)(table->data + sizeof(ACPI_TABLE_HEADER)); > - for (i = 0; i < n; i++) { > + for (i = 0, j = 0; i < n; i++) { > fwts_acpi_table_info *tbl; > + > if (fwts_acpi_find_table_by_addr(fw, entries[i], &tbl) != FWTS_OK) > return FWTS_ERROR; > if (tbl) { > + if (strncmp(tbl->name, "RSDP", 4) == 0) > + continue; > if (strncmp(tbl->name, "FACP", 4) == 0) { > - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); > + fwts_acpica_XSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); > } else { > - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(tbl->data); > + fwts_acpica_XSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(tbl->data); > } > - } else { > - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(NULL); > } > } > fwts_acpica_XSDT->Header.Checksum = 0; > @@ -1160,6 +1162,7 @@ int fwts_acpica_init(fwts_framework *fw) > return FWTS_ERROR; > if (table) { > uint32_t *entries; > + int j; > > fwts_acpica_RSDT = fwts_low_calloc(1, table->length); > if (fwts_acpica_RSDT == NULL) { > @@ -1170,19 +1173,18 @@ int fwts_acpica_init(fwts_framework *fw) > > n = (table->length - sizeof(ACPI_TABLE_HEADER)) / sizeof(uint32_t); > entries = (uint32_t*)(table->data + sizeof(ACPI_TABLE_HEADER)); > - for (i = 0; i < n; i++) { > + for (i = 0, j = 0; i < n; i++) { > fwts_acpi_table_info *tbl; > if (fwts_acpi_find_table_by_addr(fw, entries[i], &tbl) != FWTS_OK) > return FWTS_ERROR; > if (tbl) { > + if (strncmp(tbl->name, "RSDP", 4) == 0) > + continue; > if (strncmp(tbl->name, "FACP", 4) == 0) { > - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); > - } > - else { > - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(tbl->data); > + fwts_acpica_RSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); > + } else { > + fwts_acpica_RSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(tbl->data); > } > - } else { > - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(NULL); > } > } > fwts_acpica_RSDT->Header.Checksum = 0; Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c index 75f8fe3d..65dad3c5 100644 --- a/src/acpica/fwts_acpica.c +++ b/src/acpica/fwts_acpica.c @@ -1125,6 +1125,7 @@ int fwts_acpica_init(fwts_framework *fw) return FWTS_ERROR; if (table != NULL) { uint64_t *entries; + int j; fwts_acpica_XSDT = fwts_low_calloc(1, table->length); if (fwts_acpica_XSDT == NULL) { @@ -1135,18 +1136,19 @@ int fwts_acpica_init(fwts_framework *fw) n = (table->length - sizeof(ACPI_TABLE_HEADER)) / sizeof(uint64_t); entries = (uint64_t*)(table->data + sizeof(ACPI_TABLE_HEADER)); - for (i = 0; i < n; i++) { + for (i = 0, j = 0; i < n; i++) { fwts_acpi_table_info *tbl; + if (fwts_acpi_find_table_by_addr(fw, entries[i], &tbl) != FWTS_OK) return FWTS_ERROR; if (tbl) { + if (strncmp(tbl->name, "RSDP", 4) == 0) + continue; if (strncmp(tbl->name, "FACP", 4) == 0) { - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); + fwts_acpica_XSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); } else { - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(tbl->data); + fwts_acpica_XSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(tbl->data); } - } else { - fwts_acpica_XSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(NULL); } } fwts_acpica_XSDT->Header.Checksum = 0; @@ -1160,6 +1162,7 @@ int fwts_acpica_init(fwts_framework *fw) return FWTS_ERROR; if (table) { uint32_t *entries; + int j; fwts_acpica_RSDT = fwts_low_calloc(1, table->length); if (fwts_acpica_RSDT == NULL) { @@ -1170,19 +1173,18 @@ int fwts_acpica_init(fwts_framework *fw) n = (table->length - sizeof(ACPI_TABLE_HEADER)) / sizeof(uint32_t); entries = (uint32_t*)(table->data + sizeof(ACPI_TABLE_HEADER)); - for (i = 0; i < n; i++) { + for (i = 0, j = 0; i < n; i++) { fwts_acpi_table_info *tbl; if (fwts_acpi_find_table_by_addr(fw, entries[i], &tbl) != FWTS_OK) return FWTS_ERROR; if (tbl) { + if (strncmp(tbl->name, "RSDP", 4) == 0) + continue; if (strncmp(tbl->name, "FACP", 4) == 0) { - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); - } - else { - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(tbl->data); + fwts_acpica_RSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(fwts_acpica_FADT); + } else { + fwts_acpica_RSDT->TableOffsetEntry[j++] = ACPI_PTR_TO_PHYSADDR(tbl->data); } - } else { - fwts_acpica_RSDT->TableOffsetEntry[i] = ACPI_PTR_TO_PHYSADDR(NULL); } } fwts_acpica_RSDT->Header.Checksum = 0;