?Hi, On Mon, 22 Aug 2022 at 01:34, Rasmus Villemoes <rasmus.villem...@prevas.dk> wrote: > > A recurring theme on LKML is the boot process deadlocking due to some > process blocking waiting for random numbers, while the kernel's > Cryptographic Random Number Generator (crng) is not initalized yet, > but that very blocking means no activity happens that would generate > the entropy necessary to finalize seeding the crng. > > This is not a problem on boards that have a good hwrng (when the > kernel is configured to trust it), whether in the CPU or in a TPM or > elsewhere. However, that's far from all boards out there. Moreover, > there are consumers in the kernel that try to obtain random numbers > very early, before the kernel has had any chance to initialize any > hwrng or other peripherals. > > Allow a board to provide a board_rng_seed() function, which is > responsible for providing a value to be put into the rng-seed property > under the /chosen node. > > The board code is responsible for how to actually obtain those > bytes. > > - One possibility is for the board to load a seed "file" from > somewhere (it need not be a file in a filesystem of course), and > then ensure that that the same seed file does not get used on > subsequent boots. > > * One way to do that is to delete the file, or otherwise mark it as > invalid, then rely on userspace to create a new one, and living > with the possibility of not finding a seed file during some boots. > > * Another is to use the scheme used by systemd-boot and create a new > seed file immediately, but in a way that the seed passed to the > kernel and the new (i.e. next) seed cannot be deduced from each > other, see the explanation at > https://lore.kernel.org/lkml/20190929090512.GB13049@gardel-login/ > and the current code at > https://github.com/systemd/systemd/blob/main/src/boot/efi/random-seed.c > > - The board may have an hwrng from which some bytes can be read; while > the kernel can also do that, doing it in U-Boot and providing a seed > ensures that even very early users in the kernel get good random > numbers. > > - If the board has a sensor of some sort (temperature, humidity, GPS, > RTC, whatever), mixing in a reading of that doesn't hurt. > > - etc. etc. > > These can of course be combined. > > The rng-seed property is mixed into the pool used by the linux > kernel's CRNG very early during boot. Whether it then actually > contributes towards the kernel considering the CRNG initialized > depends on whether the kernel has been configured with > CONFIG_RANDOM_TRUST_BOOTLOADER (nowadays overridable via the > random.trust_bootloader command line option). But that's for the BSP > developer to ultimately decide. > > So, if the board needs to have all that logic, why not also just have > it do the actual population of /chosen/rng-seed in ft_board_setup(), > which is not that many extra lines of code? > > I considered that, but decided handling this logically belongs in > fdt_chosen(). Also, apart from saving the board code from the few > lines of boilerplate, doing it in ft_board_setup() is too late for at > least some use cases. For example, I want to allow the board logic to > decide > > ok, let's pass back this buffer and use that as seed, but also let's > set random.trust_bootloader=n so no entropy is credited. > > This requires the rng-seed handling to happen before bootargs > handling. For example, during the very first boot, the board might not > have a proper seed file, but the board could still return (a hash of) > some CPU serial# or whatnot, so that at least no two boards ever get > the same seed - the kernel always mixes in the value passed in > rng-seed, but if it is not "trusted", the kernel would still go > through the same motions as it would if no rng-seed was passed before > considering its CRNG initialized. I.e., by returning that > unique-to-this-board value and setting random.trust_bootloader=n, the > board would be no worse off than if board_rng_seed() returned nothing > at all. > > Signed-off-by: Rasmus Villemoes <rasmus.villem...@prevas.dk> > --- > common/Kconfig | 14 ++++++++++++++ > common/fdt_support.c | 13 +++++++++++++ > include/fdt_support.h | 13 +++++++++++++ > 3 files changed, 40 insertions(+) > > diff --git a/common/Kconfig b/common/Kconfig > index e7914ca750..ebee856e56 100644 > --- a/common/Kconfig > +++ b/common/Kconfig > @@ -768,6 +768,20 @@ config TPL_STACKPROTECTOR > bool "Stack Protector buffer overflow detection for TPL" > depends on STACKPROTECTOR && TPL > > +config BOARD_RNG_SEED > + bool "Provide /chosen/rng-seed property to the linux kernel" > + help > + Selecting this option requires the board to define a > + board_rng_seed() function, which should return a buffer > + which will be used to populate the /chosen/rng-seed property > + in the device tree for the OS being booted. > + > + It is up to the board code (and more generally the whole > + BSP) where and how to store (or generate) such a seed, how > + to ensure a given seed is only used once, how to create a > + new seed for use on subsequent boots, and whether or not the > + kernel should account any entropy from the given seed. > + > endmenu > > menu "Update support" > diff --git a/common/fdt_support.c b/common/fdt_support.c > index 8c18af2ce1..baf7fb7065 100644 > --- a/common/fdt_support.c > +++ b/common/fdt_support.c > @@ -7,6 +7,7 @@ > */ > > #include <common.h> > +#include <abuf.h> > #include <env.h> > #include <log.h> > #include <mapmem.h> > @@ -279,6 +280,7 @@ __weak char *board_fdt_chosen_bootargs(void) > > int fdt_chosen(void *fdt) > { > + struct abuf buf = {}; > int nodeoffset; > int err; > char *str; /* used to set string properties */ > @@ -294,6 +296,17 @@ int fdt_chosen(void *fdt) > if (nodeoffset < 0) > return nodeoffset; > > + if (IS_ENABLED(CONFIG_BOARD_RNG_SEED) && !board_rng_seed(&buf)) { > + err = fdt_setprop(fdt, nodeoffset, "rng-seed", > + abuf_data(&buf), abuf_size(&buf)); > + abuf_uninit(&buf); > + if (err < 0) { > + printf("WARNING: could not set rng-seed %s.\n", > + fdt_strerror(err)); > + return err; > + } > + } > + > str = board_fdt_chosen_bootargs(); > > if (str) { > diff --git a/include/fdt_support.h b/include/fdt_support.h > index ac76939e81..b8380716f3 100644 > --- a/include/fdt_support.h > +++ b/include/fdt_support.h > @@ -11,6 +11,7 @@ > > #include <asm/u-boot.h> > #include <linux/libfdt.h> > +#include <abuf.h> > > /** > * arch_fixup_fdt() - Write arch-specific information to fdt > @@ -186,6 +187,18 @@ int fdt_find_or_add_subnode(void *fdt, int parentoffset, > const char *name); > */ > int ft_board_setup(void *blob, struct bd_info *bd); > > +/** > + * board_rng_seed() - Provide a seed to be passed via /chosen/rng-seed > + * > + * This function is called if CONFIG_BOARD_RNG_SEED is set, and must > + * be provided by the board. It should return, via @buf, some suitable > + * seed value to pass to the kernel. > + * > + * @param buf A struct abuf for returning the seed and its size. > + * @return 0 if ok, negative on error. > + */ > +int board_rng_seed(struct abuf *buf);
Instead of yet another hook, can we use EVT_FT_FIXUP? An even better option might be to use EVT_FT_FIXUP and then call a UCLASS_BOARD method to obtain the information. > + > /** > * board_fdt_chosen_bootargs() - Arbitrarily amend fdt kernel command line > * > -- > 2.31.1 > Regards, Simon