ACK: [PATCH][V2] fwts_acpica: don't add in RSDP or null table entries to XSDT or RSDT (LP: #1829167)

[Alex Hung]

On 2019-05-15 3:08 a.m., Colin King wrote:
> From: Colin Ian King <colin.king at 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 at 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 at canonical.com>