APPLIED: [PATCH][V2] acpi: acpidump: check for short length headers (LP: #1375300)
Keng-Yu Lin
keng-yu.lin at canonical.com
Fri Oct 3 07:16:29 UTC 2014
On Wed, Oct 1, 2014 at 5:08 PM, Colin King <colin.king at canonical.com> wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> There are a bunch of places in fwts acpidump that takes the
> given table header sizes and sub-table sizes as given,
> however, firmware can get this wrong.
>
> So bail out early on bad looking header sizes rather than
> believe the data and fall off the end of the table.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
> src/acpi/acpidump/acpidump.c | 51 ++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/src/acpi/acpidump/acpidump.c b/src/acpi/acpidump/acpidump.c
> index e3fe4b1..5c1fcc1 100644
> --- a/src/acpi/acpidump/acpidump.c
> +++ b/src/acpi/acpidump/acpidump.c
> @@ -1055,6 +1055,13 @@ static void acpidump_slit(fwts_framework *fw, const fwts_acpi_table_info *table)
> uint64_t n = table->length - sizeof(fwts_acpi_table_slit);
> const uint8_t *entry;
>
> + if (table->length < sizeof(fwts_acpi_table_slit)) {
> + fwts_log_info_verbatum(fw, "SLIT header length too short, expected %zu "
> + "bytes, got %" PRIu64 " bytes instead. Aborting SLIT table dump.",
> + sizeof(fwts_acpi_table_slit), table->length);
> + return;
> + }
> +
> fwts_log_info_verbatum(fw, "# Sys Localities: 0x%" PRIx64 "(%" PRIu64 ")",
> slit->num_of_system_localities, slit->num_of_system_localities);
> if (n < slit->num_of_system_localities * slit->num_of_system_localities) {
> @@ -1250,6 +1257,13 @@ static void acpidump_asf(fwts_framework *fw, const fwts_acpi_table_info *table)
> uint8_t i;
> uint8_t *asf_ptr = ptr;
>
> + /* Minimal header check */
> + if (hdr->length < sizeof(fwts_acpi_table_asf_header)) {
> + fwts_log_info_verbatum(fw, "ASF header length too short, expected %zu "
> + "bytes, got %" PRIu16 " bytes instead. Aborting ASF table dump.",
> + sizeof(fwts_acpi_table_asf_header), hdr->length);
> + break;
> + }
> fwts_log_nl(fw);
> __acpi_dump_table_fields(fw, asf_ptr, asf_header_fields, asf_ptr - data);
>
> @@ -1333,18 +1347,25 @@ static void acpidump_dmar_device_scope(
>
> /* Parse through multiple device scope entries */
> while (length > 0) {
> - unsigned int i;
> + ssize_t i, len;
>
> fwts_acpi_table_dmar_device_scope *device_scope_entry =
> (fwts_acpi_table_dmar_device_scope *)device_scope;
> __acpi_dump_table_fields(fw, device_scope, dmar_device_scope_fields, device_scope - data);
> + len = device_scope_entry->length - sizeof(fwts_acpi_table_dmar_device_scope);
> + /* Something not good about the data */
> + if (len <= 0) {
> + fwts_log_info_verbatum(fw, "DMAR device scope entry length "
> + "too short. Aborting device scope dump.");
> + break;
> + }
> /*
> * The device scope has a variable length path,
> * so just dump this raw data out for now.
> */
> - for (i = 0; i < device_scope_entry->length - sizeof(fwts_acpi_table_dmar_device_scope); i++) {
> + for (i = 0; i < len; i++) {
> uint8_t val8 = device_scope_entry->path[i];
> - fwts_log_info_verbatum(fw, "%s 0x%2.2x [%d]", acpi_dump_field_info("Path", 1,
> + fwts_log_info_verbatum(fw, "%s 0x%2.2x [%zd]", acpi_dump_field_info("Path", 1,
> (device_scope - data) + sizeof(fwts_acpi_table_dmar_device_scope) + i),
> val8, i);
> }
> @@ -1407,6 +1428,14 @@ static void acpidump_dmar(fwts_framework *fw, const fwts_acpi_table_info *table)
> (fwts_acpi_table_dmar_header *)ptr;
>
> fwts_log_nl(fw);
> + /* Something not good with the data */
> + if (header->length < sizeof(fwts_acpi_table_dmar_header)) {
> + fwts_log_info_verbatum(fw, "DMAR header length "
> + "too short, expected %zu bytes, got %" PRIu16
> + " bytes instead. Aborting DMAR dump.",
> + sizeof(fwts_acpi_table_dmar_header), header->length);
> + break;
> + }
>
> switch (header->type) {
> case 0:
> @@ -1644,7 +1673,7 @@ static void acpidump_fpdt(fwts_framework *fw, const fwts_acpi_table_info *table)
> /* fpdt not long enough, bail out early */
> if (fpdt->length < 16) {
> size_t offset = ptr - data;
> - fwts_log_info_verbatum(fw, "Cannot decode FPDT header, size %"
> + fwts_log_info_verbatum(fw, "Cannot decode FPDT header, size %"
> PRIu8 " less than 16 bytes. Data:", fpdt->length);
> acpi_dump_raw_data(fw, ptr, table->length - offset, offset);
> break;
> @@ -1748,6 +1777,13 @@ static void acpidump_pcct(fwts_framework *fw, const fwts_acpi_table_info *table)
> fwts_acpi_table_pcct_subspace_header *header =
> (fwts_acpi_table_pcct_subspace_header *)ptr;
>
> + if (header->length < sizeof(fwts_acpi_table_pcct_subspace_header)) {
> + fwts_log_info_verbatum(fw, "PCCT subspace header length too short, expected %zu "
> + "bytes, got %" PRIu8 " bytes instead. Aborting PCCT table dump.",
> + sizeof(fwts_acpi_table_pcct_subspace_header), header->length);
> + break;
> + }
> +
> /* Currently just type 0 is supported */
> switch (header->type) {
> case 0:
> @@ -1887,6 +1923,13 @@ static void acpidump_dbg2(fwts_framework *fw, const fwts_acpi_table_info *table)
>
> __acpi_dump_table_fields(fw, table->data + offset, dbg2_info_fields, offset);
>
> + if (dbg2_info->length < sizeof(fwts_acpi_table_dbg2_info)) {
> + fwts_log_info_verbatum(fw, "DBG2 info header length too short, expected %zu "
> + "bytes, got %" PRIu16 " bytes instead. Aborting PCCT table dump.",
> + sizeof(fwts_acpi_table_dbg2_info), dbg2_info->length);
> + break;
> + }
> +
> if (dbg2_info->number_of_regs) {
> /* Dump out the register GAS and sizes */
> for (j = 0; j < dbg2_info->number_of_regs; j++) {
> --
> 2.1.0
>
>
> --
> fwts-devel mailing list
> fwts-devel at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
More information about the fwts-devel
mailing list