Re: [PATCH] efi/libstub: fix efi_random_alloc() to allocate memory at alloc_min or higher address

[KONDO KAZUMA(近藤 和真)]

Hello Ard Biesheuvel,

On Fri, 22 Mar 2024 at 22:17, Ard Biesheuvel wrote:
> On Fri, 22 Mar 2024 at 12:01, KONDO KAZUMA(近藤　和真) <kazuma-kondo@xxxxxxx> wrote:
>>
>> Following warning is sometimes observed while booting my servers:
>>   [    3.594838] DMA: preallocated 4096 KiB GFP_KERNEL pool for atomic allocations
>>   [    3.602918] swapper/0: page allocation failure: order:10, mode:0xcc1(GFP_KERNEL|GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0-1
>>   ...
>>   [    3.851862] DMA: preallocated 1024 KiB GFP_KERNEL|GFP_DMA pool for atomic allocation
>>
>> If 'nokaslr' boot option is set, the warning always happens.
>>
>> On x86, ZONE_DMA is small zone at the first 16MB of physical address
>> space. When this problem happens, most of that space seems to be used
>> by decompressed kernel. Thereby, there is not enough space at DMA_ZONE
>> to meet the request of DMA pool allocation.
>>
>> The commit 2f77465b05b1 ("x86/efistub: Avoid placing the kernel below LOAD_PHYSICAL_ADDR")
>> tried to fix this problem by introducing lower bound of allocation.
>>
>> But the fix is not complete.
>>
>> efi_random_alloc() allocates pages by following steps.
>> 1. Count total available slots ('total_slots')
>> 2. Select a slot ('target_slot') to allocate randomly
>> 3. Calculate a starting address ('target') to be included target_slot
>> 4. Allocate pages, which starting address is 'target'
>>
>> In step 1, 'alloc_min' is used to offset the starting address of
>> memory chunk. But in step 3 'alloc_min' is not considered at all.
>> As the result, 'target' can be miscalculated and become lower
>> than 'alloc_min'.
>>
>> When KASLR is disabled, 'target_slot' is always 0 and
>> the problem happens everytime if the EFI memory map of the system
>> meets the condition.
>>
>> Fix this problem by calculating 'target' considering 'alloc_min'.
>>
>> Cc: linux-efi@xxxxxxxxxxxxxxx
>> Cc: Tom Englund <tomenglund26@xxxxxxxxx>
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Fixes: 2f77465b05b1 ("x86/efistub: Avoid placing the kernel below LOAD_PHYSICAL_ADDR")
>> Signed-off-by: Kazuma Kondo <kazuma-kondo@xxxxxxx>
> 
> Hello Kazuma Kondo,
> 
> Thanks for your patch. I will take it as a fix.
> 
> You sent the same patch twice, right? Is there any difference between the two?

Sorry for the confusion.

I accidentally send same patch twice.

So, please ignore the other patch
which was sent by me on Fri, 22 Mar 2024 at 10:47 UTC.

Thanks,
Kazuma Kondo

>> ---
>>  drivers/firmware/efi/libstub/randomalloc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/firmware/efi/libstub/randomalloc.c b/drivers/firmware/efi/libstub/randomalloc.c
>> index 4e96a855fdf4..7e1852859550 100644
>> --- a/drivers/firmware/efi/libstub/randomalloc.c
>> +++ b/drivers/firmware/efi/libstub/randomalloc.c
>> @@ -120,7 +120,7 @@ efi_status_t efi_random_alloc(unsigned long size,
>>                         continue;
>>                 }
>>
>> -               target = round_up(md->phys_addr, align) + target_slot * align;
>> +               target = round_up(max(md->phys_addr, alloc_min), align) + target_slot * align;
>>                 pages = size / EFI_PAGE_SIZE;
>>
>>                 status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
>> --
>> 2.39.3