diff mbox series

efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure

Message ID 20200926085850.16342-1-ardb@kernel.org
State Accepted
Commit d32de9130f6c79533508e2c7879f18997bfbe2a0
Headers show
Series efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure | expand

Commit Message

Ard Biesheuvel Sept. 26, 2020, 8:58 a.m. UTC
Currently, on arm64, we abort on any failure from efi_get_random_bytes()
other than EFI_NOT_FOUND when it comes to setting the physical seed for
KASLR, but ignore such failures when obtaining the seed for virtual
KASLR or for early seeding of the kernel's entropy pool via the config
table. This is inconsistent, and may lead to unexpected boot failures.

So let's permit any failure for the physical seed, and simply report
the error code if it does not equal EFI_NOT_FOUND.

Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Heinrich Schuchardt Sept. 27, 2020, 8:17 a.m. UTC | #1
On 9/26/20 10:58 AM, Ard Biesheuvel wrote:
> Currently, on arm64, we abort on any failure from efi_get_random_bytes()
> other than EFI_NOT_FOUND when it comes to setting the physical seed for
> KASLR, but ignore such failures when obtaining the seed for virtual
> KASLR or for early seeding of the kernel's entropy pool via the config
> table. This is inconsistent, and may lead to unexpected boot failures.
>
> So let's permit any failure for the physical seed, and simply report
> the error code if it does not equal EFI_NOT_FOUND.
>
> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Hello Ard,

thank you for providing the patch. Unfortunately this seems not to be
enough for booting with an EFI_RNG_PROTOCOL.GetRng() returning
EFI_UNSUPPORTED.

This is the output I received on v5.9-rc6, defconfig with your patch
applied. I can retry with a branch from
https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/ if you
indicate which one to use.

EFI stub: Booting Linux Kernel...
EFI stub: ERROR: efi_get_random_bytes() failed (0x8000000000000003),
KASLR will be disabled
EFI stub: Using DTB from configuration table

EFI stub: Exiting boot services and installing virtual address map...

EFI stub: ERROR: Unable to construct new device tree.
EFI stub: ERROR: Failed to update FDT and exit boot services

This matches the problem originally reported by Scott.

Best regards

Heinrich

> ---
>  drivers/firmware/efi/libstub/arm64-stub.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index e5bfac79e5ac..21692a986beb 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -62,10 +62,10 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>  			status = efi_get_random_bytes(sizeof(phys_seed),
>  						      (u8 *)&phys_seed);
>  			if (status == EFI_NOT_FOUND) {
> -				efi_info("EFI_RNG_PROTOCOL unavailable, no randomness supplied\n");
> +				efi_info("EFI_RNG_PROTOCOL unavailable, KASLR will be disabled\n");
>  			} else if (status != EFI_SUCCESS) {
> -				efi_err("efi_get_random_bytes() failed\n");
> -				return status;
> +				efi_err("efi_get_random_bytes() failed (0x%lx), KASLR will be disabled\n",
> +					status);
>  			}
>  		} else {
>  			efi_info("KASLR disabled on kernel command line\n");
>
Ard Biesheuvel Sept. 27, 2020, 9:13 a.m. UTC | #2
On Sun, 27 Sep 2020 at 10:18, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 9/26/20 10:58 AM, Ard Biesheuvel wrote:
> > Currently, on arm64, we abort on any failure from efi_get_random_bytes()
> > other than EFI_NOT_FOUND when it comes to setting the physical seed for
> > KASLR, but ignore such failures when obtaining the seed for virtual
> > KASLR or for early seeding of the kernel's entropy pool via the config
> > table. This is inconsistent, and may lead to unexpected boot failures.
> >
> > So let's permit any failure for the physical seed, and simply report
> > the error code if it does not equal EFI_NOT_FOUND.
> >
> > Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Hello Ard,
>
> thank you for providing the patch. Unfortunately this seems not to be
> enough for booting with an EFI_RNG_PROTOCOL.GetRng() returning
> EFI_UNSUPPORTED.
>
> This is the output I received on v5.9-rc6, defconfig with your patch
> applied. I can retry with a branch from
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/ if you
> indicate which one to use.
>
> EFI stub: Booting Linux Kernel...
> EFI stub: ERROR: efi_get_random_bytes() failed (0x8000000000000003),
> KASLR will be disabled
> EFI stub: Using DTB from configuration table
>
> EFI stub: Exiting boot services and installing virtual address map...
>
> EFI stub: ERROR: Unable to construct new device tree.
> EFI stub: ERROR: Failed to update FDT and exit boot services
>
> This matches the problem originally reported by Scott.
>

OK, so we need something like the below as well:

--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -145,8 +145,6 @@ static efi_status_t update_fdt(void *orig_fdt,
unsigned long orig_fdt_size,
                        status = fdt_setprop_var(fdt, node,
"kaslr-seed", fdt_val64);
                        if (status)
                                goto fdt_set_fail;
-               } else if (efi_status != EFI_NOT_FOUND) {
-                       return efi_status;
                }
        }


