[PATCH 1/2] acpi: acpidump: don't assume FPDT tables are correct about their record sizes (LP: #1375258)

[Alex Hung]

On 14-09-29 11:00 PM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Section 5.2.23.4 and 5.2.23.5 Performance Record Formats of the
> ACPI specification states that the length field is 16 for these
> two record types. Currently fwts believes this data from the
> firmware, however, some firmware has been known to set this to
> zero causing fwts to loop on the dumping of multiple records.
>
> fwts should instead not trust this field and assume it is 16
> bytes; anything smaller should be dumped out as a hex dump,
> otherwise it can be dumped correctly.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/acpi/acpidump/acpidump.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/acpi/acpidump/acpidump.c b/src/acpi/acpidump/acpidump.c
> index 7b45932..b42363c 100644
> --- a/src/acpi/acpidump/acpidump.c
> +++ b/src/acpi/acpidump/acpidump.c
> @@ -1641,6 +1641,12 @@ static void acpidump_fpdt(fwts_framework *fw, const fwts_acpi_table_info *table)
>   	while (ptr < data + table->length) {
>   		fwts_acpi_table_fpdt_header *fpdt = (fwts_acpi_table_fpdt_header *)ptr;
>   
> +		if (fpdt->length != 16) {
> +			size_t offset = ptr - data;
> +			acpi_dump_raw_data(fw, ptr, table->length - offset, offset);
> +			break;
> +		}
> +
>   		fwts_log_nl(fw);
>   
>   		switch (fpdt->type) {
> @@ -1680,7 +1686,7 @@ static void acpidump_fpdt(fwts_framework *fw, const fwts_acpi_table_info *table)
>   			acpi_dump_raw_data(fw, ptr + fpdt_hdr_len, fpdt->length - fpdt_hdr_len, ptr + fpdt_hdr_len - data);
>   			break;
>   		}
> -		ptr += fpdt->length;
> +		ptr += 16;
Since ACPI requires fpdt->length=16, will it be better to output an 
error message if it is not?

For example,

if (fpdt->length == 0) // invalid length {
     fwts_failed(fw, "invalid fpdt length = %x ...)
     ptr += 16;
} else
     ptr += fpdt->length;

If we really want it to be 16, it can also be

if (fpdt->length != 16) // invalid length
     fwts_failed(fw, "invalid fpdt length = %x ...)

ptr += 16;

>   	}
>   }
>