> From: Ard Biesheuvel <a...@kernel.org>
> Date: Thu, 16 Dec 2021 15:54:55 +0100
Hi Ard, Ilias,
> On Thu, 16 Dec 2021 at 15:52, Ilias Apalodimas
> <ilias.apalodi...@linaro.org> wrote:
> >
> > Right now we unconditionally pass a 'kaslr-seed' property to the kernel
> > if the DTB we ended up in EFI includes the entry. However the kernel
> > EFI stub completely ignores it and only relies on EFI_RNG_PROTOCOL.
> > So let's get rid of it unconditionally since it would mess up the
> > (upcoming) DTB TPM measuring as well.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
>
> Acked-by: Ard Biesheuvel <a...@kernel.org>
>
> Note that the EFI stub itself does not consume the DTB /chosen entry
> for its own randomness needs (i.e., the randomization of the physical
> placement of the kernel, which also affects low order virtual
> placement [bits 16-20]), and will blindly overwrite the seed with
> whatever the EFI_RNG_PROTOCOL returns.
But it will only do that if EFI_RNG_PROTOCOL is implemented and
sucessfully returns some random data. Otherwise "kaslr-seed" will
survive as-is. At least that is how I read the code in
drivers/firmware/efi/libstub/fdt.c:update_fdt().
And this is good. On Apple M1 systems, the Apple bootloader actually
provides a block of entropy the the kernel in their version of the
device tree. The m1n1 bootloader uses this entropy to populate the
kaslr-seed property in the device tree it passes on. And U-Boot is
used to provide an EFI implementation such that we can boot a wide
variety of OSes. At this point we don't know yet whether the SoC
includes an RNG that we can use to implement EFI_RNG_PROTOCOL in
U-Boot.
So the effect of tis change is that a Linux kernel on this platform
will run without KASLR. That doesn't seem acceptable to me.
> > ---
> > cmd/bootefi.c | 2 ++
> > include/efi_loader.h | 2 ++
> > lib/efi_loader/efi_dt_fixup.c | 22 ++++++++++++++++++++++
> > 3 files changed, 26 insertions(+)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index d77d3b6e943d..25f9bfce9b84 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_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..e560401ac54f 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_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..02f7de73872e 100644
> > --- a/lib/efi_loader/efi_dt_fixup.c
> > +++ b/lib/efi_loader/efi_dt_fixup.c
> > @@ -40,6 +40,28 @@ static void efi_reserve_memory(u64 addr, u64 size, bool
> > nomap)
> > addr, size);
> > }
> >
> > +/**
> > + * efi_remove_kaslr_seed() - Removed unused kaslr-seed
> > + *
> > + * Kernel's EFI STUB only relies on EFI_RNG_PROTOCOL for randomization
> > + * and completely ignores the kaslr-seed. 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_purge_kaslr_seed(void *fdt)
> > +{
> > + int nodeoff = fdt_path_offset(fdt, "/chosen");
> > + int err = 0;
> > +
> > + if (nodeoff < 0)
> > + return;
> > +
> > + err = fdt_delprop(fdt, nodeoff, "kaslr-seed");
> > + if (err < 0)
> > + log_err("Error deleting kaslr-seed\n");
> > +}
> > +
> > /**
> > * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
> > *
> > --
> > 2.30.2
> >
>
