[PATCH v3] fdt: automatically add /chosen/kaslr-seed if DM_RNG is enabled

[Michal Simek]

st 22. 5. 2024 v 17:19 odesílatel Tim Harvey <tharvey at gateworks.com> napsal:
>
> On Wed, May 22, 2024 at 12:47 AM Michal Simek <michal.simek at amd.com> wrote:
> >
> >
> >
> > On 5/21/24 22:59, 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.
> > >
> > > If we have DM_RNG enabled populate this value automatically when
> > > fdt_chosen is called. We skip this if ARMV8_SEC_FIRMWARE_SUPPORT
> > > is enabled as it's implementation uses a different source of entropy
> > > that is not yet implemented as DM_RNG. We also skip this if
> > > MEASURED_BOOT is enabled as in that case any modifications to the
> > > dt will cause measured boot to fail (although there are many other
> > > places the dt is altered).
> > >
> > > As this fdt node is added elsewhere create a library function and
> > > use it to deduplicate code. We will provide a parameter to specify the
> > > index of the rng device as well as a boolean to overwrite if present.
> > >
> > > For our automatic injection, we will use the first rng device and
> > > not overwrite if already present with a non-zero value (which may
> > > have been populated by an earlier boot stage). This way if a board
> > > specific ft_board_setup() function wants to customize this behavior
> > > it can call fdt_kaslrseed with a rng device index of its choosing and
> > > set overwrite true.
> > >
> > > 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.
> > >
> > > 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.
> > >
> > > Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> > > Cc: Michal Simek <michal.simek at amd.com>
> > > Cc: Andy Yan <andy.yan at rock-chips.com>
> > > Cc: Akash Gajjar <gajjar04akash at gmail.com>
> > > Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > Cc: Simon Glass <sjg at chromium.org>
> > > Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> > > Cc: Patrice Chotard <patrice.chotard at foss.st.com>
> > > Cc: Devarsh Thakkar <devarsht at ti.com>
> > > Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > > Cc: Hugo Villeneuve <hvilleneuve at dimonoff.com>
> > > Cc: Marek Vasut <marex at denx.de>
> > > Cc: Tom Rini <trini at konsulko.com>
> > > Cc: Chris Morgan <macromorgan at hotmail.com>
> > > ---
> > > 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
> > > ---
> > >   board/xilinx/common/board.c | 35 -------------------------
> > >   boot/fdt_support.c          |  6 +++++
> > >   boot/pxe_utils.c            | 35 ++-----------------------
> > >   cmd/kaslrseed.c             | 45 +++++---------------------------
> > >   include/kaslrseed.h         | 19 ++++++++++++++
> > >   lib/Makefile                |  1 +
> > >   lib/kaslrseed.c             | 51 +++++++++++++++++++++++++++++++++++++
> > >   7 files changed, 85 insertions(+), 107 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));
> > >
> >
> > one more thing here. Please also removed unused variables.
> >
> > board/xilinx/common/board.c: In function 'ft_board_setup':
> > board/xilinx/common/board.c:707:25: warning: unused variable 'ret'
> > [-Wunused-variable]
> >    707 |         int nodeoffset, ret;
> >        |                         ^~~
> > board/xilinx/common/board.c:707:13: warning: unused variable 'nodeoffset'
> > [-Wunused-variable]
> >    707 |         int nodeoffset, ret;
> >        |             ^~~~~~~~~~
> >    AS      arch/arm/cpu/armv8/cache.o
> > board/xilinx/common/board.c:706:12: warning: unused variable 'buf'
> > [-Wunused-variable]
> >    706 |         u8 buf[MAX_RAND_SIZE];
> >        |            ^~~
> > board/xilinx/common/board.c:705:25: warning: unused variable 'dev'
> > [-Wunused-variable]
> >    705 |         struct udevice *dev;
> >        |                         ^~~
> > board/xilinx/common/board.c:704:16: warning: unused variable 'n' [-Wunused-variable]
> >    704 |         size_t n = MAX_RAND_SIZE;
> >        |                ^
> >
>
> Hi Michal,
>
> Thanks... missed those. What defconfig are you building that gets to
> this codepath?

xilinx_zynqmp_virt_defconfig

>
> I'll send a v4 after another day or so for others to respond.
>
> With these changes (and the previous response) do you have a rb or
> tested-by tag for me?

The rest looks good to me and no problem to provide tags.

Thanks,
Michal