All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] allow control DTB to double as "FIT image"
@ 2026-05-19 22:54 Rasmus Villemoes
  2026-05-19 22:54 ` [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob Rasmus Villemoes
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Rasmus Villemoes @ 2026-05-19 22:54 UTC (permalink / raw)
  To: u-boot; +Cc: Tom Rini, Simon Glass, Quentin Schulz, Rasmus Villemoes

The commit message for patch 1 explains what it is I'd like to be able
to do, but here's some more background:

For a long time, we've embedded the boot script in the U-Boot binary
by building a bootscript.itb, and using a .dtsi like

  / {
          config {
                 bootscript = /incbin/("/path/to/bootscript.itb");
          };
  };

which in turn is mentioned in CONFIG_DEVICE_TREE_INCLUDES, that
bootscript.itb FIT image has been embedded in U-Boot's control
dtb. Running that was then a matter of doing

  fdt addr ${fdtcontroladdr} && fdt get addr bsaddr /config bootscript && source ${bsaddr}

There are a couple of advantage of having the bootscript (and other
script logic) embedded in the U-Boot binary. First, there's no need to
figure out some separate partition to store the script in, and making
sure that gets updated whenever the bootloader itself does. Second,
one doesn't need to worry about verifying the script; whatever steps
one needs to take to implement secure boot for U-Boot itself will by
necessity also cover the control dtb (if nothing else then because
that's where the public key for the kernel verification lives). And
third, the boot script is automatically updated together with U-Boot
itself; and if U-Boot is stored in an eMMC boot partition, that update
is guaranteed to be atomic.

Now with the stricter requirements of libfdt starting from v2026.04,
the above command no longer worked, or only half the time, because the
embedded FIT image may not land on an 8-byte aligned address. So that
line had to be changed a little (line breaks added)

  fdt addr ${fdtcontroladdr}
    && fdt get addr bsaddr /config bootscript
    && fdt get size bssize /config bootscript
    && cp.b ${bsaddr} ${loadaddr} ${bssize}
    && source ${loadaddr}

which is getting quite unwieldy.

Then it struck me that one could perhaps simplify all of this quite a
lot: Cut out the intermediate bootscript.itb, just create a .dtsi
which directly puts a /images node inside the control dtb

/ {
  	images {
		default = "bootscript";
		bootscript {
			description = "Boot script";
			data = /incbin/("/path/to/bootscript.sh");
			type = "script";
			compression = "none";
		};
	};
};

and treat the control dtb itself as a FIT image; so the command to put
in $bootcmd becomes simply

  source ${fdtcontroladdr}:bootscript

and embedding other pieces of callable scripts is quite trivial.

And that almost works out-of-the-box, except for the fit_check_format() sanity check.

Introduce a CONFIG_ knob that allows one to opt out of those sanity
checks, for the special case of the address being checked being
identical to gd->fdt_blob.

Changes in v2:

- Guard this behind a CONFIG_ option

- Move the exemption logic into fit_check_format()

- Add a section to doc/develop/devicetree/control.rst describing this feature.

- Fix and improve the included tests.

CI was happy with v1, and is mostly through this v2:
https://github.com/u-boot/u-boot/pull/969

Rasmus Villemoes (3):
  image-fit.c: introduce CONTROL_DTB_AS_FIT config knob
  doc: develop: add section on embedding scripts inside control DTB
  test: hook up test of allowing control DTB to act as FIT image

 arch/sandbox/dts/sandbox-boot.sh      |  2 +
 arch/sandbox/dts/sandbox-inner.sh     |  4 ++
 arch/sandbox/dts/sandbox-outer.sh     |  4 ++
 arch/sandbox/dts/sandbox_scripts.dtsi | 24 +++++++++++
 boot/Kconfig                          |  9 +++++
 boot/image-fit.c                      |  5 +++
 configs/sandbox_defconfig             |  2 +
 doc/develop/devicetree/control.rst    | 58 +++++++++++++++++++++++++++
 test/py/tests/test_source.py          | 18 +++++++++
 9 files changed, 126 insertions(+)
 create mode 100644 arch/sandbox/dts/sandbox-boot.sh
 create mode 100644 arch/sandbox/dts/sandbox-inner.sh
 create mode 100644 arch/sandbox/dts/sandbox-outer.sh
 create mode 100644 arch/sandbox/dts/sandbox_scripts.dtsi

-- 
2.54.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob
  2026-05-19 22:54 [PATCH v2 0/3] allow control DTB to double as "FIT image" Rasmus Villemoes
@ 2026-05-19 22:54 ` Rasmus Villemoes
  2026-05-25 15:27   ` Simon Glass
  2026-05-19 22:54 ` [PATCH v2 2/3] doc: develop: add section on embedding scripts inside control DTB Rasmus Villemoes
  2026-05-19 22:54 ` [PATCH v2 3/3] test: hook up test of allowing control DTB to act as FIT image Rasmus Villemoes
  2 siblings, 1 reply; 11+ messages in thread
From: Rasmus Villemoes @ 2026-05-19 22:54 UTC (permalink / raw)
  To: u-boot; +Cc: Tom Rini, Simon Glass, Quentin Schulz, Rasmus Villemoes

Having scripts embedded one way or the other in the U-Boot binary
means they are automatically verified/trusted by whatever mechanism
verifies U-Boot.

Writing those scripts in the built-in environment leads to
backslatitis and missing or wrong quoting and is generally not very
readable or maintainable.

Maintaining scripts in external files allows one
to have both syntax highlighting and to some extent apply shellcheck
on it (though U-Boot's shell is of course not quite POSIX sh, so some
'#shellcheck disable' directives are needed). Getting those into the
U-Boot binary is then a matter of having a suitable .dtsi file such as

/ {
	images {
		default = "boot";
		boot {
			description = "Bootscript";
			data = /incbin/("boot.sh");
			type = "script";
			compression = "none";
		};
		factory-reset {
			description = "Script for performing factory reset";
			data = /incbin/("factory-reset.sh");
			type = "script";
			compression = "none";
		};
	};
};

and making that part of CONFIG_DEVICE_TREE_INCLUDES, so that U-Boot's
control DTB effectively doubles as a FIT image containing a few
"script" entries. At run-time, one's default bootcommand can then
simply be

  source ${fdtcontroladdr}:boot

Except of course that the control DTB is in fact not quite a FIT
image. The lack of timestamp and description properties could
potentially be worked around (by just adding those via that same
.dtsi), but the no-@ check is not possible to get around. But since
the control dtb is by definition trusted, we can make an exception for
that particular address, if the new CONTROL_DTB_AS_FIT config option
is enabled.

One can of course build an ordinary FIT image with those
scripts. However, that requires extra steps in the boot command for
loading that script from storage, requires one to use "configurations"
for pointing at a single script to run, and signing the FIT image
using the same key used for verifying the kernel. Moreover, in certain
situations, such as bootstrapping/production, there is no place to
load that FIT image from, and it is much simpler to just have the
necessary scripts be part of the U-Boot image itself.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 boot/Kconfig     | 9 +++++++++
 boot/image-fit.c | 5 +++++
 2 files changed, 14 insertions(+)

diff --git a/boot/Kconfig b/boot/Kconfig
index ae6f09a6ede..7490afe73b5 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -103,6 +103,15 @@ config FIT_FULL_CHECK
 	  of bugs or omissions in the code. This includes a bad structure,
 	  multiple root nodes and the like.
 
+config CONTROL_DTB_AS_FIT
+	bool "Allow U-Boot's control DTB to act as FIT image"
+	help
+	  Enable this to exempt U-Boot's control DTB from the sanity
+	  checks done to ensure FIT images are valid. This can for
+	  example be used to embed whole scripts in the control DTB,
+	  that can then be invoked using 'source ${fdtcontroladdr}'.
+	  See doc/develop/devicetree/control.rst for details.
+
 config FIT_SIGNATURE
 	bool "Enable signature verification of FIT uImages"
 	depends on DM
diff --git a/boot/image-fit.c b/boot/image-fit.c
index b0fcaf6e17f..a182320b9c6 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1676,6 +1676,10 @@ int fit_check_format(const void *fit, ulong size)
 		return -ENOEXEC;
 	}
 
+	/* For the control DTB to act as a FIT image, we only require an /images node. */
+	if (CONFIG_IS_ENABLED(CONTROL_DTB_AS_FIT) && fit == gd_fdt_blob())
+		goto check_images_node;
+
 	if (CONFIG_IS_ENABLED(FIT_FULL_CHECK)) {
 		/*
 		 * If we are not given the size, make do with calculating it.
@@ -1724,6 +1728,7 @@ int fit_check_format(const void *fit, ulong size)
 	}
 
 	/* mandatory subimages parent '/images' node */
+check_images_node:
 	if (fdt_path_offset(fit, FIT_IMAGES_PATH) < 0) {
 		log_debug("Wrong FIT format: no images parent node\n");
 		return -ENOENT;
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] doc: develop: add section on embedding scripts inside control DTB
  2026-05-19 22:54 [PATCH v2 0/3] allow control DTB to double as "FIT image" Rasmus Villemoes
  2026-05-19 22:54 ` [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob Rasmus Villemoes
@ 2026-05-19 22:54 ` Rasmus Villemoes
  2026-05-25 15:27   ` Simon Glass
  2026-05-19 22:54 ` [PATCH v2 3/3] test: hook up test of allowing control DTB to act as FIT image Rasmus Villemoes
  2 siblings, 1 reply; 11+ messages in thread
From: Rasmus Villemoes @ 2026-05-19 22:54 UTC (permalink / raw)
  To: u-boot; +Cc: Tom Rini, Simon Glass, Quentin Schulz, Rasmus Villemoes

Add a section that explains how one can embed scripts in the control
DTB and run them from the U-Boot shell, the advantages of doing that
compared to using a separately built FIT image, and the configuration
knob that must be turned on to allow this to work.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 doc/develop/devicetree/control.rst | 58 ++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst
index 634114af59a..7d6117d5c4b 100644
--- a/doc/develop/devicetree/control.rst
+++ b/doc/develop/devicetree/control.rst
@@ -232,6 +232,64 @@ outside the U-Boot repository. You can use `DEVICE_TREE_INCLUDES` Kconfig
 option to specify a list of .dtsi files that will also be included when
 building .dtb files.
 
+Scripts embedded in control DTB
+-------------------------------
+
+The `DEVICE_TREE_INCLUDES` option can also be used to make the control
+DTB serve double duty as a FIT image. By including a `scripts.dtsi`
+file containing something like::
+
+  / {
+	images {
+		default = "boot";
+		boot {
+			description = "Bootscript";
+			data = /incbin/("boot.sh");
+			type = "script";
+			compression = "none";
+		};
+		factory-reset {
+			description = "Script for performing factory reset";
+			data = /incbin/("factory-reset.sh");
+			type = "script";
+			compression = "none";
+		};
+	};
+  };
+
+one can call those scripts using the `source` command in the U-Boot shell::
+
+  source ${fdtcontroladdr}:boot
+
+or just ``source ${fdtcontroladdr}`` for invoking the default.
+
+Since one does not need to separately build a "real" FIT image
+containing those scripts, this simplifies both the build process and
+the boot logic, as the latter does not need to first load the FIT
+image from storage.
+
+Another advantage is that when the bootloader and boot script must be
+updated together, it is easier to achieve a guaranteed atomic update
+when the boot script is embedded inside the U-Boot binary, instead of
+stored separately.
+
+For the above to work, one must enable the `CONTROL_DTB_AS_FIT` config
+option, which will (when the address passed to the `source` command is
+the address of U-Boot's control DTB) elide certain sanity checks that
+are normally done: With the above `.dtsi` snippet, the control DTB
+does not quite become a "real" FIT image - it lacks `timestamp` and
+`description` properties, but more importantly, FIT images cannot
+contain nodes with `@` in their names (unit addresses) anywhere, and
+the control DTB obviously does have such nodes.
+
+This is not a security problem, as the control DTB is necessarily
+trusted. In any secure boot setup where the bootloader is verified,
+that mechanism must also include verification of the control DTB. So
+in fact, since the scripts embedded this way are then also
+automatically verified, it simplifies implementation of secure
+boot. When using a separate FIT image, one must build it with
+appropriate signatures, just as when building a FIT image containing a
+kernel/dtb/initramfs.
 
 Devicetree bindings schema checks
 ---------------------------------
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] test: hook up test of allowing control DTB to act as FIT image
  2026-05-19 22:54 [PATCH v2 0/3] allow control DTB to double as "FIT image" Rasmus Villemoes
  2026-05-19 22:54 ` [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob Rasmus Villemoes
  2026-05-19 22:54 ` [PATCH v2 2/3] doc: develop: add section on embedding scripts inside control DTB Rasmus Villemoes
@ 2026-05-19 22:54 ` Rasmus Villemoes
  2026-05-25 15:28   ` Simon Glass
  2 siblings, 1 reply; 11+ messages in thread
From: Rasmus Villemoes @ 2026-05-19 22:54 UTC (permalink / raw)
  To: u-boot; +Cc: Tom Rini, Simon Glass, Quentin Schulz, Rasmus Villemoes

Add a test demonstrating how one can embed various scripts in the
control DTB.

Verify that the source command can be used with ${fdtcontroladdr} by
itself (invoking the default script), and with :<node-name>
suffix. Also check that the scripts themselves can invoke "sibling"
scripts.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/sandbox/dts/sandbox-boot.sh      |  2 ++
 arch/sandbox/dts/sandbox-inner.sh     |  4 ++++
 arch/sandbox/dts/sandbox-outer.sh     |  4 ++++
 arch/sandbox/dts/sandbox_scripts.dtsi | 24 ++++++++++++++++++++++++
 configs/sandbox_defconfig             |  2 ++
 test/py/tests/test_source.py          | 18 ++++++++++++++++++
 6 files changed, 54 insertions(+)
 create mode 100644 arch/sandbox/dts/sandbox-boot.sh
 create mode 100644 arch/sandbox/dts/sandbox-inner.sh
 create mode 100644 arch/sandbox/dts/sandbox-outer.sh
 create mode 100644 arch/sandbox/dts/sandbox_scripts.dtsi

diff --git a/arch/sandbox/dts/sandbox-boot.sh b/arch/sandbox/dts/sandbox-boot.sh
new file mode 100644
index 00000000000..4f7fa661151
--- /dev/null
+++ b/arch/sandbox/dts/sandbox-boot.sh
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+echo "* default script"
diff --git a/arch/sandbox/dts/sandbox-inner.sh b/arch/sandbox/dts/sandbox-inner.sh
new file mode 100644
index 00000000000..b8fc8f7484b
--- /dev/null
+++ b/arch/sandbox/dts/sandbox-inner.sh
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+# Some comment.
+echo "* inner"
diff --git a/arch/sandbox/dts/sandbox-outer.sh b/arch/sandbox/dts/sandbox-outer.sh
new file mode 100644
index 00000000000..40294085433
--- /dev/null
+++ b/arch/sandbox/dts/sandbox-outer.sh
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+echo "* outer 1"
+source ${fdtcontroladdr}:inner
+echo "* outer 2"
diff --git a/arch/sandbox/dts/sandbox_scripts.dtsi b/arch/sandbox/dts/sandbox_scripts.dtsi
new file mode 100644
index 00000000000..c800ec39e87
--- /dev/null
+++ b/arch/sandbox/dts/sandbox_scripts.dtsi
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/ {
+	images {
+		default = "boot";
+		boot {
+			description = "Test boot script";
+			data = /incbin/("sandbox-boot.sh");
+			type = "script";
+			compression = "none";
+		};
+		outer {
+			description = "Script testing recursion";
+			data = /incbin/("sandbox-outer.sh");
+			type = "script";
+			compression = "none";
+		};
+		inner {
+			description = "Another test script";
+			data = /incbin/("sandbox-inner.sh");
+			type = "script";
+			compression = "none";
+		};
+	};
+};
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index ba800f7d19d..bfdc5ee6010 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -20,6 +20,7 @@ CONFIG_EFI_CAPSULE_AUTHENTICATE=y
 CONFIG_EFI_CAPSULE_CRT_FILE="board/sandbox/capsule_pub_key_good.crt"
 CONFIG_BUTTON_CMD=y
 CONFIG_FIT=y
+CONFIG_CONTROL_DTB_AS_FIT=y
 CONFIG_FIT_SIGNATURE=y
 CONFIG_FIT_CIPHER=y
 CONFIG_FIT_VERBOSE=y
@@ -152,6 +153,7 @@ CONFIG_CMD_STACKPROTECTOR_TEST=y
 CONFIG_CMD_SPAWN=y
 CONFIG_MAC_PARTITION=y
 CONFIG_OF_LIVE=y
+CONFIG_DEVICE_TREE_INCLUDES="sandbox_scripts.dtsi"
 CONFIG_ENV_IS_NOWHERE=y
 CONFIG_ENV_IS_IN_FAT=y
 CONFIG_ENV_IS_IN_EXT4=y
diff --git a/test/py/tests/test_source.py b/test/py/tests/test_source.py
index 970d8c79869..785e6fb935b 100644
--- a/test/py/tests/test_source.py
+++ b/test/py/tests/test_source.py
@@ -9,6 +9,7 @@ import utils
 @pytest.mark.buildconfigspec('cmd_echo')
 @pytest.mark.buildconfigspec('cmd_source')
 @pytest.mark.buildconfigspec('fit')
+@pytest.mark.buildconfigspec('control_dtb_as_fit')
 def test_source(ubman):
     # Compile our test script image
     mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage')
@@ -34,3 +35,20 @@ def test_source(ubman):
     ubman.run_command('fdt rm /images default')
     assert 'Fail' in ubman.run_command('source || echo Fail')
     assert 'Fail' in ubman.run_command('source \\# || echo Fail')
+
+    output = ubman.run_command('source ${fdtcontroladdr}')
+    assert '* default script' in output
+
+    output = ubman.run_command('source ${fdtcontroladdr}:boot')
+    assert '* default script' in output
+
+    output = ubman.run_command('source ${fdtcontroladdr}:outer')
+    assert '* outer 1' in output
+    assert '* inner' in output
+    assert '* outer 2' in output
+
+    output = ubman.run_command('source ${fdtcontroladdr}:inner')
+    assert '* outer' not in output
+    assert '* inner' in output
+
+    assert 'Fail' in ubman.run_command('source ${fdtcontroladdr}:no-such-script || echo Fail')
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob
  2026-05-19 22:54 ` [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob Rasmus Villemoes
@ 2026-05-25 15:27   ` Simon Glass
  2026-05-26 21:12     ` Rasmus Villemoes
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2026-05-25 15:27 UTC (permalink / raw)
  To: ravi; +Cc: u-boot, Tom Rini, Simon Glass, Quentin Schulz

Hi Rasmus,

On 2026-05-19T22:54:57, Rasmus Villemoes <ravi@prevas.dk> wrote:
> image-fit.c: introduce CONTROL_DTB_AS_FIT config knob
>
> Having scripts embedded one way or the other in the U-Boot binary
> means they are automatically verified/trusted by whatever mechanism
> verifies U-Boot.
>
> Writing those scripts in the built-in environment leads to
> backslatitis and missing or wrong quoting and is generally not very
> readable or maintainable.
>
> Maintaining scripts in external files allows one
> to have both syntax highlighting and to some extent apply shellcheck
> on it (though U-Boot's shell is of course not quite POSIX sh, so some
> '#shellcheck disable' directives are needed). Getting those into the
> U-Boot binary is then a matter of having a suitable .dtsi file such as
>
> / {
>         images {
>                 default = 'boot';
>                 boot {
> [...]
>
> boot/Kconfig     | 9 +++++++++
>  boot/image-fit.c | 5 +++++
>  2 files changed, 14 insertions(+)

> diff --git a/boot/image-fit.c b/boot/image-fit.c
> @@ -1676,6 +1676,10 @@ int fit_check_format(const void *fit, ulong size)
>               return -ENOEXEC;
>       }
>
> +     /* For the control DTB to act as a FIT image, we only require an /images node. */
> +     if (CONFIG_IS_ENABLED(CONTROL_DTB_AS_FIT) && fit == gd_fdt_blob())
> +             goto check_images_node;
> +

I wonder if you could avoid the goto by using a bool? E.g.

   /* control DTB is trusted */
   bool as_control = CONFIG_IS_ENABLED(CONTROL_DTB_AS_FIT) &&
                     fit == gd_fdt_blob();

   if (!as_control && CONFIG_IS_ENABLED(FIT_FULL_CHECK)) {
           ...
   }
  ...

> diff --git a/boot/Kconfig b/boot/Kconfig
> @@ -103,6 +103,15 @@ config FIT_FULL_CHECK
> +config CONTROL_DTB_AS_FIT
> +     bool "Allow U-Boot's control DTB to act as FIT image"
> +     help
> +       Enable this to exempt U-Boot's control DTB from the sanity
> +       checks done to ensure FIT images are valid. This can for
> +       example be used to embed whole scripts in the control DTB,
> +       that can then be invoked using 'source ${fdtcontroladdr}'.
> +       See doc/develop/devicetree/control.rst for details.

Please note in the help that this is safe because the control DTB is
necessarily trusted (any verification covering U-Boot also covers it),
and that only the address matching gd->fdt_blob is exempted - not
arbitrary FIT loads.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] doc: develop: add section on embedding scripts inside control DTB
  2026-05-19 22:54 ` [PATCH v2 2/3] doc: develop: add section on embedding scripts inside control DTB Rasmus Villemoes
@ 2026-05-25 15:27   ` Simon Glass
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Glass @ 2026-05-25 15:27 UTC (permalink / raw)
  To: ravi; +Cc: u-boot, Tom Rini, Simon Glass, Quentin Schulz

On 2026-05-19T22:54:57, Rasmus Villemoes <ravi@prevas.dk> wrote:
> doc: develop: add section on embedding scripts inside control DTB
>
> Add a section that explains how one can embed scripts in the control
> DTB and run them from the U-Boot shell, the advantages of doing that
> compared to using a separately built FIT image, and the configuration
> knob that must be turned on to allow this to work.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>
> doc/develop/devicetree/control.rst | 58 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] test: hook up test of allowing control DTB to act as FIT image
  2026-05-19 22:54 ` [PATCH v2 3/3] test: hook up test of allowing control DTB to act as FIT image Rasmus Villemoes
@ 2026-05-25 15:28   ` Simon Glass
  2026-05-26 21:18     ` Rasmus Villemoes
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2026-05-25 15:28 UTC (permalink / raw)
  To: ravi; +Cc: u-boot, Tom Rini, Simon Glass, Quentin Schulz

Hi Rasmus,

On 2026-05-19T22:54:57, Rasmus Villemoes <ravi@prevas.dk> wrote:
> test: hook up test of allowing control DTB to act as FIT image
>
> Add a test demonstrating how one can embed various scripts in the
> control DTB.
>
> Verify that the source command can be used with ${fdtcontroladdr} by
> itself (invoking the default script), and with :<node-name>
> suffix. Also check that the scripts themselves can invoke 'sibling'
> scripts.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>
> arch/sandbox/dts/sandbox-boot.sh      |  2 ++
>  arch/sandbox/dts/sandbox-inner.sh     |  4 ++++
>  arch/sandbox/dts/sandbox-outer.sh     |  4 ++++
>  arch/sandbox/dts/sandbox_scripts.dtsi | 24 ++++++++++++++++++++++++
>  configs/sandbox_defconfig             |  2 ++
>  test/py/tests/test_source.py          | 18 ++++++++++++++++++
>  6 files changed, 54 insertions(+)

> diff --git a/test/py/tests/test_source.py b/test/py/tests/test_source.py
> @@ -9,6 +9,7 @@ import utils
>  @pytest.mark.buildconfigspec('cmd_echo')
>  @pytest.mark.buildconfigspec('cmd_source')
>  @pytest.mark.buildconfigspec('fit')
> +@pytest.mark.buildconfigspec('control_dtb_as_fit')
>  def test_source(ubman):

buildconfigspec markers are ANDed in conftest.py, so the entire
pre-existing test_source is now skipped on any sandbox build that has
FIT but not CONTROL_DTB_AS_FIT - so silently losing coverage of the
original source tests.

Please split the new assertions into a second function, say
test_source_control_dtb, with the extra marker on its own. That also
makes the new functionality independently selectable.

Regards,
Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob
  2026-05-25 15:27   ` Simon Glass
@ 2026-05-26 21:12     ` Rasmus Villemoes
  2026-05-27  4:09       ` Simon Glass
  0 siblings, 1 reply; 11+ messages in thread
From: Rasmus Villemoes @ 2026-05-26 21:12 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot, Tom Rini, Quentin Schulz

On Mon, May 25 2026, Simon Glass <sjg@chromium.org> wrote:

> Hi Rasmus,
>
> On 2026-05-19T22:54:57, Rasmus Villemoes <ravi@prevas.dk> wrote:
>> image-fit.c: introduce CONTROL_DTB_AS_FIT config knob
>>
>> Having scripts embedded one way or the other in the U-Boot binary
>> means they are automatically verified/trusted by whatever mechanism
>> verifies U-Boot.
>>
>> Writing those scripts in the built-in environment leads to
>> backslatitis and missing or wrong quoting and is generally not very
>> readable or maintainable.
>>
>> Maintaining scripts in external files allows one
>> to have both syntax highlighting and to some extent apply shellcheck
>> on it (though U-Boot's shell is of course not quite POSIX sh, so some
>> '#shellcheck disable' directives are needed). Getting those into the
>> U-Boot binary is then a matter of having a suitable .dtsi file such as
>>
>> / {
>>         images {
>>                 default = 'boot';
>>                 boot {
>> [...]
>>
>> boot/Kconfig     | 9 +++++++++
>>  boot/image-fit.c | 5 +++++
>>  2 files changed, 14 insertions(+)
>
>> diff --git a/boot/image-fit.c b/boot/image-fit.c
>> @@ -1676,6 +1676,10 @@ int fit_check_format(const void *fit, ulong size)
>>               return -ENOEXEC;
>>       }
>>
>> +     /* For the control DTB to act as a FIT image, we only require an /images node. */
>> +     if (CONFIG_IS_ENABLED(CONTROL_DTB_AS_FIT) && fit == gd_fdt_blob())
>> +             goto check_images_node;
>> +
>
> I wonder if you could avoid the goto by using a bool? E.g.
>
>    /* control DTB is trusted */
>    bool as_control = CONFIG_IS_ENABLED(CONTROL_DTB_AS_FIT) &&
>                      fit == gd_fdt_blob();
>
>    if (!as_control && CONFIG_IS_ENABLED(FIT_FULL_CHECK)) {
>            ...
>    }
>   ...

Not really. I mean, sure, I could avoid the goto, but it's not just the
FIT_FULL_CHECK I want to skip, it is also the 'description' and
'timestamp' checks, so using that bool I'd have to modify those if
statements as well. And I think the goto is actually the cleanest
approach.

The reason I didn't lift the 'check for an /images node' up and inserted
the 'fit == gd_fdt_blob()' after, doing an early return, is that I think
in the general case, the FIT_FULL_CHECK doing the basic sanity checks of
the dtb structure itself must be done before we start asking questions
about which nodes or properties it has.

So I did go back and forth a little, but in the end I felt that this was
the cleanest and most focused addition.


>> diff --git a/boot/Kconfig b/boot/Kconfig
>> @@ -103,6 +103,15 @@ config FIT_FULL_CHECK
>> +config CONTROL_DTB_AS_FIT
>> +     bool "Allow U-Boot's control DTB to act as FIT image"
>> +     help
>> +       Enable this to exempt U-Boot's control DTB from the sanity
>> +       checks done to ensure FIT images are valid. This can for
>> +       example be used to embed whole scripts in the control DTB,
>> +       that can then be invoked using 'source ${fdtcontroladdr}'.
>> +       See doc/develop/devicetree/control.rst for details.
>
> Please note in the help that this is safe because the control DTB is
> necessarily trusted (any verification covering U-Boot also covers it),
> and that only the address matching gd->fdt_blob is exempted - not
> arbitrary FIT loads.

OK. Something like

       Enable this to exempt U-Boot's control DTB from the sanity checks
       done to ensure FIT images are valid. This can for example be used
       to embed whole scripts in the control DTB, that can then be
       invoked using 'source ${fdtcontroladdr}'. In a secure boot setup,
       this is safe, as the control DTB is necessarily covered by any
       mechanism verifying U-Boot and can therefore be trusted. This
       only affects the case where the image being checked is
       gd->fdt_blob. See doc/develop/devicetree/control.rst for details.

Rasmus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] test: hook up test of allowing control DTB to act as FIT image
  2026-05-25 15:28   ` Simon Glass
@ 2026-05-26 21:18     ` Rasmus Villemoes
  2026-05-27  4:41       ` Simon Glass
  0 siblings, 1 reply; 11+ messages in thread
From: Rasmus Villemoes @ 2026-05-26 21:18 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot, Tom Rini, Quentin Schulz

On Mon, May 25 2026, Simon Glass <sjg@chromium.org> wrote:

> Hi Rasmus,
>
> On 2026-05-19T22:54:57, Rasmus Villemoes <ravi@prevas.dk> wrote:
>> test: hook up test of allowing control DTB to act as FIT image
>>
>> Add a test demonstrating how one can embed various scripts in the
>> control DTB.
>>
>> Verify that the source command can be used with ${fdtcontroladdr} by
>> itself (invoking the default script), and with :<node-name>
>> suffix. Also check that the scripts themselves can invoke 'sibling'
>> scripts.
>>
>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>>
>> arch/sandbox/dts/sandbox-boot.sh      |  2 ++
>>  arch/sandbox/dts/sandbox-inner.sh     |  4 ++++
>>  arch/sandbox/dts/sandbox-outer.sh     |  4 ++++
>>  arch/sandbox/dts/sandbox_scripts.dtsi | 24 ++++++++++++++++++++++++
>>  configs/sandbox_defconfig             |  2 ++
>>  test/py/tests/test_source.py          | 18 ++++++++++++++++++
>>  6 files changed, 54 insertions(+)
>
>> diff --git a/test/py/tests/test_source.py b/test/py/tests/test_source.py
>> @@ -9,6 +9,7 @@ import utils
>>  @pytest.mark.buildconfigspec('cmd_echo')
>>  @pytest.mark.buildconfigspec('cmd_source')
>>  @pytest.mark.buildconfigspec('fit')
>> +@pytest.mark.buildconfigspec('control_dtb_as_fit')
>>  def test_source(ubman):
>
> buildconfigspec markers are ANDed in conftest.py, so the entire
> pre-existing test_source is now skipped on any sandbox build that has
> FIT but not CONTROL_DTB_AS_FIT - so silently losing coverage of the
> original source tests.
>
> Please split the new assertions into a second function, say
> test_source_control_dtb, with the extra marker on its own. That also
> makes the new functionality independently selectable.

Yes, I did consider doing that, and I must admit I'm not really sure why
I ended up not doing it. Will redo.

Is it worth it adding another function marked with
@pytest.mark.notbuildconfigspec('control_dtb_as_fit') and checking that
'source ${fdtcontroladdr}' always fails in that case?

Rasmus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob
  2026-05-26 21:12     ` Rasmus Villemoes
@ 2026-05-27  4:09       ` Simon Glass
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Glass @ 2026-05-27  4:09 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: u-boot, Tom Rini, Quentin Schulz

Hi Rasmus,

On Tue, 26 May 2026 at 23:12, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On Mon, May 25 2026, Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Rasmus,
> >
> > On 2026-05-19T22:54:57, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >> image-fit.c: introduce CONTROL_DTB_AS_FIT config knob
> >>
> >> Having scripts embedded one way or the other in the U-Boot binary
> >> means they are automatically verified/trusted by whatever mechanism
> >> verifies U-Boot.
> >>
> >> Writing those scripts in the built-in environment leads to
> >> backslatitis and missing or wrong quoting and is generally not very
> >> readable or maintainable.
> >>
> >> Maintaining scripts in external files allows one
> >> to have both syntax highlighting and to some extent apply shellcheck
> >> on it (though U-Boot's shell is of course not quite POSIX sh, so some
> >> '#shellcheck disable' directives are needed). Getting those into the
> >> U-Boot binary is then a matter of having a suitable .dtsi file such as
> >>
> >> / {
> >>         images {
> >>                 default = 'boot';
> >>                 boot {
> >> [...]
> >>
> >> boot/Kconfig     | 9 +++++++++
> >>  boot/image-fit.c | 5 +++++
> >>  2 files changed, 14 insertions(+)
> >
> >> diff --git a/boot/image-fit.c b/boot/image-fit.c
> >> @@ -1676,6 +1676,10 @@ int fit_check_format(const void *fit, ulong size)
> >>               return -ENOEXEC;
> >>       }
> >>
> >> +     /* For the control DTB to act as a FIT image, we only require an /images node. */
> >> +     if (CONFIG_IS_ENABLED(CONTROL_DTB_AS_FIT) && fit == gd_fdt_blob())
> >> +             goto check_images_node;
> >> +
> >
> > I wonder if you could avoid the goto by using a bool? E.g.
> >
> >    /* control DTB is trusted */
> >    bool as_control = CONFIG_IS_ENABLED(CONTROL_DTB_AS_FIT) &&
> >                      fit == gd_fdt_blob();
> >
> >    if (!as_control && CONFIG_IS_ENABLED(FIT_FULL_CHECK)) {
> >            ...
> >    }
> >   ...
>
> Not really. I mean, sure, I could avoid the goto, but it's not just the
> FIT_FULL_CHECK I want to skip, it is also the 'description' and
> 'timestamp' checks, so using that bool I'd have to modify those if
> statements as well. And I think the goto is actually the cleanest
> approach.
>
> The reason I didn't lift the 'check for an /images node' up and inserted
> the 'fit == gd_fdt_blob()' after, doing an early return, is that I think
> in the general case, the FIT_FULL_CHECK doing the basic sanity checks of
> the dtb structure itself must be done before we start asking questions
> about which nodes or properties it has.
>
> So I did go back and forth a little, but in the end I felt that this was
> the cleanest and most focused addition.

OK I really cannot countenance a 'goto' in the FIT code, one of the
foundations of verified boot. It's OK for error handling, but this is
no' that.

So how about splitting out the middle of fit_check_format() into a new
function that you call conditionally?

>
>
> >> diff --git a/boot/Kconfig b/boot/Kconfig
> >> @@ -103,6 +103,15 @@ config FIT_FULL_CHECK
> >> +config CONTROL_DTB_AS_FIT
> >> +     bool "Allow U-Boot's control DTB to act as FIT image"
> >> +     help
> >> +       Enable this to exempt U-Boot's control DTB from the sanity
> >> +       checks done to ensure FIT images are valid. This can for
> >> +       example be used to embed whole scripts in the control DTB,
> >> +       that can then be invoked using 'source ${fdtcontroladdr}'.
> >> +       See doc/develop/devicetree/control.rst for details.
> >
> > Please note in the help that this is safe because the control DTB is
> > necessarily trusted (any verification covering U-Boot also covers it),
> > and that only the address matching gd->fdt_blob is exempted - not
> > arbitrary FIT loads.
>
> OK. Something like
>
>        Enable this to exempt U-Boot's control DTB from the sanity checks
>        done to ensure FIT images are valid. This can for example be used
>        to embed whole scripts in the control DTB, that can then be
>        invoked using 'source ${fdtcontroladdr}'. In a secure boot setup,
>        this is safe, as the control DTB is necessarily covered by any
>        mechanism verifying U-Boot and can therefore be trusted. This
>        only affects the case where the image being checked is
>        gd->fdt_blob. See doc/develop/devicetree/control.rst for details.
>

Perfect, thanks

Regards,
Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] test: hook up test of allowing control DTB to act as FIT image
  2026-05-26 21:18     ` Rasmus Villemoes
@ 2026-05-27  4:41       ` Simon Glass
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Glass @ 2026-05-27  4:41 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: u-boot, Tom Rini, Quentin Schulz

Hi Rasmus,

On Tue, 26 May 2026 at 15:18, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On Mon, May 25 2026, Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Rasmus,
> >
> > On 2026-05-19T22:54:57, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >> test: hook up test of allowing control DTB to act as FIT image
> >>
> >> Add a test demonstrating how one can embed various scripts in the
> >> control DTB.
> >>
> >> Verify that the source command can be used with ${fdtcontroladdr} by
> >> itself (invoking the default script), and with :<node-name>
> >> suffix. Also check that the scripts themselves can invoke 'sibling'
> >> scripts.
> >>
> >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> >>
> >> arch/sandbox/dts/sandbox-boot.sh      |  2 ++
> >>  arch/sandbox/dts/sandbox-inner.sh     |  4 ++++
> >>  arch/sandbox/dts/sandbox-outer.sh     |  4 ++++
> >>  arch/sandbox/dts/sandbox_scripts.dtsi | 24 ++++++++++++++++++++++++
> >>  configs/sandbox_defconfig             |  2 ++
> >>  test/py/tests/test_source.py          | 18 ++++++++++++++++++
> >>  6 files changed, 54 insertions(+)
> >
> >> diff --git a/test/py/tests/test_source.py b/test/py/tests/test_source.py
> >> @@ -9,6 +9,7 @@ import utils
> >>  @pytest.mark.buildconfigspec('cmd_echo')
> >>  @pytest.mark.buildconfigspec('cmd_source')
> >>  @pytest.mark.buildconfigspec('fit')
> >> +@pytest.mark.buildconfigspec('control_dtb_as_fit')
> >>  def test_source(ubman):
> >
> > buildconfigspec markers are ANDed in conftest.py, so the entire
> > pre-existing test_source is now skipped on any sandbox build that has
> > FIT but not CONTROL_DTB_AS_FIT - so silently losing coverage of the
> > original source tests.
> >
> > Please split the new assertions into a second function, say
> > test_source_control_dtb, with the extra marker on its own. That also
> > makes the new functionality independently selectable.
>
> Yes, I did consider doing that, and I must admit I'm not really sure why
> I ended up not doing it. Will redo.
>
> Is it worth it adding another function marked with
> @pytest.mark.notbuildconfigspec('control_dtb_as_fit') and checking that
> 'source ${fdtcontroladdr}' always fails in that case?

The seems like a good idea to me.

Regards,
Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-05-27  4:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19 22:54 [PATCH v2 0/3] allow control DTB to double as "FIT image" Rasmus Villemoes
2026-05-19 22:54 ` [PATCH v2 1/3] image-fit.c: introduce CONTROL_DTB_AS_FIT config knob Rasmus Villemoes
2026-05-25 15:27   ` Simon Glass
2026-05-26 21:12     ` Rasmus Villemoes
2026-05-27  4:09       ` Simon Glass
2026-05-19 22:54 ` [PATCH v2 2/3] doc: develop: add section on embedding scripts inside control DTB Rasmus Villemoes
2026-05-25 15:27   ` Simon Glass
2026-05-19 22:54 ` [PATCH v2 3/3] test: hook up test of allowing control DTB to act as FIT image Rasmus Villemoes
2026-05-25 15:28   ` Simon Glass
2026-05-26 21:18     ` Rasmus Villemoes
2026-05-27  4:41       ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.