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