[PATCH] acpi: gtdt: Fix the wrong checking of GTDT block

[Sunil V L]

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 at hpe.com>
>> Signed-off-by: Sunil V L <sunil.vl at 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,
>>
>
> .
>