ACK: [PATCH] uefi: uefirtvariable: fix the fail GUID checking for uniqueness of variable (LP: #1311822)

[IvanHu]

On 04/25/2014 04:27 PM, Colin Ian King wrote:
> On 25/04/14 08:44, Ivan Hu wrote:
>> While using a simple hash bucket algorithm to check the uniqueness of each
>> variable and error if we encounter any duplicates, it used the same memory
>> address for GUID checking in each buckets. It makes the GUID would be identical,
>> so the result will be unexpected.
>
> After I figured out what the code was doing I realised what the final
> sentence should probably be:
>
> "Unfortunately, this makes the GUIDs identical, which causes incorrect
> GUID matches."
>
> Apart from that, it's good to me.

Thanks Colin, my poor English.:)
I'll amend the sentence when I push the patch to upstream.

>
>
>>
>> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
>> ---
>>   src/uefi/uefirtvariable/uefirtvariable.c |   16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
>> index 4b32693..5d31ad8 100644
>> --- a/src/uefi/uefirtvariable/uefirtvariable.c
>> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
>> @@ -520,6 +520,7 @@ static void bucket_destroy(void)
>>   			struct efi_var_item *chain = item->next;
>>
>>   			free(item->name);
>> +			free(item->guid);
>>   			free(item);
>>   			item = chain;
>>   		}
>> @@ -574,14 +575,24 @@ static int getnextvariable_test3(fwts_framework *fw)
>>   			goto err;
>>   		}
>>
>> -		item->guid = &vendorguid;
>> -		item->next = NULL;
>> +		item->guid = malloc(sizeof(EFI_GUID));
>> +		if (!item->guid) {
>> +			fwts_failed(fw, LOG_LEVEL_HIGH,
>> +				"UEFIRuntimeGetNextVariableName",
>> +				"Failed to allocate memory for test.");
>> +			free(item);
>> +			goto err;
>> +		}
>>
>> +		memcpy(item->guid, &vendorguid, sizeof(EFI_GUID));
>> +
>> +		item->next = NULL;
>>   		item->name = malloc(variablenamesize);
>>   		if (!item->name) {
>>   			fwts_failed(fw, LOG_LEVEL_HIGH,
>>   				"UEFIRuntimeGetNextVariableName",
>>   				"Failed to allocate memory for test.");
>> +			free(item->guid);
>>   			free(item);
>>   			goto err;
>>   		}
>> @@ -596,6 +607,7 @@ static int getnextvariable_test3(fwts_framework *fw)
>>   				"UEFIRuntimeGetNextVariableName",
>>   				"Duplicate variable name found.");
>>   			free(item->name);
>> +			free(item->guid);
>>   			free(item);
>>   			goto err;
>>   		}
>>
>
>
> Acked-by: Colin Ian King <colin.king at canonical.com>
>