[V2] fwts_acpica: don't add in RSDP or null table entries toXSDT or RSDT (LP: #1829167)
diff mbox series

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)
Related show

Commit Message

Colin Ian King May 15, 2019, 10:08 a.m. UTC
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(-)

Comments

Alex Hung May 15, 2019, 5:03 p.m. UTC | #1
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>
ivanhu May 16, 2019, 3:15 a.m. UTC | #2
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>

Patch
diff mbox series

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;