when "fwts gtdt" is executed, fwts throws below error even when the GTDT table looks fine in the "fwts acpidump".
gtdt: GTDT Generic Timer Description Table test ------------------------------- Test 1 of 1: GTDT Generic Timer Description Table test. FAILED [HIGH] GTDTShortBlock: Test 1, GTDT block is too short -------------------------------
This patch removes the error checking, since (ptr + 20) is always less than end_ptr even for the valid GTDT block.
Signed-off-by: Vikas C Sajjan vikas.cha.sajjan@hpe.com Signed-off-by: Sunil V L sunil.vl@hpe.com --- src/acpi/gtdt/gtdt.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c index 421f17f..05ccea1 100644 --- a/src/acpi/gtdt/gtdt.c +++ b/src/acpi/gtdt/gtdt.c @@ -75,13 +75,6 @@ static int gtdt_test1(fwts_framework *fw) case 0x00: /* GT Block Structure */ block = (fwts_acpi_table_gtdt_block *)ptr; - if (ptr + 20 < end_ptr) { - passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "GTDTShortBlock", - "GTDT block is too short"); - goto done; - } if (block->length < 20) { passed = false; fwts_failed(fw, LOG_LEVEL_HIGH,
Hi there,
can you send me an ACPI dump of your GTDT so I can double check the data verses my code and the specification?
Thanks
Colin
On 23/03/16 08:45, Vikas C Sajjan wrote:
when "fwts gtdt" is executed, fwts throws below error even when the GTDT table looks fine in the "fwts acpidump".
gtdt: GTDT Generic Timer Description Table test
Test 1 of 1: GTDT Generic Timer Description Table test. FAILED [HIGH] GTDTShortBlock: Test 1, GTDT block is too short -------------------------------
This patch removes the error checking, since (ptr + 20) is always less than end_ptr even for the valid GTDT block.
Signed-off-by: Vikas C Sajjan vikas.cha.sajjan@hpe.com Signed-off-by: Sunil V L sunil.vl@hpe.com
src/acpi/gtdt/gtdt.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c index 421f17f..05ccea1 100644 --- a/src/acpi/gtdt/gtdt.c +++ b/src/acpi/gtdt/gtdt.c @@ -75,13 +75,6 @@ static int gtdt_test1(fwts_framework *fw) case 0x00: /* GT Block Structure */ block = (fwts_acpi_table_gtdt_block *)ptr;
if (ptr + 20 < end_ptr) {passed = false;fwts_failed(fw, LOG_LEVEL_HIGH,"GTDTShortBlock","GTDT block is too short");goto done;} if (block->length < 20) { passed = false; fwts_failed(fw, LOG_LEVEL_HIGH,
Hi Colin,
On Wed, Mar 30, 2016 at 3:08 PM, Colin Ian King colin.king@canonical.com wrote:
Hi there,
can you send me an ACPI dump of your GTDT so I can double check the data verses my code and the specification?
Below is the acpidump of gtdt.
GTDT @ bff02823 (224 bytes) (loaded from file) ---- [000h 0000 4] Signature : "GTDT" [Generic Timer Description Table] [004h 0004 4] Table Length : 000000E0 [008h 0008 1] Revision : 02 [009h 0009 1] Checksum : 85 [00Ah 0010 6] Oem ID : "HPE " [010h 0016 8] Oem Table ID : "ProLiant" [018h 0024 4] Oem Revision : 00000001 [01Ch 0028 4] Asl Compiler ID : "INTL" [020h 0032 4] Asl Compiler Revision : 20140724 [024h 0036 8] Counter Block Address : 0000000000000000 [02Ch 0044 4] Reserved : 00000000 [030h 0048 4] Secure EL1 Interrupt : 00000010 [034h 0052 4] EL1 Flags (decoded below) : 00000000
Trigger Mode : 0 Polarity : 0 Always On : 0 [038h 0056 4] Non-Secure EL1 Interrupt : 0000001D [03Ch 0060 4] NEL1 Flags (decoded below) : 00000000 Trigger Mode : 0 Polarity : 0 Always On : 0 [040h 0064 4] Virtual Timer Interrupt : 0000001E [044h 0068 4] VT Flags (decoded below) : 00000000 Trigger Mode : 0 Polarity : 0 Always On : 0 [048h 0072 4] Non-Secure EL2 Interrupt : 0000001F [04Ch 0076 4] NEL2 Flags (decoded below) : 00000000 Trigger Mode : 0 Polarity : 0 Always On : 0 [050h 0080 8] Counter Read Block Address : 0000000000000000 [058h 0088 4] Platform Timer Count : 00000002 [05Ch 0092 4] Platform Timer Offset : 00000060 [060h 0096 1] Subtable Type : 00 [Generic Timer Block] [061h 0097 2] Length : 0064 [063h 0099 1] Reserved : 00 [063h 0099 1] Reserved : 00 [064h 0100 8] Block Address : 0000000000000000 [06Ch 0108 4] Timer Count : 00000002 [070h 0112 4] Timer Offset : 00000014 [074h 0116 1] Frame Number : 00 [075h 0117 3] Reserved : 000000 [078h 0120 8] Base Address : 0000000000000000 [080h 0128 8] EL0 Base Address : 0000000000000000 [088h 0136 4] Timer Interrupt : 00000000 [08Ch 0140 4] Timer Flags (decoded below) : 00000001 Trigger Mode : 1 Polarity : 0 [090h 0144 4] Virtual Timer Interrupt : 00000000 [094h 0148 4] Virtual Timer Flags (decoded below) : 00000001 Trigger Mode : 1 Polarity : 0 [098h 0152 4] Common Flags (decoded below) : 00000000 Secure : 0 Always On : 0 [09Ch 0156 1] Frame Number : 01 [09Dh 0157 3] Reserved : 000000 [0A0h 0160 8] Base Address : 0000000000000000 [0A8h 0168 8] EL0 Base Address : 0000000000000000 [0B0h 0176 4] Timer Interrupt : 00000000 [0B4h 0180 4] Timer Flags (decoded below) : 00000001 Trigger Mode : 1 Polarity : 0 [0B8h 0184 4] Virtual Timer Interrupt : 00000000 [0BCh 0188 4] Virtual Timer Flags (decoded below) : 00000001 Trigger Mode : 1 Polarity : 0 [0C0h 0192 4] Common Flags (decoded below) : 00000000 Secure : 0 Always On : 0 [0C4h 0196 1] Subtable Type : 01 [Generic Watchdog Timer] [0C5h 0197 2] Length : 001C [0C7h 0199 1] Reserved : 00 [0C8h 0200 8] Refresh Frame Address : 0000000000000000 [0D0h 0208 8] Control Frame Address : 0000000000000000 [0D8h 0216 4] Timer Interrupt : 00000000 [0DCh 0220 4] Timer Flags (decoded below) : 00000001 Trigger Mode : 1 Polarity : 0 Security : 0
Thanks
Colin
On 23/03/16 08:45, Vikas C Sajjan wrote:
when "fwts gtdt" is executed, fwts throws below error even when the GTDT table looks fine in the "fwts acpidump".
gtdt: GTDT Generic Timer Description Table test
Test 1 of 1: GTDT Generic Timer Description Table test. FAILED [HIGH] GTDTShortBlock: Test 1, GTDT block is too short -------------------------------
This patch removes the error checking, since (ptr + 20) is always less than end_ptr even for the valid GTDT block.
Signed-off-by: Vikas C Sajjan vikas.cha.sajjan@hpe.com Signed-off-by: Sunil V L sunil.vl@hpe.com
src/acpi/gtdt/gtdt.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c index 421f17f..05ccea1 100644 --- a/src/acpi/gtdt/gtdt.c +++ b/src/acpi/gtdt/gtdt.c @@ -75,13 +75,6 @@ static int gtdt_test1(fwts_framework *fw) case 0x00: /* GT Block Structure */ block = (fwts_acpi_table_gtdt_block *)ptr;
if (ptr + 20 < end_ptr) {passed = false;fwts_failed(fw, LOG_LEVEL_HIGH,"GTDTShortBlock","GTDT block is too short");goto done;} if (block->length < 20) { passed = false; fwts_failed(fw, LOG_LEVEL_HIGH,
Linaro-acpi mailing list Linaro-acpi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-acpi
Hi there,
I can see the bug now, I think we need to combine your patch plus a fix to the original code and then we get the best of both..
On 23/03/16 08:45, Vikas C Sajjan wrote:
when "fwts gtdt" is executed, fwts throws below error even when the GTDT table looks fine in the "fwts acpidump".
gtdt: GTDT Generic Timer Description Table test
Test 1 of 1: GTDT Generic Timer Description Table test. FAILED [HIGH] GTDTShortBlock: Test 1, GTDT block is too short -------------------------------
This patch removes the error checking, since (ptr + 20) is always less than end_ptr even for the valid GTDT block.
Signed-off-by: Vikas C Sajjan vikas.cha.sajjan@hpe.com Signed-off-by: Sunil V L sunil.vl@hpe.com
src/acpi/gtdt/gtdt.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c index 421f17f..05ccea1 100644 --- a/src/acpi/gtdt/gtdt.c +++ b/src/acpi/gtdt/gtdt.c @@ -75,13 +75,6 @@ static int gtdt_test1(fwts_framework *fw) case 0x00: /* GT Block Structure */ block = (fwts_acpi_table_gtdt_block *)ptr;
if (ptr + 20 < end_ptr) {
My code was wrong here, it should be checking to see if we don't run off the end of the buffer with the following check:
if (ptr + 20 > end_ptr)
I'll post a fix later on with your header sanity check plus a fix to my broken comparison.
passed = false;fwts_failed(fw, LOG_LEVEL_HIGH,"GTDTShortBlock","GTDT block is too short");goto done;} if (block->length < 20) { passed = false; fwts_failed(fw, LOG_LEVEL_HIGH,
On 30/03/16 11:23, Colin Ian King wrote:
Hi there,
I can see the bug now, I think we need to combine your patch plus a fix to the original code and then we get the best of both..
Actually, it was a single change and I've tested this against a template table generated by iasl, so I am happy it fixes the issue. Fix sent to mailing list: https://lists.ubuntu.com/archives/fwts-devel/2016-March/007705.html
thanks for reporting this Vikas.
Colin
On 23/03/16 08:45, Vikas C Sajjan wrote:
when "fwts gtdt" is executed, fwts throws below error even when the GTDT table looks fine in the "fwts acpidump".
gtdt: GTDT Generic Timer Description Table test
Test 1 of 1: GTDT Generic Timer Description Table test. FAILED [HIGH] GTDTShortBlock: Test 1, GTDT block is too short -------------------------------
This patch removes the error checking, since (ptr + 20) is always less than end_ptr even for the valid GTDT block.
Signed-off-by: Vikas C Sajjan vikas.cha.sajjan@hpe.com Signed-off-by: Sunil V L sunil.vl@hpe.com
src/acpi/gtdt/gtdt.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c index 421f17f..05ccea1 100644 --- a/src/acpi/gtdt/gtdt.c +++ b/src/acpi/gtdt/gtdt.c @@ -75,13 +75,6 @@ static int gtdt_test1(fwts_framework *fw) case 0x00: /* GT Block Structure */ block = (fwts_acpi_table_gtdt_block *)ptr;
if (ptr + 20 < end_ptr) {My code was wrong here, it should be checking to see if we don't run off the end of the buffer with the following check:
if (ptr + 20 > end_ptr)I'll post a fix later on with your header sanity check plus a fix to my broken comparison.
passed = false;fwts_failed(fw, LOG_LEVEL_HIGH,"GTDTShortBlock","GTDT block is too short");goto done;} if (block->length < 20) { passed = false; fwts_failed(fw, LOG_LEVEL_HIGH,
Hi Colin,
Similar bug exists while checking for SBSA Generic watchdog structure. Currently we are not hitting this issue because i)the size is exactly 28 bytes ii)the watchdog structure doesn't come before GT block structure. But ACPI spec doesn't mandate any such order. Hence, it would be better the issue is fixed here also.
@@ -179,7 +172,7 @@ static int gtdt_test1(fwts_framework *fw) case 0x01: /* SBSA Generic Watchdog Timer Structure */ watchdog = (fwts_acpi_table_gtdt_watchdog *)ptr; - if (ptr + 28 < end_ptr) { + if (ptr + 28 > end_ptr) { passed = false; fwts_failed(fw, LOG_LEVEL_HIGH, "GTDTShortWatchDogTimer",
Thanks Sunil On Wednesday 30 March 2016 03:53 PM, Colin Ian King wrote:
Hi there,
I can see the bug now, I think we need to combine your patch plus a fix to the original code and then we get the best of both..
On 23/03/16 08:45, Vikas C Sajjan wrote:
when "fwts gtdt" is executed, fwts throws below error even when the GTDT table looks fine in the "fwts acpidump".
gtdt: GTDT Generic Timer Description Table test
Test 1 of 1: GTDT Generic Timer Description Table test. FAILED [HIGH] GTDTShortBlock: Test 1, GTDT block is too short -------------------------------
This patch removes the error checking, since (ptr + 20) is always less than end_ptr even for the valid GTDT block.
Signed-off-by: Vikas C Sajjan vikas.cha.sajjan@hpe.com Signed-off-by: Sunil V L sunil.vl@hpe.com
src/acpi/gtdt/gtdt.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c index 421f17f..05ccea1 100644 --- a/src/acpi/gtdt/gtdt.c +++ b/src/acpi/gtdt/gtdt.c @@ -75,13 +75,6 @@ static int gtdt_test1(fwts_framework *fw) case 0x00: /* GT Block Structure */ block = (fwts_acpi_table_gtdt_block *)ptr;
if (ptr + 20 < end_ptr) {My code was wrong here, it should be checking to see if we don't run off the end of the buffer with the following check:
if (ptr + 20 > end_ptr)I'll post a fix later on with your header sanity check plus a fix to my broken comparison.
passed = false;fwts_failed(fw, LOG_LEVEL_HIGH,"GTDTShortBlock","GTDT block is too short");goto done;} if (block->length < 20) { passed = false; fwts_failed(fw, LOG_LEVEL_HIGH,.
-
Colin Ian King -
Sunil V L -
Vikas C Sajjan -
Vikas Sajjan