Could you please check whether that fixes the issue?
Heinrich Schuchardt Sept. 27, 2020, 2:08 p.m. UTC | #3
On 9/27/20 11:13 AM, Ard Biesheuvel wrote:
> On Sun, 27 Sep 2020 at 10:18, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 9/26/20 10:58 AM, Ard Biesheuvel wrote:
>>> Currently, on arm64, we abort on any failure from efi_get_random_bytes()
>>> other than EFI_NOT_FOUND when it comes to setting the physical seed for
>>> KASLR, but ignore such failures when obtaining the seed for virtual
>>> KASLR or for early seeding of the kernel's entropy pool via the config
>>> table. This is inconsistent, and may lead to unexpected boot failures.
>>>
>>> So let's permit any failure for the physical seed, and simply report
>>> the error code if it does not equal EFI_NOT_FOUND.
>>>
>>> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>
>> Hello Ard,
>>
>> thank you for providing the patch. Unfortunately this seems not to be
>> enough for booting with an EFI_RNG_PROTOCOL.GetRng() returning
>> EFI_UNSUPPORTED.
>>
>> This is the output I received on v5.9-rc6, defconfig with your patch
>> applied. I can retry with a branch from
>> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/ if you
>> indicate which one to use.
>>
>> EFI stub: Booting Linux Kernel...
>> EFI stub: ERROR: efi_get_random_bytes() failed (0x8000000000000003),
>> KASLR will be disabled
>> EFI stub: Using DTB from configuration table
>>
>> EFI stub: Exiting boot services and installing virtual address map...
>>
>> EFI stub: ERROR: Unable to construct new device tree.
>> EFI stub: ERROR: Failed to update FDT and exit boot services
>>
>> This matches the problem originally reported by Scott.
>>
>
> OK, so we need something like the below as well:
>
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -145,8 +145,6 @@ static efi_status_t update_fdt(void *orig_fdt,
> unsigned long orig_fdt_size,
>                         status = fdt_setprop_var(fdt, node,
> "kaslr-seed", fdt_val64);
>                         if (status)
>                                 goto fdt_set_fail;
> -               } else if (efi_status != EFI_NOT_FOUND) {
> -                       return efi_status;
>                 }
>         }
>
>
> Could you please check whether that fixes the issue?
>

The new change allows booting.

I could not observe the new message from the patch on my ARM64 system.
The only related messages I found are:

[  +0.000000] efi: EFI v2.80 by Das U-Boot
[  +0.000000] efi: RTPROP=0x7aef9040 SMBIOS=0x7aef5000 MEMRESERVE=0x566df040
[  +0.000000] random: get_random_bytes called from
start_kernel+0x314/0x4e8 with crng_init=0
[  +0.003506] KASLR disabled due to lack of seed

Best regards

