On Thu, Jun 20, 2024 at 6:58 AM Caleb Connolly <caleb.conno...@linaro.org> wrote: > > Hi Tim, > > On 18/06/2024 23:06, Tim Harvey 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. > > > > Add a fdt_kaslrseed function to accommodate this allowing an existing > > node to be overwritten if present. For now use the first rng device > > but it would be good to enhance this in the future to allow some sort > > of selection or policy in choosing the rng device used. > > > > Signed-off-by: Tim Harvey <thar...@gateworks.com> > > Reviewed-by: Simon Glass <s...@chromium.org> > > Cc: Michal Simek <michal.si...@amd.com> > > Cc: Andy Yan <andy....@rock-chips.com> > > Cc: Akash Gajjar <gajjar04ak...@gmail.com> > > Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org> > > Cc: Simon Glass <s...@chromium.org> > > Cc: Patrick Delaunay <patrick.delau...@foss.st.com> > > Cc: Patrice Chotard <patrice.chot...@foss.st.com> > > Cc: Devarsh Thakkar <devar...@ti.com> > > Cc: Heinrich Schuchardt <xypron.g...@gmx.de> > > Cc: Hugo Villeneuve <hvillene...@dimonoff.com> > > Cc: Marek Vasut <ma...@denx.de> > > Cc: Tom Rini <tr...@konsulko.com> > > Cc: Chris Morgan <macromor...@hotmail.com> > > --- > > v6: > > - collected tags > > v5: > > - move function to boot/fdt_support.c > > - remove ability to select rng index and note in the commit log > > something like this as a future enhancement. > > - fixed typo in commit message s/it's/its/ > > - use cmd_process_error per Michal's suggestion > > v4: > > - add missing /n to notice in kaslrseed cmd > > - combine ints in declaration > > - remove unused vars from board/xilinx/common/board.c ft_board_setup > > v3: > > - skip if CONFIG_MEASURED_BOOT > > - fix skip for CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT > > - pass in rng index and bool to specify overwrite > > - remove duplicate error strings printed outside of fdt_kaslrseed > > - added note to commit log about how EFI STUB weeds out kalsr-seed > > 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 > > --- > > boot/fdt_support.c | 44 +++++++++++++++++++++++++++++++++++++++++++ > > include/fdt_support.h | 10 ++++++++++ > > 2 files changed, 54 insertions(+) > > > > diff --git a/boot/fdt_support.c b/boot/fdt_support.c > > index 2bd80a9dfb18..b1b2679dea0c 100644 > > --- a/boot/fdt_support.c > > +++ b/boot/fdt_support.c > > @@ -7,12 +7,15 @@ > > */ > > > > #include <common.h> > > +#include <dm.h> > > #include <abuf.h> > > #include <env.h> > > #include <log.h> > > #include <mapmem.h> > > #include <net.h> > > +#include <rng.h> > > #include <stdio_dev.h> > > +#include <dm/device_compat.h> > > #include <dm/ofnode.h> > > #include <linux/ctype.h> > > #include <linux/types.h> > > @@ -274,6 +277,47 @@ int fdt_initrd(void *fdt, ulong initrd_start, ulong > > initrd_end) > > return 0; > > } > > > > +int fdt_kaslrseed(void *fdt, bool overwrite) > > +{ > > + int len, err, nodeoffset; > > + struct udevice *dev; > > + const u64 *orig; > > + u64 data = 0; > > + > > + err = fdt_check_header(fdt); > > + if (err < 0) > > + return err; > > All the paths that call fdt_kaslrseed() call fdt_check_header() > beforehand, I think it's safe to drop this.
Hi Caleb, The do_kaslr_seed path does not check for this and instead passes working_fdt - perhaps your point is still valid if working_fdt is guaranteed to always be a valid fdt? > > + > > + /* find or create "/chosen" node. */ > > + nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen"); > > + if (nodeoffset < 0) > > + return nodeoffset; > > Arguably, this shouldn't be the responsibility of this function, maybe > it would be better to error our here? I'm not sure what your point is here... it is erroring out. Best Regards, Tim > > + > > + /* return without error if we are not overwriting and existing > > non-zero node */ > > + orig = fdt_getprop(fdt, nodeoffset, "kaslr-seed", &len); > > + if (orig && len == sizeof(*orig)) > > + data = fdt64_to_cpu(*orig); > > + if (data && !overwrite) { > > + debug("not overwriting existing kaslr-seed\n"); > > + return 0; > > + } > > + err = uclass_get_device(UCLASS_RNG, 0, &dev); > > + if (err) { > > + printf("No RNG device\n"); > > + return err; > > + } > > + err = dm_rng_read(dev, &data, sizeof(data)); > > + if (err) { > > + dev_err(dev, "dm_rng_read failed: %d\n", err); > > + return 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; > > +} > > + > > /** > > * board_fdt_chosen_bootargs - boards may override this function to use > > * alternative kernel command line arguments > > diff --git a/include/fdt_support.h b/include/fdt_support.h > > index 4b71b8948d99..741e2360c224 100644 > > --- a/include/fdt_support.h > > +++ b/include/fdt_support.h > > @@ -463,4 +463,14 @@ void fdt_fixup_board_enet(void *blob); > > #ifdef CONFIG_CMD_PSTORE > > void fdt_fixup_pstore(void *blob); > > #endif > > + > > +/** > > + * fdt_kaslrseed() - create a 'kaslr-seed' node in chosen > > + * > > + * @blob: fdt blob > > + * @overwrite: do not overwrite existing non-zero node unless true > > + * Return: 0 if OK, -ve on error > > + */ > > +int fdt_kaslrseed(void *blob, bool overwrite); > > + > > #endif /* ifndef __FDT_SUPPORT_H */ > > -- > // Caleb (they/them)