[PATCH] fdt_support: add optional board_rng_seed() hook

[Simon Glass]

?Hi,

On Mon, 22 Aug 2022 at 01:34, Rasmus Villemoes
<rasmus.villemoes at 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.villemoes at 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