Hi Kees,
On Wed, Aug 14, 2019 at 01:55:50PM -0700, Kees Cook wrote:
> When UEFI booting, if allocate_pages() fails (either via KASLR or
> regular boot), efi_low_alloc() is used for fall back. If it, too, fails,
> it reports "Failed to relocate kernel". Then handle_kernel_image()
> reports the failure to its caller, which unhelpfully reports exactly
> the same string again:
>
> EFI stub: ERROR: Failed to relocate kernel
> EFI stub: ERROR: Failed to relocate kernel
>
> While debugging linker errors in the UEFI code that created insane memory
> sizes that all the allocation attempts would fail at, this was a cause
> for confusion. Knowing each allocation had failed would have helped me
> isolate the issue sooner. To that end, this improves the error messages
> to detail which specific allocations have failed.
>
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
> drivers/firmware/efi/libstub/arm64-stub.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c
> b/drivers/firmware/efi/libstub/arm64-stub.c
> index 1550d244e996..24022f956e01 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -111,6 +111,8 @@ efi_status_t handle_kernel_image(efi_system_table_t
> *sys_table_arg,
> status = efi_random_alloc(sys_table_arg, *reserve_size,
> MIN_KIMG_ALIGN, reserve_addr,
> (u32)phys_seed);
> + if (status != EFI_SUCCESS)
> + pr_efi_err(sys_table_arg, "KASLR allocate_pages()
> failed\n");
>
> *image_addr = *reserve_addr + offset;
> } else {
> @@ -135,6 +137,8 @@ efi_status_t handle_kernel_image(efi_system_table_t
> *sys_table_arg,
> EFI_LOADER_DATA,
> *reserve_size / EFI_PAGE_SIZE,
> (efi_physical_addr_t *)reserve_addr);
> + if (status != EFI_SUCCESS)
> + pr_efi_err(sys_table_arg, "regular allocate_pages()
> failed\n");
> }
Not sure I see the need to distinsuish the 'KASLR' case from the 'regular'
case -- only one should run, right? That also didn't seem to be part of
the use-case in the commit, unless I'm missing something.
Maybe combine the prints as per the diff below?
Will
--->8
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c
b/drivers/firmware/efi/libstub/arm64-stub.c
index 1550d244e996..820c58cc149e 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -143,13 +143,15 @@ efi_status_t handle_kernel_image(efi_system_table_t
*sys_table_arg,
MIN_KIMG_ALIGN, reserve_addr);
if (status != EFI_SUCCESS) {
- pr_efi_err(sys_table_arg, "Failed to relocate
kernel\n");
+ pr_efi_err(sys_table_arg, "efi_low_alloc() failed\n");
*reserve_size = 0;
return status;
}
*image_addr = *reserve_addr + TEXT_OFFSET;
+ } else {
+ pr_efi_err(sys_table_arg, "allocate_pages() failed\n");
}
- memcpy((void *)*image_addr, old_image_addr, kernel_size);
+ memcpy((void *)*image_addr, old_image_addr, kernel_size);
return EFI_SUCCESS;
}
