[U-Boot] [PATCH 1/1] efi_loader: HII protocols: fix new_package_list()

[Heinrich Schuchardt]

On 3/2/19 5:32 PM, Alexander Graf wrote:
> 
> On 28.02.19 23:20, Heinrich Schuchardt wrote:
>> In new_package_list() we call new_packagelist() to create a new package
>> list. Next we try to add the packages which fails for form packages. Due
>> to this error we call free_packagelist(). Now in free_packagelist()
>> list_del() is called for an uninitialized field hii->link. This leads to
>> changing random memory addresses.
>>
>> To solve the problem move the initialization of hii->link to
>> new_packagelist().
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>> @Takahiro:
>> Please, review the patch.
>> ---
>>  lib/efi_loader/efi_hii.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
>> index d63d2d84184..0ed4b196333 100644
>> --- a/lib/efi_loader/efi_hii.c
>> +++ b/lib/efi_loader/efi_hii.c
>> @@ -343,6 +343,7 @@ static struct efi_hii_packagelist *new_packagelist(void)
>>  	struct efi_hii_packagelist *hii;
>>  
>>  	hii = malloc(sizeof(*hii));
>> +	list_add_tail(&hii->link, &efi_package_lists);
> 
> 
> Why in new_packagelist() and not in new_package_list()?

The important thing is that we do not call list_del() before
list_add_tail().

new_packagelist() is only called by new_package_list(). So you could
move any line of new_packagelist() to new_package_list().

But I see new_packagelist as the counterpart of free_packagelist(). So
it seems natural to put list_add_tail() into new_packagelist() to match
the list_del() in free_packagelist().

Anything specific you dislike about it?

Best regards

Heinrich

> 
> 
> Alex
> 
> 
>>  	hii->max_string_id = 0;
>>  	INIT_LIST_HEAD(&hii->string_tables);
>>  	INIT_LIST_HEAD(&hii->guid_list);
>> @@ -465,7 +466,6 @@ new_package_list(const struct efi_hii_database_protocol *this,
>>  	}
>>  
>>  	hii->driver_handle = driver_handle;
>> -	list_add_tail(&hii->link, &efi_package_lists);
>>  	*handle = hii;
>>  
>>  	return EFI_EXIT(EFI_SUCCESS);
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>