From 14982ec7d81f3f90a58faf4f7883c91182bb10e0 Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 14 Sep 2026 11:49:16 -0700 Subject: [PATCH] Fix to keep the FIT DTB's own bootargs unless LINUX_BOOTARGS is explicitly configured --- docs/Targets.md | 23 ++++++++------- docs/compile.md | 10 +++++++ hal/mpfs250.c | 17 +++++------ hal/versal.c | 21 +++----------- hal/zynq.c | 20 +++---------- include/fdt.h | 17 +++++++++++ src/fdt.c | 36 +++++++++++++++++++++++ tools/unit-tests/unit-fdt.c | 58 +++++++++++++++++++++++++++++++++++++ 8 files changed, 148 insertions(+), 54 deletions(-) diff --git a/docs/Targets.md b/docs/Targets.md index 90f645bf00..a8cdeccb36 100644 --- a/docs/Targets.md +++ b/docs/Targets.md @@ -1290,7 +1290,9 @@ power-on. `0x01000000`, which is uncached and coherent for all harts. Cacheable L2-scratchpad memory must not be used for cross-hart signalling (stores can be lost on dirty-line eviction). -**Device-tree fixups** applied to the loaded dtb (`hal/mpfs250.c`): bootargs/root device, +**Device-tree fixups** applied to the loaded dtb (`hal/mpfs250.c`): bootargs/root device +(the DTB's own `/chosen/bootargs` win unless `LINUX_BOOTARGS`/`LINUX_BOOTARGS_ROOT` is set; +see "Linux kernel command line (bootargs)" in `docs/compile.md`), MAC addresses from the device serial number, and all five MSS watchdog nodes are disabled. **Watchdog policy:** the MSS watchdogs always count and reset the chip on timeout (they cannot @@ -4448,7 +4450,10 @@ images { that wolfBoot decompresses straight to the kernel load address at boot. See the [Versal "Booting Linux via FIT image"](#versal-gen-1-vmk180) section for a full walkthrough - the flow is identical apart from the -load addresses and the `bl31`/`fsbl` versus `bl31`/`plm` boot chain. Set +load addresses and the `bl31`/`fsbl` versus `bl31`/`plm` boot chain, +including the bootargs handling (the FIT DTB's own `/chosen/bootargs` +win unless `LINUX_BOOTARGS`/`LINUX_BOOTARGS_ROOT` is set; see +"Linux kernel command line (bootargs)" in `docs/compile.md`). Set `GZIP=0` in `.config` if you want to keep using an uncompressed `Image` plus `compression = "none"`. @@ -5129,21 +5134,17 @@ A stock PetaLinux `image.ub` carries a `ramdisk` sub-image that `bootm` passes t `WOLFBOOT_LOAD_RAMDISK_ADDRESS` defaults to 0, which uses the ramdisk in place inside the staged FIT. Set it to a DDR address clear of the kernel, DTB and staging area if the payload needs a fixed location. -**DTB Fixup for Root Filesystem** +**Kernel Command Line (bootargs)** -wolfBoot automatically modifies the device tree to set the kernel command line (`bootargs`). The default configuration mounts the root filesystem from SD card partition 2: - -``` -earlycon root=/dev/mmcblk0p2 rootwait -``` - -To customize the root device, add to your config: +If the FIT's DTB carries `/chosen/bootargs`, wolfBoot keeps them by default - an image boots with the arguments its kernel was validated with. Setting `LINUX_BOOTARGS` or `LINUX_BOOTARGS_ROOT` in the config replaces the DTB's value (the replaced value is logged); `CFLAGS_EXTRA+=-DLINUX_BOOTARGS_OVERRIDE=0` demotes an explicit `LINUX_BOOTARGS` to a fallback used only when the DTB has none. ```makefile -# Mount root from SD card partition 4 +# Replace the image's bootargs, mounting root from SD card partition 4 CFLAGS_EXTRA+=-DLINUX_BOOTARGS_ROOT=\"/dev/mmcblk0p4\" ``` +On Versal the PS UART console is `ttyAMA0` (PL011); when supplying your own bootargs prefer an explicit `earlycon=pl011,mmio32,0xFF000000,115200n8 console=ttyAMA0,115200`. + **Automated Testing** ```sh diff --git a/docs/compile.md b/docs/compile.md index 92ca052d90..9ea92b63e0 100644 --- a/docs/compile.md +++ b/docs/compile.md @@ -292,6 +292,16 @@ downgrades, compile with `ALLOW_DOWNGRADE=1`. Warning: this option will disable version checking before the updates, thus exposing the system to potential forced downgrade attacks. +### Linux kernel command line (bootargs) + +On the FIT Linux-boot targets whose DTB comes from the signed FIT (Versal, ZynqMP, PolarFire SoC), a DTB that already carries a non-empty `/chosen/bootargs` keeps it by default - the image boots with the arguments its kernel was validated with, and wolfBoot logs `FDT: using DTB bootargs: ...`. Three macros control this: + +- `LINUX_BOOTARGS` - the full command line wolfBoot injects. Setting it (via `CFLAGS_EXTRA+=-DLINUX_BOOTARGS='"..."'`) replaces the DTB's value; the replaced value is logged. +- `LINUX_BOOTARGS_ROOT` - shorthand that only swaps the `root=` device in the HAL's default command line; setting it also selects replace semantics. +- `LINUX_BOOTARGS_OVERRIDE` - explicit polarity control: `1` forces replacement, `0` demotes `LINUX_BOOTARGS` to a fallback used only when the DTB carries no bootargs. Unset, it defaults to `1` when either macro above is configured and `0` otherwise. + +The Raspberry Pi CM4 firmware-DTB path is excluded and always injects wolfBoot's command line, because that DTB is firmware-provided and unverified. Note for raw-DTB targets: an unauthenticated raw DTB's bootargs are honored under the default too, so either sign the DTB (`sign --dts`, see the next section) or set an explicit `LINUX_BOOTARGS` when the DTB partition is not trusted. + ### Require an authenticated device tree (raw-DTB targets) On non-FIT MMU targets that load a raw device tree from flash, wolfBoot authenticates the DTB against the `HDR_DEVICE_TREE_DIGEST` TLV bound to the signed kernel (`sign --dts `, see `docs/Signing.md`). A DTB carrying the digest is always verified; a raw DTB with no digest only warns and boots by default. Compile with `WOLFBOOT_REQUIRE_SIGNED_DTB=1` to make a missing digest a hard failure (fail-closed) once every raw-DTB payload is signed with `--dts`. diff --git a/hal/mpfs250.c b/hal/mpfs250.c index 8f0fd3225d..3bad3aae08 100644 --- a/hal/mpfs250.c +++ b/hal/mpfs250.c @@ -585,6 +585,8 @@ int mpfs_read_serial_number(uint8_t *serial) } /* Linux kernel command line arguments */ +/* Must stay below the fdt.h include: the LINUX_BOOTARGS_OVERRIDE default + * there keys off build-supplied macros only, not this fallback. */ #ifndef LINUX_BOOTARGS #ifndef LINUX_BOOTARGS_ROOT /* wolfBoot SD layout (tools/scripts/program-sdcard.sh): p1=boot FIT, @@ -637,16 +639,11 @@ static int mpfs_dts_fixup_inplace(void* dts_addr, uint32_t capacity) return ret; } - /* Find /chosen node */ - off = fdt_subnode_offset(&ctx, 0, "chosen"); - if (off < 0) { - /* Create /chosen node if it doesn't exist */ - off = fdt_add_subnode(&ctx, 0, "chosen"); - } - - if (off >= 0) { - /* Set bootargs property */ - fdt_fixup_str(&ctx, off, "chosen", "bootargs", LINUX_BOOTARGS); + /* Later fixups (watchdog disable, serial number) must run even if the + * bootargs fixup fails, so log and continue rather than return. */ + ret = fdt_fixup_bootargs(&ctx, LINUX_BOOTARGS, LINUX_BOOTARGS_OVERRIDE); + if (ret < 0) { + wolfBoot_printf("FDT: Failed to set bootargs (%d)\n", ret); } #if defined(MPFS_DDR_INIT) && defined(WOLFBOOT_MMODE_SMODE_BOOT) diff --git a/hal/versal.c b/hal/versal.c index ce0c5ce472..30ba626e0c 100644 --- a/hal/versal.c +++ b/hal/versal.c @@ -64,6 +64,8 @@ */ /* Linux kernel command line arguments */ +/* Must stay below the fdt.h include: the LINUX_BOOTARGS_OVERRIDE default + * there keys off build-supplied macros only, not this fallback. */ #ifndef LINUX_BOOTARGS #ifndef LINUX_BOOTARGS_ROOT /* Default Versal SD layout: rootfs on partition 2. Configurations that use @@ -1290,7 +1292,7 @@ void* hal_get_dts_update_address(void) int hal_dts_fixup(void* dts_addr, uint32_t capacity) { fdt_ctx ctx; - int off, ret; + int ret; /* Validate the blob against the window it actually occupies. */ ret = fdt_open(&ctx, dts_addr, capacity); @@ -1310,22 +1312,7 @@ int hal_dts_fixup(void* dts_addr, uint32_t capacity) return ret; } - /* Find /chosen node; create it only if genuinely missing. Any other - * negative return (malformed FDT, etc.) is surfaced directly rather - * than masked by a follow-on fdt_add_subnode() failure. */ - off = fdt_subnode_offset(&ctx, 0, "chosen"); - if (off == -FDT_ERR_NOTFOUND) { - off = fdt_add_subnode(&ctx, 0, "chosen"); - } - - if (off < 0) { - wolfBoot_printf("FDT: Failed to find/create chosen node (%d)\n", off); - return off; - } - - /* Set bootargs property - overrides the PetaLinux default root= with - * the wolfBoot partition layout. */ - ret = fdt_fixup_str(&ctx, off, "chosen", "bootargs", LINUX_BOOTARGS); + ret = fdt_fixup_bootargs(&ctx, LINUX_BOOTARGS, LINUX_BOOTARGS_OVERRIDE); if (ret < 0) { wolfBoot_printf("FDT: Failed to set bootargs (%d)\n", ret); return ret; diff --git a/hal/zynq.c b/hal/zynq.c index 06c38cfde9..804b764d09 100644 --- a/hal/zynq.c +++ b/hal/zynq.c @@ -64,6 +64,8 @@ * Note: console=ttyPS0 is ZynqMP-specific (PS UART0). Versal's default * (hal/versal.c) omits the console= token because Versal relies on * earlycon alone plus a DT-declared stdout-path. */ +/* Must stay below the fdt.h include: the LINUX_BOOTARGS_OVERRIDE default + * there keys off build-supplied macros only, not this fallback. */ #ifndef LINUX_BOOTARGS #ifndef LINUX_BOOTARGS_ROOT #define LINUX_BOOTARGS_ROOT "/dev/mmcblk0p4" @@ -2632,7 +2634,7 @@ void* hal_get_dts_address(void) int hal_dts_fixup(void* dts_addr, uint32_t capacity) { fdt_ctx ctx; - int off, ret; + int ret; /* Validate the blob against the window it actually occupies. Every * bound the parser applies below comes from `capacity`, not from the @@ -2657,21 +2659,7 @@ int hal_dts_fixup(void* dts_addr, uint32_t capacity) return ret; } - /* Find /chosen node; create it only if genuinely missing. Any other - * negative return (malformed FDT, etc.) is surfaced directly rather - * than masked by a follow-on fdt_add_subnode() failure. */ - off = fdt_subnode_offset(&ctx, 0, "chosen"); - if (off == -FDT_ERR_NOTFOUND) { - off = fdt_add_subnode(&ctx, 0, "chosen"); - } - if (off < 0) { - wolfBoot_printf("FDT: Failed to find/create chosen node (%d)\n", off); - return off; - } - - /* Set bootargs property - overrides PetaLinux default root= with - * the wolfBoot partition layout. */ - ret = fdt_fixup_str(&ctx, off, "chosen", "bootargs", LINUX_BOOTARGS); + ret = fdt_fixup_bootargs(&ctx, LINUX_BOOTARGS, LINUX_BOOTARGS_OVERRIDE); if (ret < 0) { wolfBoot_printf("FDT: Failed to set bootargs (%d)\n", ret); return ret; diff --git a/include/fdt.h b/include/fdt.h index b25646d89d..6d039e332c 100644 --- a/include/fdt.h +++ b/include/fdt.h @@ -314,6 +314,23 @@ int fdt_del_node(fdt_ctx* ctx, int nodeoffset); int fdt_add_mem_rsv(fdt_ctx* ctx, uint64_t address, uint64_t size); /* Logging wrappers around fdt_setprop() used by the HAL dts fixups. */ +/* LINUX_BOOTARGS_OVERRIDE: nonzero replaces the DTB's own bootargs, zero + * keeps them when present (LINUX_BOOTARGS then only fills a missing value). + * Defaults to replace when the build supplies LINUX_BOOTARGS or + * LINUX_BOOTARGS_ROOT, and to keep when only the HAL fallback exists. */ +#ifndef LINUX_BOOTARGS_OVERRIDE + #if defined(LINUX_BOOTARGS) || defined(LINUX_BOOTARGS_ROOT) + #define LINUX_BOOTARGS_OVERRIDE 1 + #else + #define LINUX_BOOTARGS_OVERRIDE 0 + #endif +#endif + +/* Set /chosen bootargs; force 0 keeps an existing non-empty DTB value. + * Evaluated here (before any HAL fallback LINUX_BOOTARGS definition) so + * the default reflects only build-supplied macros. */ +int fdt_fixup_bootargs(fdt_ctx* ctx, const char* args, int force); + int fdt_fixup_str(fdt_ctx* ctx, int off, const char* node, const char* name, const char* str); int fdt_fixup_val(fdt_ctx* ctx, int off, const char* node, const char* name, diff --git a/src/fdt.c b/src/fdt.c index fe08da552b..741a877d67 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -1287,6 +1287,42 @@ int fdt_fixup_val64(fdt_ctx* ctx, int off, const char* node, const char* name, return fdt_setprop(ctx, off, name, &be, (int)sizeof(be)); } +/* Set /chosen bootargs. With force nonzero the DTB's existing value is + * replaced by `args` (and logged); with force 0 an existing non-empty + * value wins, so an image keeps the arguments its kernel was validated + * with unless the build explicitly overrides them. */ +int fdt_fixup_bootargs(fdt_ctx* ctx, const char* args, int force) +{ + const char* old_args; + int off, old_len = 0; + + if (!fdt_ctx_ok(ctx) || args == NULL) { + return -FDT_ERR_BADARG; + } + off = fdt_subnode_offset(ctx, 0, "chosen"); + if (off == -FDT_ERR_NOTFOUND) { + off = fdt_add_subnode(ctx, 0, "chosen"); + } + if (off < 0) { + wolfBoot_printf("FDT: Failed to find/create chosen node (%d)\n", off); + return off; + } + /* Treat the existing value as present only when it is a non-empty, + * NUL-terminated string: property bytes are opaque, and %s printing or + * keeping an unterminated value would be wrong either way. Malformed + * or empty bootargs are simply replaced. */ + old_args = (const char*)fdt_getprop(ctx, off, "bootargs", &old_len); + if (old_args != NULL && old_len > 1 && old_args[0] != '\0' && + memchr(old_args, '\0', (size_t)old_len) != NULL) { + if (!force) { + wolfBoot_printf("FDT: using DTB bootargs: %s\n", old_args); + return 0; + } + wolfBoot_printf("FDT: replacing DTB bootargs: %s\n", old_args); + } + return fdt_fixup_str(ctx, off, "chosen", "bootargs", args); +} + int fdt_fixup_initrd(fdt_ctx* ctx, uint64_t start, uint64_t size) { int off, ret; diff --git a/tools/unit-tests/unit-fdt.c b/tools/unit-tests/unit-fdt.c index 83044cf115..553e0b4d87 100644 --- a/tools/unit-tests/unit-fdt.c +++ b/tools/unit-tests/unit-fdt.c @@ -801,6 +801,63 @@ START_TEST(test_fdt_fixup_initrd) } END_TEST +/* fdt_fixup_bootargs(): DTB-provided bootargs win unless force is set; + * a missing property is always filled in. */ +START_TEST(test_fdt_fixup_bootargs_keep_and_force) +{ + static uint8_t buf[0x800]; + fdt_ctx ctx; + const char *val; + int off, len; + + (void)build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abc\0", 4); + ck_assert_int_eq(fdt_open(&ctx, buf, (uint32_t)sizeof(buf)), 0); + + /* absent: set regardless of force */ + ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "one=1", 0), 0); + off = fdt_subnode_offset(&ctx, 0, "chosen"); + ck_assert_int_gt(off, 0); + val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len); + ck_assert_str_eq(val, "one=1"); + + /* present + force 0: existing value kept */ + ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "two=2", 0), 0); + val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len); + ck_assert_str_eq(val, "one=1"); + + /* present + force 1: replaced */ + ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "two=2", 1), 0); + off = fdt_subnode_offset(&ctx, 0, "chosen"); + val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len); + ck_assert_str_eq(val, "two=2"); + + /* empty (lone NUL) value counts as missing: filled even with force 0 */ + ck_assert_int_eq(fdt_fixup_str(&ctx, off, "chosen", "bootargs", ""), 0); + ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "three=3", 0), 0); + off = fdt_subnode_offset(&ctx, 0, "chosen"); + val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len); + ck_assert_str_eq(val, "three=3"); + + /* NULL args are rejected */ + ck_assert_int_eq(fdt_fixup_bootargs(&ctx, NULL, 0), -FDT_ERR_BADARG); + + /* a value with no NUL inside its declared length is malformed and is + * replaced even with force 0 */ + ck_assert_int_eq(fdt_setprop(&ctx, off, "bootargs", "abc", 3), 0); + ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "four=4", 0), 0); + off = fdt_subnode_offset(&ctx, 0, "chosen"); + val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len); + ck_assert_str_eq(val, "four=4"); + + /* an empty string padded to length 2 is still empty: replaced */ + ck_assert_int_eq(fdt_setprop(&ctx, off, "bootargs", "\0", 2), 0); + ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "five=5", 0), 0); + off = fdt_subnode_offset(&ctx, 0, "chosen"); + val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len); + ck_assert_str_eq(val, "five=5"); +} +END_TEST + /* A start+size that wraps must be rejected: linux,initrd-end would * otherwise precede linux,initrd-start. */ START_TEST(test_fdt_fixup_initrd_rejects_wrapped_end) @@ -948,6 +1005,7 @@ static Suite *fdt_suite(void) tcase_add_test(tc, test_fdt_add_subnode_bounded_by_capacity); tcase_add_test(tc, test_fdt_setprop_resizes_existing_property); tcase_add_test(tc, test_fdt_fixup_initrd); + tcase_add_test(tc, test_fdt_fixup_bootargs_keep_and_force); tcase_add_test(tc, test_fdt_fixup_initrd_rejects_wrapped_end); tcase_add_test(tc, test_fdt_peek_size_header_only); tcase_add_test(tc, test_fit_find_images_rejects_unterminated_image_name);