Heinrich
Ard Biesheuvel Sept. 29, 2020, 1:51 p.m. UTC | #4
On Sun, 27 Sep 2020 at 16:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 9/27/20 11:13 AM, Ard Biesheuvel wrote:
> > On Sun, 27 Sep 2020 at 10:18, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> On 9/26/20 10:58 AM, Ard Biesheuvel wrote:
> >>> Currently, on arm64, we abort on any failure from efi_get_random_bytes()
> >>> other than EFI_NOT_FOUND when it comes to setting the physical seed for
> >>> KASLR, but ignore such failures when obtaining the seed for virtual
> >>> KASLR or for early seeding of the kernel's entropy pool via the config
> >>> table. This is inconsistent, and may lead to unexpected boot failures.
> >>>
> >>> So let's permit any failure for the physical seed, and simply report
> >>> the error code if it does not equal EFI_NOT_FOUND.
> >>>
> >>> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >>
> >> Hello Ard,
> >>
> >> thank you for providing the patch. Unfortunately this seems not to be
> >> enough for booting with an EFI_RNG_PROTOCOL.GetRng() returning
> >> EFI_UNSUPPORTED.
> >>
> >> This is the output I received on v5.9-rc6, defconfig with your patch
> >> applied. I can retry with a branch from
> >> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/ if you
> >> indicate which one to use.
> >>
> >> EFI stub: Booting Linux Kernel...
> >> EFI stub: ERROR: efi_get_random_bytes() failed (0x8000000000000003),
> >> KASLR will be disabled
> >> EFI stub: Using DTB from configuration table
> >>
> >> EFI stub: Exiting boot services and installing virtual address map...
> >>
> >> EFI stub: ERROR: Unable to construct new device tree.
> >> EFI stub: ERROR: Failed to update FDT and exit boot services
> >>
> >> This matches the problem originally reported by Scott.
> >>
> >
> > OK, so we need something like the below as well:
> >
> > --- a/drivers/firmware/efi/libstub/fdt.c
> > +++ b/drivers/firmware/efi/libstub/fdt.c
> > @@ -145,8 +145,6 @@ static efi_status_t update_fdt(void *orig_fdt,
> > unsigned long orig_fdt_size,
> >                         status = fdt_setprop_var(fdt, node,
> > "kaslr-seed", fdt_val64);
> >                         if (status)
> >                                 goto fdt_set_fail;
> > -               } else if (efi_status != EFI_NOT_FOUND) {
> > -                       return efi_status;
> >                 }
> >         }
> >
> >
> > Could you please check whether that fixes the issue?
> >
>
> The new change allows booting.
>
> I could not observe the new message from the patch on my ARM64 system.
> The only related messages I found are:
>
> [  +0.000000] efi: EFI v2.80 by Das U-Boot
> [  +0.000000] efi: RTPROP=0x7aef9040 SMBIOS=0x7aef5000 MEMRESERVE=0x566df040
> [  +0.000000] random: get_random_bytes called from
> start_kernel+0x314/0x4e8 with crng_init=0
> [  +0.003506] KASLR disabled due to lack of seed
>


Thanks. The EFI diagnostic messages are usually written directly to
the serial console - they are not captured by dmesg.
diff mbox series

Patch

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index e5bfac79e5ac..21692a986beb 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -62,10 +62,10 @@  efi_status_t handle_kernel_image(unsigned long *image_addr,
 			status = efi_get_random_bytes(sizeof(phys_seed),
 						      (u8 *)&phys_seed);
 			if (status == EFI_NOT_FOUND) {
-				efi_info("EFI_RNG_PROTOCOL unavailable, no randomness supplied\n");
+				efi_info("EFI_RNG_PROTOCOL unavailable, KASLR will be disabled\n");
 			} else if (status != EFI_SUCCESS) {
-				efi_err("efi_get_random_bytes() failed\n");
-				return status;
+				efi_err("efi_get_random_bytes() failed (0x%lx), KASLR will be disabled\n",
+					status);
 			}
 		} else {
 			efi_info("KASLR disabled on kernel command line\n");