diff mbox series

[v3] efi_loader: Get rid of kaslr-seed if EFI_RNG_PROTOCOL is installed

Message ID 20220103120738.47835-1-ilias.apalodimas@linaro.org
State Accepted
Commit a2f1482fc0e6c5dbdbafecd360d168f9c12fc529
Headers show
Series [v3] efi_loader: Get rid of kaslr-seed if EFI_RNG_PROTOCOL is installed | expand

Commit Message

Ilias Apalodimas Jan. 3, 2022, 12:07 p.m. UTC 
U-Boot, in some occasions, injects a 'kaslr-seed' property on the /chosen
node.  That would be problematic in case we want to measure the DTB we
install in the config table, since it would change across reboots.

The Linux kernel EFI-stub completely ignores it and only relies on
EFI_RNG_PROTOCOL for it's own randomness needs (i.e the randomization
of the physical placement of the kernel).  In fact it (blindly) overwrites
the existing seed if the protocol is installed.  However it still uses it
for randomizing it's virtual placement.
So let's get rid of it in the presence of the RNG protocol.

It's worth noting that TPMs also provide an RNG.  So if we tweak our
EFI_RNG_PROTOCOL slightly and install the protocol when a TPM device
is present the 'kaslr-seed' property will always be removed, allowing
us to reliably measure our DTB.

Acked-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
changes since v2:
- Mark proposed a better commit message description
changes since v1:
- Only removing the property if EFI_RNG_PROTOCOL is installed, since some
  OS'es rely on kaslr-seed
- Only display an error message if the kaslr-seed entry was found but not
  removed
 cmd/bootefi.c                 |  2 ++
 include/efi_loader.h          |  2 ++
 lib/efi_loader/efi_dt_fixup.c | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

Comments

Mark Kettenis Jan. 3, 2022, 12:28 p.m. UTC | #1 
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Date: Mon,  3 Jan 2022 14:07:37 +0200
> 
> U-Boot, in some occasions, injects a 'kaslr-seed' property on the /chosen
> node.  That would be problematic in case we want to measure the DTB we
> install in the config table, since it would change across reboots.
> 
> The Linux kernel EFI-stub completely ignores it and only relies on
> EFI_RNG_PROTOCOL for it's own randomness needs (i.e the randomization
> of the physical placement of the kernel).  In fact it (blindly) overwrites
> the existing seed if the protocol is installed.  However it still uses it
> for randomizing it's virtual placement.
> So let's get rid of it in the presence of the RNG protocol.
> 
> It's worth noting that TPMs also provide an RNG.  So if we tweak our
> EFI_RNG_PROTOCOL slightly and install the protocol when a TPM device
> is present the 'kaslr-seed' property will always be removed, allowing
> us to reliably measure our DTB.
> 
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> changes since v2:
> - Mark proposed a better commit message description
> changes since v1:
> - Only removing the property if EFI_RNG_PROTOCOL is installed, since some
>   OS'es rely on kaslr-seed
> - Only display an error message if the kaslr-seed entry was found but not
>   removed
>  cmd/bootefi.c                 |  2 ++
>  include/efi_loader.h          |  2 ++
>  lib/efi_loader/efi_dt_fixup.c | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+)

Reviewed-by: Mark Kettenis <kettenis@openbsd.org>

> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index d77d3b6e943d..57f13ce701ec 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -310,6 +310,8 @@ efi_status_t efi_install_fdt(void *fdt)
>  	/* Create memory reservations as indicated by the device tree */
>  	efi_carve_out_dt_rsv(fdt);
>  
> +	efi_try_purge_kaslr_seed(fdt);
> +
>  	/* Install device tree as UEFI table */
>  	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
>  	if (ret != EFI_SUCCESS) {
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 9dd6c2033634..1fe003db69e0 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -519,6 +519,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition,
>  					void **address);
>  /* Carve out DT reserved memory ranges */
>  void efi_carve_out_dt_rsv(void *fdt);
> +/* Purge unused kaslr-seed */
> +void efi_try_purge_kaslr_seed(void *fdt);
>  /* Called by bootefi to make console interface available */
>  efi_status_t efi_console_register(void);
>  /* Called by bootefi to make all disk storage accessible as EFI objects */
> diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c
> index b6fe5d2e5a34..d3923e5dba1b 100644
> --- a/lib/efi_loader/efi_dt_fixup.c
> +++ b/lib/efi_loader/efi_dt_fixup.c
> @@ -8,6 +8,7 @@
>  #include <common.h>
>  #include <efi_dt_fixup.h>
>  #include <efi_loader.h>
> +#include <efi_rng.h>
>  #include <fdtdec.h>
>  #include <mapmem.h>
>  
> @@ -40,6 +41,38 @@ static void efi_reserve_memory(u64 addr, u64 size, bool nomap)
>  			addr, size);
>  }
>  
> +/**
> + * efi_try_purge_kaslr_seed() - Remove unused kaslr-seed
> + *
> + * Kernel's EFI STUB only relies on EFI_RNG_PROTOCOL for randomization
> + * and completely ignores the kaslr-seed for its own randomness needs
> + * (i.e the randomization of the physical placement of the kernel).
> + * Weed it out from the DTB we hand over, which would mess up our DTB
> + * TPM measurements as well.
> + *
> + * @fdt: Pointer to device tree
> + */
> +void efi_try_purge_kaslr_seed(void *fdt)
> +{
> +	const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID;
> +	struct efi_handler *handler;
> +	efi_status_t ret;
> +	int nodeoff = 0;
> +	int err = 0;
> +
> +	ret = efi_search_protocol(efi_root, &efi_guid_rng_protocol, &handler);
> +	if (ret != EFI_SUCCESS)
> +		return;
> +
> +	nodeoff = fdt_path_offset(fdt, "/chosen");
> +	if (nodeoff < 0)
> +		return;
> +
> +	err = fdt_delprop(fdt, nodeoff, "kaslr-seed");
> +	if (err < 0 && err != -FDT_ERR_NOTFOUND)
> +		log_err("Error deleting kaslr-seed\n");
> +}
> +
>  /**
>   * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
>   *
> -- 
> 2.30.2
> 
>
diff mbox series

Patch

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index d77d3b6e943d..57f13ce701ec 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -310,6 +310,8 @@  efi_status_t efi_install_fdt(void *fdt)
 	/* Create memory reservations as indicated by the device tree */
 	efi_carve_out_dt_rsv(fdt);
 
+	efi_try_purge_kaslr_seed(fdt);
+
 	/* Install device tree as UEFI table */
 	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
 	if (ret != EFI_SUCCESS) {
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9dd6c2033634..1fe003db69e0 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -519,6 +519,8 @@  efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition,
 					void **address);
 /* Carve out DT reserved memory ranges */
 void efi_carve_out_dt_rsv(void *fdt);
+/* Purge unused kaslr-seed */
+void efi_try_purge_kaslr_seed(void *fdt);
 /* Called by bootefi to make console interface available */
 efi_status_t efi_console_register(void);
 /* Called by bootefi to make all disk storage accessible as EFI objects */
diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c
index b6fe5d2e5a34..d3923e5dba1b 100644
--- a/lib/efi_loader/efi_dt_fixup.c
+++ b/lib/efi_loader/efi_dt_fixup.c
@@ -8,6 +8,7 @@ 
 #include <common.h>
 #include <efi_dt_fixup.h>
 #include <efi_loader.h>
+#include <efi_rng.h>
 #include <fdtdec.h>
 #include <mapmem.h>
 
@@ -40,6 +41,38 @@  static void efi_reserve_memory(u64 addr, u64 size, bool nomap)
 			addr, size);
 }
 
+/**
+ * efi_try_purge_kaslr_seed() - Remove unused kaslr-seed
+ *
+ * Kernel's EFI STUB only relies on EFI_RNG_PROTOCOL for randomization
+ * and completely ignores the kaslr-seed for its own randomness needs
+ * (i.e the randomization of the physical placement of the kernel).
+ * Weed it out from the DTB we hand over, which would mess up our DTB
+ * TPM measurements as well.
+ *
+ * @fdt: Pointer to device tree
+ */
+void efi_try_purge_kaslr_seed(void *fdt)
+{
+	const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID;
+	struct efi_handler *handler;
+	efi_status_t ret;
+	int nodeoff = 0;
+	int err = 0;
+
+	ret = efi_search_protocol(efi_root, &efi_guid_rng_protocol, &handler);
+	if (ret != EFI_SUCCESS)
+		return;
+
+	nodeoff = fdt_path_offset(fdt, "/chosen");
+	if (nodeoff < 0)
+		return;
+
+	err = fdt_delprop(fdt, nodeoff, "kaslr-seed");
+	if (err < 0 && err != -FDT_ERR_NOTFOUND)
+		log_err("Error deleting kaslr-seed\n");
+}
+
 /**
  * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
  *