From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: xypron.glpk@gmx.de, ilias.apalodimas@linaro.org, ardb@kernel.org,
agraf@csgraf.de, u-boot@lists.denx.de
Subject: Re: [PATCH v3] efi_loader: Get rid of kaslr-seed if EFI_RNG_PROTOCOL is installed
Date: Mon, 3 Jan 2022 13:28:54 +0100 (CET) [thread overview]
Message-ID: <d3cb8336d41b4c01@bloch.sibelius.xs4all.nl> (raw)
In-Reply-To: <20220103120738.47835-1-ilias.apalodimas@linaro.org> (message from Ilias Apalodimas on Mon, 3 Jan 2022 14:07:37 +0200)
> 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
>
>
prev parent reply other threads:[~2022-01-03 12:29 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-03 12:07 [PATCH v3] efi_loader: Get rid of kaslr-seed if EFI_RNG_PROTOCOL is installed Ilias Apalodimas
2022-01-03 12:28 ` Mark Kettenis [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d3cb8336d41b4c01@bloch.sibelius.xs4all.nl \
--to=mark.kettenis@xs4all.nl \
--cc=agraf@csgraf.de \
--cc=ardb@kernel.org \
--cc=ilias.apalodimas@linaro.org \
--cc=u-boot@lists.denx.de \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).