[PATCH v3 1/3] efi: random: reduce seed size to 32 bytes

[PATCH v3 1/3] efi: random: reduce seed size to 32 bytes

[Ard Biesheuvel]

We no longer need at least 64 bytes of random seed to permit the early
crng init to complete. The RNG is now based on Blake2s, so reduce the
EFI seed size to the Blake2s hash size, which is sufficient for our
purposes.

While at it, drop the READ_ONCE(), which was supposed to prevent size
from being evaluated after seed was unmapped. However, this cannot
actually happen, so READ_ONCE() is unnecessary here.

Cc: <stable@vger.kernel.org> # v4.14+
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/firmware/efi/efi.c | 2 +-
 include/linux/efi.h        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 9624735f1575..a949509de62f 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -609,7 +609,7 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
 
 		seed = early_memremap(efi_rng_seed, sizeof(*seed));
 		if (seed != NULL) {
-			size = READ_ONCE(seed->size);
+			size = min(seed->size, EFI_RANDOM_SEED_SIZE);
 			early_memunmap(seed, sizeof(*seed));
 		} else {
 			pr_err("Could not map UEFI random seed!\n");
diff --git a/include/linux/efi.h b/include/linux/efi.h
index da3974bf05d3..cf96f8d5f15f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1225,7 +1225,7 @@ efi_status_t efi_random_get_seed(void);
 	arch_efi_call_virt_teardown();					\
 })
 
-#define EFI_RANDOM_SEED_SIZE		64U
+#define EFI_RANDOM_SEED_SIZE		32U // BLAKE2S_HASH_SIZE
 
 struct linux_efi_random_seed {
 	u32	size;
-- 
2.35.1

[PATCH v3 2/3] efi: random: Use 'ACPI reclaim' memory for random seed

[Ard Biesheuvel]

EFI runtime services data is guaranteed to be preserved by the OS,
making it a suitable candidate for the EFI random seed table, which may
be passed to kexec kernels as well (after refreshing the seed), and so
we need to ensure that the memory is preserved without support from the
OS itself.

However, runtime services data is intended for allocations that are
relevant to the implementations of the runtime services themselves, and
so they are unmapped from the kernel linear map, and mapped into the EFI
page tables that are active while runtime service invocations are in
progress. None of this is needed for the RNG seed.

So let's switch to EFI 'ACPI reclaim' memory: in spite of the name,
there is nothing exclusively ACPI about it, it is simply a type of
allocation that carries firmware provided data which may or may not be
relevant to the OS, and it is left up to the OS to decide whether to
reclaim it after having consumed its contents.

Given that in Linux, we never reclaim these allocations, it is a good
choice for the EFI RNG seed, as the allocation is guaranteed to survive
kexec reboots.

One additional reason for changing this now is to align it with the
upcoming recommendation for EFI bootloader provided RNG seeds, which
must not use EFI runtime services code/data allocations.

Cc: <stable@vger.kernel.org> # v4.14+
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/random.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index 24aa37535372..183dc5cdb8ed 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -75,7 +75,7 @@ efi_status_t efi_random_get_seed(void)
 	if (status != EFI_SUCCESS)
 		return status;
 
-	status = efi_bs_call(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
+	status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
 			     sizeof(*seed) + EFI_RANDOM_SEED_SIZE,
 			     (void **)&seed);
 	if (status != EFI_SUCCESS)
-- 
2.35.1

Re: [PATCH v3 2/3] efi: random: Use 'ACPI reclaim' memory for random seed

[Ilias Apalodimas]

Hi Ard,

On Thu, 20 Oct 2022 at 11:40, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> EFI runtime services data is guaranteed to be preserved by the OS,
> making it a suitable candidate for the EFI random seed table, which may
> be passed to kexec kernels as well (after refreshing the seed), and so
> we need to ensure that the memory is preserved without support from the
> OS itself.
>
> However, runtime services data is intended for allocations that are
> relevant to the implementations of the runtime services themselves, and
> so they are unmapped from the kernel linear map, and mapped into the EFI
> page tables that are active while runtime service invocations are in
> progress. None of this is needed for the RNG seed.
>
> So let's switch to EFI 'ACPI reclaim' memory: in spite of the name,
> there is nothing exclusively ACPI about it, it is simply a type of
> allocation that carries firmware provided data which may or may not be
> relevant to the OS, and it is left up to the OS to decide whether to
> reclaim it after having consumed its contents.
>
> Given that in Linux, we never reclaim these allocations, it is a good
> choice for the EFI RNG seed, as the allocation is guaranteed to survive
> kexec reboots.

Can we add this as a comment right above the efi_bs_call()

>
> One additional reason for changing this now is to align it with the
> upcoming recommendation for EFI bootloader provided RNG seeds, which
> must not use EFI runtime services code/data allocations.
>
> Cc: <stable@vger.kernel.org> # v4.14+
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/random.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
> index 24aa37535372..183dc5cdb8ed 100644
> --- a/drivers/firmware/efi/libstub/random.c
> +++ b/drivers/firmware/efi/libstub/random.c
> @@ -75,7 +75,7 @@ efi_status_t efi_random_get_seed(void)
>         if (status != EFI_SUCCESS)
>                 return status;
>
> -       status = efi_bs_call(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
> +       status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
>                              sizeof(*seed) + EFI_RANDOM_SEED_SIZE,
>                              (void **)&seed);
>         if (status != EFI_SUCCESS)
> --
> 2.35.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH v3 1/3] efi: random: reduce seed size to 32 bytes

[Ilias Apalodimas]

On Thu, Oct 20, 2022 at 10:39:08AM +0200, Ard Biesheuvel wrote:
> We no longer need at least 64 bytes of random seed to permit the early
> crng init to complete. The RNG is now based on Blake2s, so reduce the
> EFI seed size to the Blake2s hash size, which is sufficient for our
> purposes.
> 
> While at it, drop the READ_ONCE(), which was supposed to prevent size
> from being evaluated after seed was unmapped. However, this cannot
> actually happen, so READ_ONCE() is unnecessary here.
> 
> Cc: <stable@vger.kernel.org> # v4.14+
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  drivers/firmware/efi/efi.c | 2 +-
>  include/linux/efi.h        | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 9624735f1575..a949509de62f 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -609,7 +609,7 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
>  
>  		seed = early_memremap(efi_rng_seed, sizeof(*seed));
>  		if (seed != NULL) {
> -			size = READ_ONCE(seed->size);
> +			size = min(seed->size, EFI_RANDOM_SEED_SIZE);
>  			early_memunmap(seed, sizeof(*seed));
>  		} else {
>  			pr_err("Could not map UEFI random seed!\n");
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index da3974bf05d3..cf96f8d5f15f 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1225,7 +1225,7 @@ efi_status_t efi_random_get_seed(void);
>  	arch_efi_call_virt_teardown();					\
>  })
>  
> -#define EFI_RANDOM_SEED_SIZE		64U
> +#define EFI_RANDOM_SEED_SIZE		32U // BLAKE2S_HASH_SIZE
>  
>  struct linux_efi_random_seed {
>  	u32	size;
> -- 
> 2.35.1
> 

Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>