[PATCH] efi_loader: Get rid of kaslr-seed

[PATCH] efi_loader: Get rid of kaslr-seed

[Ilias Apalodimas]

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.apalodimas@linaro.org>
---
 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

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Ard Biesheuvel]

On Thu, 16 Dec 2021 at 15:52, Ilias Apalodimas
<ilias.apalodimas@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.apalodimas@linaro.org>

Acked-by: Ard Biesheuvel <ardb@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.


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

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Mark Kettenis]

> From: Ard Biesheuvel <ardb@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.apalodimas@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.apalodimas@linaro.org>
> 
> Acked-by: Ard Biesheuvel <ardb@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
> >
> 

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Ard Biesheuvel]

On Thu, 16 Dec 2021 at 17:56, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Ard Biesheuvel <ardb@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.apalodimas@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.apalodimas@linaro.org>
> >
> > Acked-by: Ard Biesheuvel <ardb@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.
>

Wouldn't it be better to use this block of entropy to seed a DRBG and
subsequently use that as a source of random numbers?

> 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.
>

I agree that this kind of regression should be avoided. But the
reality is that /chosen/kaslr-seed is Linux internal ABI (EFI
stub<->kernel) that somehow got promoted to boot protocol, in a way
that doesn't even work correctly with the EFI stub itself, since
nobody ever proposed a way to *consume* this kaslr-seed in a way that
permits the EFI stub itself to perform load address randomization when
EFI_RNG_PROTOCOL is absent. Note that randomization of the physical
placement is important not only for physical KASLR but also for
virtual KASLR in Linux. (The virtual placement modulo 2 MiB is decided
by the physical placement directly)

So in my opinion, this is a blatant layering violation, and EFI boot
should be fixed, by implementing EFI_RNG_PROTOCOL in all cases where
u-boot apparently has a source of entropy, as it is able to populate
the kaslr-seed property.

For non-EFI boot, the situation is obviously different, and it's
perfectly fine to use /chosen/kaslr-seed directly for the the parts of
KASLR that can still be used in this case.



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

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Mark Kettenis]

> From: Ard Biesheuvel <ardb@kernel.org>
> Date: Thu, 16 Dec 2021 18:12:02 +0100
> 
> On Thu, 16 Dec 2021 at 17:56, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > From: Ard Biesheuvel <ardb@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.apalodimas@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.apalodimas@linaro.org>
> > >
> > > Acked-by: Ard Biesheuvel <ardb@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.
> >
> 
> Wouldn't it be better to use this block of entropy to seed a DRBG and
> subsequently use that as a source of random numbers?

Hmm, I didn't consider that as an option.  We actually get 512 bits of
entropy from m1n1, which should be good enough to seed a DRBG isn't
it?  Not really my area of expertise though.  So I'll need some help
here.

> > 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.
> >
> 
> I agree that this kind of regression should be avoided. But the
> reality is that /chosen/kaslr-seed is Linux internal ABI (EFI
> stub<->kernel) that somehow got promoted to boot protocol, in a way
> that doesn't even work correctly with the EFI stub itself, since
> nobody ever proposed a way to *consume* this kaslr-seed in a way that
> permits the EFI stub itself to perform load address randomization when
> EFI_RNG_PROTOCOL is absent. Note that randomization of the physical
> placement is important not only for physical KASLR but also for
> virtual KASLR in Linux. (The virtual placement modulo 2 MiB is decided
> by the physical placement directly)
> 
> So in my opinion, this is a blatant layering violation, and EFI boot
> should be fixed, by implementing EFI_RNG_PROTOCOL in all cases where
> u-boot apparently has a source of entropy, as it is able to populate
> the kaslr-seed property.
> 
> For non-EFI boot, the situation is obviously different, and it's
> perfectly fine to use /chosen/kaslr-seed directly for the the parts of
> KASLR that can still be used in this case.
> 
> 
> 
> > > > ---
> > > >  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
> > > >
> > >
> 

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Ard Biesheuvel]

