Hey :)
On Mon, 2025-11-24 at 13:00 +0100, Ahmad Fatoum wrote:
> Hi Fabian,
>
> On 11/24/25 11:16 AM, Fabian Pflug wrote:
> > In verified boot settings, it might not be wanted to have a root=
> > command line parameter for the kernel, because if the initramfs is not
> > configured, or barebox boots the kernel directly due to
> > misconfiguration, the device might boot correctly, but without verified
> > boot, because the kernel will parse root=
> >
> > Allowing to change the parameter name will help circumvent such errors,
> > as there now needs to be multiple misconfigurations for the verified
> > boot to go wrong and the initramfs can use a the different name from the
> > commandline to get the rootfs.
>
> Thanks for your patch!
>
> This is a useful thing to have, but I think deciding replacement of
> root= across all functionality at compile-time is too invasive:
>
> Imagine you use the same barebox both for development and in release
> mode (or you support runtime unlocking with a token).
> When booting securely, you want to use verity_root= for example, but
> when you do boot /mnt/nfs, you'll want barebox to fix up root= as before.
>
> I thus think that this should be a $global.bootm.root_arg evaluated at
> runtime instead.
>
> By adding it to bootm_data_restore_defaults(), its value will then be
> restored after the boot script has done executing.
>
> What do you think?
Do you want to have string-replacement in bootm, or should all of them query
the value for root_arg?
Are you thinking something along the lines of:
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -868,6 +868,11 @@ int bootm_boot(struct bootm_data *bootm_data)
if (IS_ERR(rootarg)) {
pr_err("Failed to append kernel cmdline parameter
'root='\n");
} else {
+ if (bootm_data->prefix_root) {
+ char* rootarg_old = rootarg;
+ rootarg =
xasprintf("%s%s",bootm_data->prefix_root, rootarg+5);
+ free(rootarg_old);
+ }
pr_info("Adding \"%s\" to Kernel commandline\n",
rootarg);
globalvar_add_simple("linux.bootargs.bootm.appendroot",
rootarg);
This could work as a hack, but it does seem error-prone.
Kind regards
Fabian
>
> Cheers,
> Ahmad
>
> >
> > Signed-off-by: Fabian Pflug <[email protected]>
> > ---
> > common/Kconfig | 13 +++++++++++++
> > common/block.c | 3 +--
> > common/bootm.c | 8 ++++----
> > drivers/mci/mci-core.c | 2 +-
> > fs/9p/vfs_super.c | 2 +-
> > fs/nfs.c | 2 +-
> > fs/squashfs/squashfs.c | 2 +-
> > fs/ubifs/ubifs.c | 2 +-
> > 8 files changed, 23 insertions(+), 11 deletions(-)
> >
> > diff --git a/common/Kconfig b/common/Kconfig
> > index d923d4c4b6..62797a9e69 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -1252,6 +1252,19 @@ config SERIAL_NUMBER_FIXUP_SYSTEMD_HOSTNAME
> > This option without effect if global.bootm.provide_hostname
> > is unset.
> >
> > +config ROOT_COMMANDLINE_PARAMETER
> > + string "parameter name for root partition"
> > + default "root"
> > + help
> > + When setting the root partition on the commandline string to the os,
> > use
> > + this name as an indicator of the root partition.
> > +
> > + Changing this could be useful in verified boot contexts to not give
> > the
> > + kernel the chance to boot a non-secure image, but still use the power
> > of
> > + barebox to get the right disk for the rootfs.
> > +
> > + If unsure, leave as default (root)
> > +
> > config SYSTEMD_OF_WATCHDOG
> > bool "inform devicetree-enabled kernel of used watchdog"
> > depends on WATCHDOG && OFTREE && FLEXIBLE_BOOTARGS
> > diff --git a/common/block.c b/common/block.c
> > index ca2ed37dbd..bb8b01c0a0 100644
> > --- a/common/block.c
> > +++ b/common/block.c
> > @@ -601,11 +601,10 @@ char *cdev_get_linux_rootarg(const struct cdev
> > *partcdev)
> > if (blk->ops->get_rootarg)
> > rootarg = blk->ops->get_rootarg(blk, partcdev);
> > if (!rootarg && partcdev->partuuid[0] != 0)
> > - rootarg = basprintf("root=PARTUUID=%s", partcdev->partuuid);
> > + rootarg = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER
> > "=PARTUUID=%s", partcdev->partuuid);
> >
> > if (IS_ENABLED(CONFIG_ROOTWAIT_BOOTARG) && blk->rootwait)
> > rootarg = linux_bootargs_append_rootwait(rootarg);
> >
> > return rootarg;
> > }
> > -
> > diff --git a/common/bootm.c b/common/bootm.c
> > index 17792b2a1d..4549bfeb8b 100644
> > --- a/common/bootm.c
> > +++ b/common/bootm.c
> > @@ -852,10 +852,10 @@ int bootm_boot(struct bootm_data *bootm_data)
> > rootarg = ERR_PTR(-EINVAL);
> >
> > if (!root_cdev)
> > - pr_err("no cdev found for %s, cannot
> > set root= option\n",
> > + pr_err("no cdev found for %s, cannot
> > set "
> > CONFIG_ROOT_COMMANDLINE_PARAMETER "= option\n",
> > root_dev_name);
> > else if (!root_cdev->partuuid[0])
> > - pr_err("%s doesn't have a PARTUUID,
> > cannot set root= option\n",
> > + pr_err("%s doesn't have a PARTUUID,
> > cannot set "
> > CONFIG_ROOT_COMMANDLINE_PARAMETER "= option\n",
> > root_dev_name);
> > }
> >
> > @@ -866,7 +866,7 @@ int bootm_boot(struct bootm_data *bootm_data)
> > }
> >
> > if (IS_ERR(rootarg)) {
> > - pr_err("Failed to append kernel cmdline parameter
> > 'root='\n");
> > + pr_err("Failed to append kernel cmdline parameter '"
> > CONFIG_ROOT_COMMANDLINE_PARAMETER
> > "='\n");
> > } else {
> > pr_info("Adding \"%s\" to Kernel commandline\n",
> > rootarg);
> > globalvar_add_simple("linux.bootargs.bootm.appendroot",
> > @@ -1165,7 +1165,7 @@ BAREBOX_MAGICVAR(global.bootm.dryrun, "bootm default
> > dryrun level");
> > BAREBOX_MAGICVAR(global.bootm.verify, "bootm default verify level");
> > BAREBOX_MAGICVAR(global.bootm.verbose, "bootm default verbosity level
> > (0=quiet)");
> > BAREBOX_MAGICVAR(global.bootm.earlycon, "Add earlycon option to Kernel for
> > early log output");
> > -BAREBOX_MAGICVAR(global.bootm.appendroot, "Add root= option to Kernel to
> > mount rootfs from the device the Kernel
> > comes from (default, device can be overridden via global.bootm.root_dev)");
> > +BAREBOX_MAGICVAR(global.bootm.appendroot, "Add "
> > CONFIG_ROOT_COMMANDLINE_PARAMETER "= option to Kernel to mount
> > rootfs from the device the Kernel comes from (default, device can be
> > overridden via global.bootm.root_dev)");
> > BAREBOX_MAGICVAR(global.bootm.root_dev, "bootm default root device
> > (overrides default device in
> > global.bootm.appendroot)");
> > BAREBOX_MAGICVAR(global.bootm.provide_machine_id, "If true, append
> > systemd.machine_id=$global.machine_id to Kernel
> > command line");
> > BAREBOX_MAGICVAR(global.bootm.provide_hostname, "If true, append
> > systemd.hostname=$global.hostname to Kernel
> > command line");
> > diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> > index 57825bc849..79cebe19f3 100644
> > --- a/drivers/mci/mci-core.c
> > +++ b/drivers/mci/mci-core.c
> > @@ -2691,7 +2691,7 @@ static char *mci_get_linux_mmcblkdev(struct
> > block_device *blk,
> > * skipping it.
> > */
> > if (cdev_partname_equal(partcdev, cdev))
> > - return basprintf("root=/dev/mmcblk%dp%d", id, partnum);
> > + return basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER
> > "=/dev/mmcblk%dp%d", id, partnum);
> > if (cdev->flags & DEVFS_PARTITION_FROM_TABLE)
> > partnum++;
> > }
> > diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
> > index 6a451d9ef5..39a8529a4e 100644
> > --- a/fs/9p/vfs_super.c
> > +++ b/fs/9p/vfs_super.c
> > @@ -70,7 +70,7 @@ static void v9fs_set_rootarg(struct v9fs_session_info
> > *v9ses,
> > trans = v9ses->clnt->trans_mod->name;
> > path = v9ses->aname;
> >
> > - str = basprintf("root=%s rootfstype=9p rootflags=trans=%s,msize=%d,"
> > + str = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER "=%s rootfstype=9p
> > rootflags=trans=%s,msize=%d,"
> > "cache=loose,uname=%s,dfltuid=0,dfltgid=0,aname=%s",
> > tag, trans, v9ses->clnt->msize, tag, path);
> >
> > diff --git a/fs/nfs.c b/fs/nfs.c
> > index 5c2476cd88..9db23c5d18 100644
> > --- a/fs/nfs.c
> > +++ b/fs/nfs.c
> > @@ -1558,7 +1558,7 @@ static void nfs_set_rootarg(struct nfs_priv *npriv,
> > struct fs_device *fsdev)
> > char *str, *tmp;
> > const char *bootargs;
> >
> > - str = basprintf("root=/dev/nfs nfsroot=%pI4:%s%s%s", &npriv->server,
> > npriv->path,
> > + str = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER "=/dev/nfs
> > nfsroot=%pI4:%s%s%s", &npriv->server, npriv-
> > >path,
> > rootnfsopts[0] ? "," : "", rootnfsopts);
> >
> > /* forward specific mount options on demand */
> > diff --git a/fs/squashfs/squashfs.c b/fs/squashfs/squashfs.c
> > index e30372627a..8267e3cba0 100644
> > --- a/fs/squashfs/squashfs.c
> > +++ b/fs/squashfs/squashfs.c
> > @@ -60,7 +60,7 @@ static void squashfs_set_rootarg(struct fs_device *fsdev)
> > ubi_get_device_info(vi.ubi_num, &di);
> > mtd = di.mtd;
> >
> > - str = basprintf("root=/dev/ubiblock%d_%d ubi.mtd=%s ubi.block=%d,%d
> > rootfstype=squashfs",
> > + str = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER "=/dev/ubiblock%d_%d
> > ubi.mtd=%s ubi.block=%d,%d
> > rootfstype=squashfs",
> > vi.ubi_num, vi.vol_id, mtd->cdev.partname, vi.ubi_num,
> > vi.vol_id);
> >
> > fsdev_set_linux_rootarg(fsdev, str);
> > diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
> > index 37986acbb2..34151a0c9f 100644
> > --- a/fs/ubifs/ubifs.c
> > +++ b/fs/ubifs/ubifs.c
> > @@ -442,7 +442,7 @@ static void ubifs_set_rootarg(struct ubifs_priv *priv,
> >
> > mtd = di.mtd;
> >
> > - str = basprintf("root=ubi0:%s ubi.mtd=%s rootfstype=ubifs",
> > + str = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER "=ubi0:%s ubi.mtd=%s
> > rootfstype=ubifs",
> > vi.name, mtd->cdev.partname);
> >
> > fsdev_set_linux_rootarg(fsdev, str);
