Hi Tim, On Tue, 21 May 2024 at 19:37, Tim Harvey <thar...@gateworks.com> wrote: > > On Tue, May 21, 2024 at 5:05 AM Ilias Apalodimas > <ilias.apalodi...@linaro.org> wrote: > > > > Hi Tim > > > > On Wed, 15 May 2024 at 23:50, Tim Harvey <thar...@gateworks.com> wrote: > > > > > > If RANDOMIZE_BASE is enabled in the Linux kernel instructing it to > > > randomize the virtual address at which the kernel image is loaded, it > > > expects entropy to be provided by the bootloader by populating > > > /chosen/kaslr-seed with a 64-bit value from source of entropy at boot. > > > > Since you'll send a v2, mind adding some description for UEFI on the > > commit message? > > efi_try_purge_kaslr_seed() has a comment of the behaviour > > > > Hi Ilias, > > Ok, I will add the following to the commit log for clarity: > Note that the 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). It gets weeded out from the DTB that gets handed over via > efi_install_fdt() as it would also mess up the measured boot DTB TPM > measurements as well. > > Does the above mean that I don't need to worry about protecting the > call to fdt_kaslrseed() with a check for MEASURED BOOT as > efi_install_fdt/efi_try_purge_kaslr_seed always gets called in that > case?
There are 2 ways to do measured boot. One is via the bootm command and the other is via the EFI protocols. CONFIG_MEASURED_BOOT only applies to bootm. OTOH kaslr-seed might still have value if EFI_RNG is disabled, even when booting with EFI, since the kernel will randomize the virtual placement if it finds that. But in any case, a kaslr-seed entry can't be present in the DTB if you plan to measure it. Cheers /Ilias > > Best Regards, > > Tim > > > Thanks > > /Ilias > > > > > > If we have DM_RNG enabled populate this value automatically when > > > fdt_chosen is called unless ARMV8_SEC_FIRMWARE_SUPPORT is enabled as > > > it's implementation uses a different source of entropy. > > > > > > As this fdt node is added elsewhere create a library function and > > > use it to deduplicate code. > > > > > > Note that the kalsrseed command (CMD_KASLRSEED) is likely pointless now > > > but left in place in case boot scripts exist that rely on this command > > > existing and returning success. An informational message is printed to > > > alert users of this command that it is likely no longer needed. > > > > > > Signed-off-by: Tim Harvey <thar...@gateworks.com> > > > --- > > > v2: > > > - fix typo in commit msg > > > - use stack for seed to avoid unecessary malloc/free > > > - move to a library function and deduplicate code by using it elsewhere > > > --- > > > board/xilinx/common/board.c | 35 ----------------------------- > > > boot/fdt_support.c | 10 +++++++++ > > > boot/pxe_utils.c | 35 +++-------------------------- > > > cmd/kaslrseed.c | 45 ++++++------------------------------- > > > include/kaslrseed.h | 17 ++++++++++++++ > > > lib/Makefile | 1 + > > > lib/kaslrseed.c | 34 ++++++++++++++++++++++++++++ > > > 7 files changed, 72 insertions(+), 105 deletions(-) > > > create mode 100644 include/kaslrseed.h > > > create mode 100644 lib/kaslrseed.c > > > > > > diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c > > > index 30a81376ac41..f741e8957818 100644 > > > --- a/board/xilinx/common/board.c > > > +++ b/board/xilinx/common/board.c > > > @@ -713,41 +713,6 @@ int ft_board_setup(void *blob, struct bd_info *bd) > > > if (IS_ENABLED(CONFIG_FDT_FIXUP_PARTITIONS) && > > > IS_ENABLED(CONFIG_NAND_ZYNQ)) > > > fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes)); > > > > > > - if (uclass_get_device(UCLASS_RNG, 0, &dev) || !dev) { > > > - debug("No RNG device\n"); > > > - return 0; > > > - } > > > - > > > - if (dm_rng_read(dev, buf, n)) { > > > - debug("Reading RNG failed\n"); > > > - return 0; > > > - } > > > - > > > - if (!blob) { > > > - debug("No FDT memory address configured. Please > > > configure\n" > > > - "the FDT address via \"fdt addr <address>\" > > > command.\n" > > > - "Aborting!\n"); > > > - return 0; > > > - } > > > - > > > - ret = fdt_check_header(blob); > > > - if (ret < 0) { > > > - debug("fdt_chosen: %s\n", fdt_strerror(ret)); > > > - return ret; > > > - } > > > - > > > - nodeoffset = fdt_find_or_add_subnode(blob, 0, "chosen"); > > > - if (nodeoffset < 0) { > > > - debug("Reading chosen node failed\n"); > > > - return nodeoffset; > > > - } > > > - > > > - ret = fdt_setprop(blob, nodeoffset, "kaslr-seed", buf, > > > sizeof(buf)); > > > - if (ret < 0) { > > > - debug("Unable to set kaslr-seed on chosen node: %s\n", > > > fdt_strerror(ret)); > > > - return ret; > > > - } > > > - > > > return 0; > > > } > > > #endif > > > diff --git a/boot/fdt_support.c b/boot/fdt_support.c > > > index 874ca4d6f5af..3455d60d69dc 100644 > > > --- a/boot/fdt_support.c > > > +++ b/boot/fdt_support.c > > > @@ -8,6 +8,7 @@ > > > > > > #include <abuf.h> > > > #include <env.h> > > > +#include <kaslrseed.h> > > > #include <log.h> > > > #include <mapmem.h> > > > #include <net.h> > > > @@ -300,6 +301,15 @@ int fdt_chosen(void *fdt) > > > if (nodeoffset < 0) > > > return nodeoffset; > > > > > > + if (IS_ENABLED(CONFIG_DM_RNG) && > > > !IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT)) { > > > + err = fdt_kaslrseed(fdt); > > > + if (err) { > > > + printf("WARNING: could not set kaslr-seed %s.\n", > > > + fdt_strerror(err)); > > > + return err; > > > + } > > > + } > > > + > > > if (IS_ENABLED(CONFIG_BOARD_RNG_SEED) && !board_rng_seed(&buf)) { > > > err = fdt_setprop(fdt, nodeoffset, "rng-seed", > > > abuf_data(&buf), abuf_size(&buf)); > > > diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c > > > index 4b22bb6f525a..8d70233fc08d 100644 > > > --- a/boot/pxe_utils.c > > > +++ b/boot/pxe_utils.c > > > @@ -8,6 +8,7 @@ > > > #include <dm.h> > > > #include <env.h> > > > #include <image.h> > > > +#include <kaslrseed.h> > > > #include <log.h> > > > #include <malloc.h> > > > #include <mapmem.h> > > > @@ -323,10 +324,6 @@ static void label_boot_kaslrseed(void) > > > #if CONFIG_IS_ENABLED(DM_RNG) > > > ulong fdt_addr; > > > struct fdt_header *working_fdt; > > > - size_t n = 0x8; > > > - struct udevice *dev; > > > - u64 *buf; > > > - int nodeoffset; > > > int err; > > > > > > /* Get the main fdt and map it */ > > > @@ -342,35 +339,9 @@ static void label_boot_kaslrseed(void) > > > if (err <= 0) > > > return; > > > > > > - if (uclass_get_device(UCLASS_RNG, 0, &dev) || !dev) { > > > - printf("No RNG device\n"); > > > - return; > > > - } > > > - > > > - nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen"); > > > - if (nodeoffset < 0) { > > > - printf("Reading chosen node failed\n"); > > > - return; > > > - } > > > - > > > - buf = malloc(n); > > > - if (!buf) { > > > - printf("Out of memory\n"); > > > - return; > > > - } > > > - > > > - if (dm_rng_read(dev, buf, n)) { > > > - printf("Reading RNG failed\n"); > > > - goto err; > > > - } > > > - > > > - err = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf, > > > sizeof(buf)); > > > - if (err < 0) { > > > + err = fdt_kaslrseed(working_fdt); > > > + if (err < 0) > > > printf("Unable to set kaslr-seed on chosen node: %s\n", > > > fdt_strerror(err)); > > > - goto err; > > > - } > > > -err: > > > - free(buf); > > > #endif > > > return; > > > } > > > diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c > > > index e0d3c7fe7489..ea5c07d729cf 100644 > > > --- a/cmd/kaslrseed.c > > > +++ b/cmd/kaslrseed.c > > > @@ -9,33 +9,16 @@ > > > #include <command.h> > > > #include <dm.h> > > > #include <hexdump.h> > > > +#include <kaslrseed.h> > > > #include <malloc.h> > > > #include <rng.h> > > > #include <fdt_support.h> > > > > > > static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int argc, char > > > *const argv[]) > > > { > > > - size_t n = 0x8; > > > - struct udevice *dev; > > > - u64 *buf; > > > - int nodeoffset; > > > - int ret = CMD_RET_SUCCESS; > > > + int err; > > > > > > - if (uclass_get_device(UCLASS_RNG, 0, &dev) || !dev) { > > > - printf("No RNG device\n"); > > > - return CMD_RET_FAILURE; > > > - } > > > - > > > - buf = malloc(n); > > > - if (!buf) { > > > - printf("Out of memory\n"); > > > - return CMD_RET_FAILURE; > > > - } > > > - > > > - if (dm_rng_read(dev, buf, n)) { > > > - printf("Reading RNG failed\n"); > > > - return CMD_RET_FAILURE; > > > - } > > > + printf("Notice: a /chosen/kaslr-seed is automatically added to > > > the device-tree when booted via booti/bootm/bootz therefore using this > > > command is likely no longer needed"); > > > > > > if (!working_fdt) { > > > printf("No FDT memory address configured. Please > > > configure\n" > > > @@ -44,27 +27,13 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int > > > flag, int argc, char *const > > > return CMD_RET_FAILURE; > > > } > > > > > > - ret = fdt_check_header(working_fdt); > > > - if (ret < 0) { > > > - printf("fdt_chosen: %s\n", fdt_strerror(ret)); > > > + err = fdt_kaslrseed(working_fdt); > > > + if (err < 0) { > > > + printf("Unable to set kaslr-seed on chosen node: %s\n", > > > fdt_strerror(err)); > > > return CMD_RET_FAILURE; > > > } > > > > > > - nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen"); > > > - if (nodeoffset < 0) { > > > - printf("Reading chosen node failed\n"); > > > - return CMD_RET_FAILURE; > > > - } > > > - > > > - ret = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf, > > > sizeof(buf)); > > > - if (ret < 0) { > > > - printf("Unable to set kaslr-seed on chosen node: %s\n", > > > fdt_strerror(ret)); > > > - return CMD_RET_FAILURE; > > > - } > > > - > > > - free(buf); > > > - > > > - return ret; > > > + return CMD_RET_SUCCESS; > > > } > > > > > > U_BOOT_LONGHELP(kaslrseed, > > > diff --git a/include/kaslrseed.h b/include/kaslrseed.h > > > new file mode 100644 > > > index 000000000000..3fe82f418acf > > > --- /dev/null > > > +++ b/include/kaslrseed.h > > > @@ -0,0 +1,17 @@ > > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > > +/* > > > + * Copyright 2024 Tim Harvey <thar...@gateworks.com> > > > + */ > > > + > > > +#if !defined _KASLRSEED_H_ > > > +#define _KASLRSEED_H_ > > > + > > > +/** > > > + * fdt_kaslrseed() - create a 'kaslr-seed' node in chosen > > > + * > > > + * @blob: fdt blob > > > + * Return: 0 if OK, -ve on error > > > + */ > > > +int fdt_kaslrseed(void *blob); > > > + > > > +#endif /* _KASLRSEED_H_ */ > > > diff --git a/lib/Makefile b/lib/Makefile > > > index 2a76acf100d0..20a0242055fa 100644 > > > --- a/lib/Makefile > > > +++ b/lib/Makefile > > > @@ -149,6 +149,7 @@ obj-y += date.o > > > obj-y += rtc-lib.o > > > obj-$(CONFIG_LIB_ELF) += elf.o > > > > > > +obj-$(CONFIG_DM_RNG) += kaslrseed.o > > > obj-$(CONFIG_$(SPL_TPL_)SEMIHOSTING) += semihosting.o > > > > > > # > > > diff --git a/lib/kaslrseed.c b/lib/kaslrseed.c > > > new file mode 100644 > > > index 000000000000..ad06bee2b88d > > > --- /dev/null > > > +++ b/lib/kaslrseed.c > > > @@ -0,0 +1,34 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Copyright 2024 Tim Harvey <thar...@gateworks.com> > > > + */ > > > +#include <dm.h> > > > +#include <rng.h> > > > +#include <fdt_support.h> > > > + > > > +int fdt_kaslrseed(void *fdt) > > > +{ > > > + struct udevice *dev; > > > + int nodeoffset; > > > + u64 data; > > > + int err; > > > + > > > + err = fdt_check_header(fdt); > > > + if (err < 0) > > > + return err; > > > + > > > + /* find or create "/chosen" node. */ > > > + nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen"); > > > + if (nodeoffset < 0) > > > + return nodeoffset; > > > + > > > + err = uclass_get_device(UCLASS_RNG, 0, &dev); > > > + if (!err) > > > + err = dm_rng_read(dev, &data, sizeof(data)); > > > + if (!err) > > > + err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", &data, > > > sizeof(data)); > > > + if (err < 0) > > > + printf("WARNING: could not set kaslr-seed %s.\n", > > > fdt_strerror(err)); > > > + > > > + return err; > > > +} > > > -- > > > 2.25.1 > > >