On Thu, 16 Dec 2021 at 18:55, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Date: Thu, 16 Dec 2021 18:12:02 +0100
> >
> > On Thu, 16 Dec 2021 at 17:56, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > >
> > > > From: Ard Biesheuvel <ardb@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.apalodimas@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.apalodimas@linaro.org>
> > > >
> > > > Acked-by: Ard Biesheuvel <ardb@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.
> > >
> >
> > Wouldn't it be better to use this block of entropy to seed a DRBG and
> > subsequently use that as a source of random numbers?
>
> Hmm, I didn't consider that as an option.  We actually get 512 bits of
> entropy from m1n1, which should be good enough to seed a DRBG isn't
> it?  Not really my area of expertise though.  So I'll need some help
> here.
>

Yes, all the DRBGs defined by NIST SP800-90A take a seed in the order
of 300 - 500 bits IIRC.

On an arm64 system that implements the crypto extensions, stringing
together a couple of AES instructions to implement the AES-CTR of
flavor of DRBG should be rather straight-forward, and tiny in terms of
code size.

Of course, reusing an existing implementation would be even better but
I don't know from the top of my head if anything suitable exists under
an appropriate license that we can just drop in.

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Ilias Apalodimas]

Hi,

On Thu, 16 Dec 2021 at 21:00, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 16 Dec 2021 at 18:55, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > > Date: Thu, 16 Dec 2021 18:12:02 +0100
> > >
> > > On Thu, 16 Dec 2021 at 17:56, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > > >
> > > > > From: Ard Biesheuvel <ardb@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.apalodimas@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.apalodimas@linaro.org>
> > > > >
> > > > > Acked-by: Ard Biesheuvel <ardb@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.
> > > >
> > >
> > > Wouldn't it be better to use this block of entropy to seed a DRBG and
> > > subsequently use that as a source of random numbers?
> >
> > Hmm, I didn't consider that as an option.  We actually get 512 bits of
> > entropy from m1n1, which should be good enough to seed a DRBG isn't
> > it?  Not really my area of expertise though.  So I'll need some help
> > here.
> >
>
> Yes, all the DRBGs defined by NIST SP800-90A take a seed in the order
> of 300 - 500 bits IIRC.
>
> On an arm64 system that implements the crypto extensions, stringing
> together a couple of AES instructions to implement the AES-CTR of
> flavor of DRBG should be rather straight-forward, and tiny in terms of
> code size.
>
> Of course, reusing an existing implementation would be even better but
> I don't know from the top of my head if anything suitable exists under
> an appropriate license that we can just drop in.

Right here's an idea.  The main problem I had here was measuring the
DTB.  But measuring means we have a TPM and if we have a TPM we have
an RNG.  So what we could do, is support EFI_RNG_PROTOCOL whenever a
TPM is there.  For those boards I can unconditionally weed out the
kaslr-seed and everyone will be happy.  It won't solve the problem of
ASLR when booting via EFI and a kaslr-seed persists, but we can always
fix that later.


Cheers
/Ilias

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Mark Kettenis]

> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Date: Thu, 16 Dec 2021 16:52:08 +0200
> 
> 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.

NAK

OpenBSD uses the kaslr-seed property in the bootloader to mix in some
additional entropy.  (It will also use EFI_RNG_PROTOCOL if it is
avilable, but most U-Boot boards don't provide that, or at least not
yet).

Even on Linux the EFI stub isn't the only way to load a Linux kernel.
You can use a conventional EFI bootloader like grub.

Cheers,

Mark

> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  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
> 
> 

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Ard Biesheuvel]

On Thu, 16 Dec 2021 at 16:25, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Date: Thu, 16 Dec 2021 16:52:08 +0200
> >
> > 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.
>
> NAK
>
> OpenBSD uses the kaslr-seed property in the bootloader to mix in some
> additional entropy.  (It will also use EFI_RNG_PROTOCOL if it is
> avilable, but most U-Boot boards don't provide that, or at least not
> yet).
>

What is the point of using both the DT property and the protocol if
both are available?

> Even on Linux the EFI stub isn't the only way to load a Linux kernel.
> You can use a conventional EFI bootloader like grub.
>

