linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure
@ 2020-09-26  8:58 Ard Biesheuvel
  2020-09-27  8:17 ` Heinrich Schuchardt
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-09-26  8:58 UTC (permalink / raw)
  To: linux-efi; +Cc: xypron.glpk, Ard Biesheuvel

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

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");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure
  2020-09-26  8:58 [PATCH] efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure Ard Biesheuvel
@ 2020-09-27  8:17 ` Heinrich Schuchardt
  2020-09-27  9:13   ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2020-09-27  8:17 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-efi; +Cc: Scott K Logan

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");
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure
  2020-09-27  8:17 ` Heinrich Schuchardt
@ 2020-09-27  9:13   ` Ard Biesheuvel
  2020-09-27 14:08     ` Heinrich Schuchardt
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-09-27  9:13 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: linux-efi, Scott K Logan

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?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure
  2020-09-27  9:13   ` Ard Biesheuvel
@ 2020-09-27 14:08     ` Heinrich Schuchardt
  2020-09-29 13:51       ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2020-09-27 14:08 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Scott K Logan

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure
  2020-09-27 14:08     ` Heinrich Schuchardt
@ 2020-09-29 13:51       ` Ard Biesheuvel
  0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2020-09-29 13:51 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: linux-efi, Scott K Logan

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-29 13:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-26  8:58 [PATCH] efi/arm64: libstub: Deal gracefully with EFI_RNG_PROTOCOL failure Ard Biesheuvel
2020-09-27  8:17 ` Heinrich Schuchardt
2020-09-27  9:13   ` Ard Biesheuvel
2020-09-27 14:08     ` Heinrich Schuchardt
2020-09-29 13:51       ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).