On Tue, May 21, 2024 at 9:54 AM Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > 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.
ok, sounds like I need to use: if (IS_ENABLED(CONFIG_DM_RNG) && !IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT)) && !IS_ENABLED(CONFIG_MEASURED_BOOT) { fdt_kaslrseed... } Thanks, I'll send a v3 hopefully later today. Tim > > 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 > > > >