No, you cannot, at least not on architectures other than x86. GRUB on
ARM always boots via the EFI stub.

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Mark Kettenis]

> From: Ard Biesheuvel <ardb@kernel.org>
> Date: Thu, 16 Dec 2021 16:28:06 +0100
> 
> On Thu, 16 Dec 2021 at 16:25, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > Date: Thu, 16 Dec 2021 16:52:08 +0200
> > >
> > > 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.
> >
> > NAK
> >
> > OpenBSD uses the kaslr-seed property in the bootloader to mix in some
> > additional entropy.  (It will also use EFI_RNG_PROTOCOL if it is
> > avilable, but most U-Boot boards don't provide that, or at least not
> > yet).
> >
> 
> What is the point of using both the DT property and the protocol if
> both are available?

Unless kaslr-seed is coming from a different entropy source, there
probably isn't a point.  But it doesn't hurt and it made the
bootloader code simpler.

It does mean there is some room for compromise though.  If U-Boot
would only remove kaslr-seed if it implements EFI_RNG_PROTOCOL it
wouldn't be a problem.

> > Even on Linux the EFI stub isn't the only way to load a Linux kernel.
> > You can use a conventional EFI bootloader like grub.
> >
> 
> No, you cannot, at least not on architectures other than x86. GRUB on
> ARM always boots via the EFI stub.

Ok.  It isn't immediately clear from the documentation that this is
the case.  It would still be possible to write such a bootloader, but
if it isn't a thing, it isn't a thing.  But not all the world is
Linux.

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Ilias Apalodimas]

Hi Mark, 

On Thu, Dec 16, 2021 at 04:48:04PM +0100, Mark Kettenis wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Date: Thu, 16 Dec 2021 16:28:06 +0100
> > 
> > On Thu, 16 Dec 2021 at 16:25, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > >
> > > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > Date: Thu, 16 Dec 2021 16:52:08 +0200
> > > >
> > > > 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.
> > >
> > > NAK

I don't understand why though.  I can simply send a V2 and have a Kconfig
e.g CONFIG_MEASURE_DTB, which would control stripping the property -- or we 
could save- > strip -> measure -> re-inject (which is a horrible idea but 
that's the current reality).

> > >
> > > OpenBSD uses the kaslr-seed property in the bootloader to mix in some
> > > additional entropy.  (It will also use EFI_RNG_PROTOCOL if it is
> > > avilable, but most U-Boot boards don't provide that, or at least not
> > > yet).
> > >
> > 
> > What is the point of using both the DT property and the protocol if
> > both are available?
> 
> Unless kaslr-seed is coming from a different entropy source, there
> probably isn't a point.  But it doesn't hurt and it made the
> bootloader code simpler.

It does hurt the measurements though.  The current situation is a bit
weird.  Ideally the firmware would provide the DTB and I would be be able
to measure prior to any fixups.  However that's not the reality today.  So
we do have to measure just before we install the config table.  Which means
that all entries that can change across reboots needs to be ignored.

> 
> It does mean there is some room for compromise though.  If U-Boot
> would only remove kaslr-seed if it implements EFI_RNG_PROTOCOL it
> wouldn't be a problem.
> 
> > > Even on Linux the EFI stub isn't the only way to load a Linux kernel.
> > > You can use a conventional EFI bootloader like grub.
> > >
> > 
> > No, you cannot, at least not on architectures other than x86. GRUB on
> > ARM always boots via the EFI stub.
> 
> Ok.  It isn't immediately clear from the documentation that this is
> the case.  It would still be possible to write such a bootloader, but
> if it isn't a thing, it isn't a thing.  But not all the world is
> Linux.

Yea but measuring is a reality (and a spec for all it matters).  So we need
to find some way of fixing this.

Regards
/Ilias

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Heinrich Schuchardt]

