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

[Ard Biesheuvel]

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?


> ---
>  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