From: Tom Rini <trini@konsulko.com>
To: Tim Harvey <tharvey@gateworks.com>,
Hou Zhiqiang <Zhiqiang.Hou@nxp.com>,
Ruchika Gupta <ruchika.gupta@nxp.com>,
Sean Anderson <sean.anderson@seco.com>,
Thomas Hebb <tommyhebb@gmail.com>,
Sumit Garg <sumit.garg@nxp.com>, Michael Walle <michael@walle.cc>
Cc: u-boot@lists.denx.de, Michal Simek <michal.simek@amd.com>,
Andy Yan <andy.yan@rock-chips.com>,
Akash Gajjar <gajjar04akash@gmail.com>,
Ilias Apalodimas <ilias.apalodimas@linaro.org>,
Simon Glass <sjg@chromium.org>,
Patrick Delaunay <patrick.delaunay@foss.st.com>,
Patrice Chotard <patrice.chotard@foss.st.com>,
Devarsh Thakkar <devarsht@ti.com>,
Heinrich Schuchardt <xypron.glpk@gmx.de>,
Hugo Villeneuve <hvilleneuve@dimonoff.com>,
Marek Vasut <marex@denx.de>
Subject: Re: [PATCH v2] fdt: automatically add /chosen/kaslr-seed if DM_RNG is enabled
Date: Thu, 16 May 2024 10:15:10 -0600 [thread overview]
Message-ID: <20240516161510.GB2568172@bill-the-cat> (raw)
In-Reply-To: <CAJ+vNU16cn31h_W4g6of8wDTNZNfuzm42K0B-_BMX6zeJSJTmg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11894 bytes --]
On Thu, May 16, 2024 at 08:58:09AM -0700, Tim Harvey wrote:
> On Wed, May 15, 2024 at 1:50 PM Tim Harvey <tharvey@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.
> >
> > 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 <tharvey@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 <tharvey@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 <tharvey@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;
> > +}
[snip]
> Tom, can sec_firmware.c be moved to dm-rng to eliminate the need for a
> separate fdt_fixup_kaslr?
I'm really just the "last resort" on that file, and git history by
default is too short, in this case. Adding a few more people now.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
next prev parent reply other threads:[~2024-05-16 16:15 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-15 20:50 [PATCH v2] fdt: automatically add /chosen/kaslr-seed if DM_RNG is enabled Tim Harvey
2024-05-15 21:06 ` Marek Vasut
2024-05-15 21:11 ` Tim Harvey
2024-05-15 21:15 ` Marek Vasut
2024-05-16 15:58 ` Tim Harvey
2024-05-16 16:15 ` Tom Rini [this message]
2024-05-20 8:29 ` Michal Simek
2024-05-20 16:37 ` Tim Harvey
2024-05-21 6:57 ` Michal Simek
2024-05-21 15:50 ` Chris Morgan
2024-05-21 12:04 ` Ilias Apalodimas
2024-05-21 16:36 ` Tim Harvey
2024-05-21 16:53 ` Ilias Apalodimas
2024-05-21 17:05 ` Tim Harvey
2024-05-21 17:29 ` Ilias Apalodimas
2024-05-21 18:05 ` Tim Harvey
2024-05-21 18:58 ` Ilias Apalodimas
2024-05-21 20:53 ` Tim Harvey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240516161510.GB2568172@bill-the-cat \
--to=trini@konsulko.com \
--cc=Zhiqiang.Hou@nxp.com \
--cc=andy.yan@rock-chips.com \
--cc=devarsht@ti.com \
--cc=gajjar04akash@gmail.com \
--cc=hvilleneuve@dimonoff.com \
--cc=ilias.apalodimas@linaro.org \
--cc=marex@denx.de \
--cc=michael@walle.cc \
--cc=michal.simek@amd.com \
--cc=patrice.chotard@foss.st.com \
--cc=patrick.delaunay@foss.st.com \
--cc=ruchika.gupta@nxp.com \
--cc=sean.anderson@seco.com \
--cc=sjg@chromium.org \
--cc=sumit.garg@nxp.com \
--cc=tharvey@gateworks.com \
--cc=tommyhebb@gmail.com \
--cc=u-boot@lists.denx.de \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).