On 12/16/21 16:48, Mark Kettenis wrote:
>> From: Ard Biesheuvel <ardb@kernel.org>
>> Date: Thu, 16 Dec 2021 16:28:06 +0100
>>
>> On Thu, 16 Dec 2021 at 16:25, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>>
>>>> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>>> Date: Thu, 16 Dec 2021 16:52:08 +0200
>>>>
>>>> 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.
>>>
>>> NAK
>>>
>>> OpenBSD uses the kaslr-seed property in the bootloader to mix in some
>>> additional entropy.  (It will also use EFI_RNG_PROTOCOL if it is
>>> avilable, but most U-Boot boards don't provide that, or at least not
>>> yet).
>>>
>>
>> What is the point of using both the DT property and the protocol if
>> both are available?
>
> Unless kaslr-seed is coming from a different entropy source, there
> probably isn't a point.  But it doesn't hurt and it made the
> bootloader code simpler.
>
> It does mean there is some room for compromise though.  If U-Boot
> would only remove kaslr-seed if it implements EFI_RNG_PROTOCOL it
> wouldn't be a problem.

Only QEMU's ARM virt machine fills kaslr-seed in the device-tree.

On QEMU the EFI_RNG_PROTOCOL is available if you call QEMU with a
virtio-rng device.

All physical devices will have an EFI_RNG_PROTOCOL if there is a driver
available and enabled in U-Boot. There are two platforms which in the
fixup phase set kaslr-seed using an SMC call but that is after the place
where Ilias's patch is removing this property.

The EFI_RNG_PROTOCOL is the standardized way to provide entropy on UEFI.
This will also work with ACPI based systems. So it would be advisable
for OpenBSD to follow this route.

Best regards

Heinrich

>
>>> Even on Linux the EFI stub isn't the only way to load a Linux kernel.
>>> You can use a conventional EFI bootloader like grub.
>>>
>>
>> No, you cannot, at least not on architectures other than x86. GRUB on
>> ARM always boots via the EFI stub.
>
> Ok.  It isn't immediately clear from the documentation that this is
> the case.  It would still be possible to write such a bootloader, but
> if it isn't a thing, it isn't a thing.  But not all the world is
> Linux.

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Ilias Apalodimas]

Hi Heinrich, 
On Thu, Dec 16, 2021 at 04:59:02PM +0100, Heinrich Schuchardt wrote:
> On 12/16/21 16:48, Mark Kettenis wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > > Date: Thu, 16 Dec 2021 16:28:06 +0100
> > > 
> > > On Thu, 16 Dec 2021 at 16:25, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > > > 
> > > > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > Date: Thu, 16 Dec 2021 16:52:08 +0200
> > > > > 
> > > > > 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.
> > > > 
> > > > NAK
> > > > 
> > > > OpenBSD uses the kaslr-seed property in the bootloader to mix in some
> > > > additional entropy.  (It will also use EFI_RNG_PROTOCOL if it is
> > > > avilable, but most U-Boot boards don't provide that, or at least not
> > > > yet).
> > > > 
> > > 
> > > What is the point of using both the DT property and the protocol if
> > > both are available?
> > 
> > Unless kaslr-seed is coming from a different entropy source, there
> > probably isn't a point.  But it doesn't hurt and it made the
> > bootloader code simpler.
> > 
> > It does mean there is some room for compromise though.  If U-Boot
> > would only remove kaslr-seed if it implements EFI_RNG_PROTOCOL it
> > wouldn't be a problem.
> 

I can limit the stripping if EFI_RNG_PROTOCOL is installed or a specific
Kconfig option is selected and hopefully we can get rid of the Kconfig in
the future. 

> Only QEMU's ARM virt machine fills kaslr-seed in the device-tree.
> 

U-Boot injects it as well in some cases e,g sec_firmware_get_random()

[...]

Regards
/Ilias

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Heinrich Schuchardt]

On 12/16/21 15:52, Ilias Apalodimas 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.apalodimas@linaro.org>
> ---
>   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

name mismatch

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

If the node does not present this is not an error!

Best regards

Heinrich

> +}
> +
>   /**
>    * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
>    *

Re: [PATCH] efi_loader: Get rid of kaslr-seed

[Ilias Apalodimas]

Hi Heinrich, 

> > @@ -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
> 
> name mismatch
> 
> > + *
> > + * 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");
> 
> If the node does not present this is not an error!

Ah true, I'll fix that

Cheers
/Ilias