[PATCH 2/4] efi_loader: check alignment in efi_add_memory_map()

[Michael Walle]

Am 2020-05-14 20:35, schrieb Heinrich Schuchardt:
> On 5/14/20 2:38 PM, Michael Walle wrote:
>> The first argument has to be aligned with EFI_PAGE_SIZE. This 
>> alignment
>> is already checked for external callers but it is not checked for
>> internal callers. Unfortunately, most of the time the return value is
>> not checked, so scream loud and clear.
> 
> Why do you mention the return value here?

most callers just ignore the return value. so if not aligned this will
silently fail.

>> 
>> Signed-off-by: Michael Walle <michael at walle.cc>
>> ---
>>  lib/efi_loader/efi_memory.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index fd79178da9..b56e19cb30 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -248,6 +248,9 @@ efi_status_t efi_add_memory_map(uint64_t start, 
>> uint64_t pages, int memory_type,
>>  	EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__,
>>  		  start, pages, memory_type, overlap_only_ram ? "yes" : "no");
>> 
>> +	if (start & EFI_PAGE_MASK)
>> +		panic("%s: start not aligned\n", __func__);
>> +
> 
> Did you find any internal caller that has a problem?
See next patch.

> We do not want to increase code size.
Mh, even within the efi_loader? Well I could do a

if (start & EFI_PAGE_MASK) {
     debug("%s: start not aligned\n", __func__);
     return EFI_INVALID_PARAMETER;
}

but as I said, nobody checks the return value.


-- 
-michael