From 21705d39144a51a50c0206c74588c6e9e4018e7a Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Tue, 10 Jun 2025 07:41:17 +0000 Subject: [PATCH 1/5] tools: mkimage: propagate error codes from fit_handle_file() The fit_handle_file() function previously returned a hardcoded -1 on error. This change updates the logic to return the actual error code stored in `ret`, allowing for error propagation. This improves debuggability and enables downstream callers to distinguish different failure causes, such as FDT_ERR_NOTFOUND or other errors. Signed-off-by: Aristo Chen --- tools/fit_image.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/fit_image.c b/tools/fit_image.c index caed8d5f901..3d2fbad3853 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -750,7 +750,7 @@ static int fit_handle_file(struct image_tool_params *params) char bakfile[MKIMAGE_MAX_TMPFILE_LEN + 4] = {0}; char cmd[MKIMAGE_MAX_DTC_CMDLINE_LEN]; size_t size_inc; - int ret; + int ret = EXIT_FAILURE; /* Flattened Image Tree (FIT) format handling */ debug ("FIT format handling\n"); @@ -854,7 +854,7 @@ static int fit_handle_file(struct image_tool_params *params) err_system: unlink(tmpfile); unlink(bakfile); - return -1; + return ret; } /** From 7a8b25a771f069819eab6134fd1c4df520ebc9ca Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Tue, 10 Jun 2025 07:41:18 +0000 Subject: [PATCH 2/5] tools: mkimage: validate image references in FIT configurations When parsing a FIT image source (ITS), mkimage does not currently check whether the image names referenced in the /configurations section (e.g. "kernel", "fdt", "ramdisk", "loadables") actually exist in the /images node. This patch introduces a validation step during FIT import that iterates over each configuration and verifies that all referenced image names are defined under /images. If a missing image is detected, an appropriate error is reported and mkimage exits with FDT_ERR_NOTFOUND. This ensures that configuration integrity is validated at build time. Signed-off-by: Aristo Chen --- tools/fit_image.c | 38 ++++++++++++++++++++++++++++++++++++++ tools/mkimage.c | 7 ++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/tools/fit_image.c b/tools/fit_image.c index 3d2fbad3853..8fdde012bda 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -627,6 +627,7 @@ static int fit_import_data(struct image_tool_params *params, const char *fname) struct stat sbuf; int ret; int images; + int confs; int node; fd = mmap_fdt(params->cmdname, fname, 0, &old_fdt, &sbuf, false, false); @@ -695,6 +696,43 @@ static int fit_import_data(struct image_tool_params *params, const char *fname) } } + confs = fdt_path_offset(fdt, FIT_CONFS_PATH); + static const char * const props[] = { FIT_KERNEL_PROP, + FIT_RAMDISK_PROP, + FIT_FDT_PROP, + FIT_LOADABLE_PROP, + FIT_FPGA_PROP, + FIT_FIRMWARE_PROP, + FIT_SCRIPT_PROP}; + + fdt_for_each_subnode(node, fdt, confs) { + const char *conf_name = fdt_get_name(fdt, node, NULL); + + for (int i = 0; i < ARRAY_SIZE(props); i++) { + int count = fdt_stringlist_count(fdt, node, props[i]); + + if (count < 0) + continue; + + for (int j = 0; j < count; j++) { + const char *img_name = + fdt_stringlist_get(fdt, node, props[i], j, NULL); + if (!img_name || !*img_name) + continue; + + int img = fdt_subnode_offset(fdt, images, img_name); + + if (img < 0) { + fprintf(stderr, + "Error: configuration '%s' references undefined image '%s' in property '%s'\n", + conf_name, img_name, props[i]); + ret = FDT_ERR_NOTFOUND; + goto err_munmap; + } + } + } + } + munmap(old_fdt, sbuf.st_size); /* Close the old fd so we can re-use it. */ diff --git a/tools/mkimage.c b/tools/mkimage.c index 2954626a283..361711c53b2 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -519,8 +519,13 @@ int main(int argc, char **argv) */ retval = tparams->fflag_handle(¶ms); - if (retval != EXIT_SUCCESS) + if (retval != EXIT_SUCCESS) { + if (retval == FDT_ERR_NOTFOUND) { + // Already printed error, exit cleanly + exit(EXIT_FAILURE); + } usage("Bad parameters for FIT image type"); + } } if (params.lflag || params.fflag) { From 3071e264c9baba4d55aa6d1a07a60c969d57eccf Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Tue, 10 Jun 2025 07:41:19 +0000 Subject: [PATCH 3/5] binman: test: Ensure all config references exist in /images node Several binman FIT test device trees reference image nodes such as atf and uboot in their /configurations sections, but those image nodes were not actually defined in the /images node. This mismatch can lead to validation errors when stricter consistency checks are introduced. This patch adds minimal definitions for atf and uboot under the /images node in all relevant test DTS files. Signed-off-by: Aristo Chen --- tools/binman/test/170_fit_fdt.dts | 14 ++++++++++++++ tools/binman/test/220_fit_subentry_bintool.dts | 2 +- tools/binman/test/223_fit_fdt_oper.dts | 14 ++++++++++++++ tools/binman/test/284_fit_fdt_list.dts | 14 ++++++++++++++ tools/binman/test/333_fit_fdt_dir.dts | 14 ++++++++++++++ tools/binman/test/334_fit_fdt_compat.dts | 14 ++++++++++++++ tools/binman/test/335_fit_fdt_phase.dts | 14 ++++++++++++++ tools/binman/test/345_fit_fdt_name.dts | 14 ++++++++++++++ 8 files changed, 99 insertions(+), 1 deletion(-) diff --git a/tools/binman/test/170_fit_fdt.dts b/tools/binman/test/170_fit_fdt.dts index 0197ffd1597..4b1e9b41ec0 100644 --- a/tools/binman/test/170_fit_fdt.dts +++ b/tools/binman/test/170_fit_fdt.dts @@ -15,6 +15,20 @@ fit,fdt-list = "of-list"; images { + atf { + description = "atf firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; + uboot { + description = "U-Boot firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; kernel { description = "Vanilla Linux kernel"; type = "kernel"; diff --git a/tools/binman/test/220_fit_subentry_bintool.dts b/tools/binman/test/220_fit_subentry_bintool.dts index 6e29d41eeb3..b1d8fb0feae 100644 --- a/tools/binman/test/220_fit_subentry_bintool.dts +++ b/tools/binman/test/220_fit_subentry_bintool.dts @@ -12,7 +12,7 @@ #address-cells = <1>; images { - test { + kernel { description = "Something using a bintool"; type = "kernel"; arch = "arm"; diff --git a/tools/binman/test/223_fit_fdt_oper.dts b/tools/binman/test/223_fit_fdt_oper.dts index e630165acf4..cb3b31e36f6 100644 --- a/tools/binman/test/223_fit_fdt_oper.dts +++ b/tools/binman/test/223_fit_fdt_oper.dts @@ -15,6 +15,20 @@ fit,fdt-list = "of-list"; images { + atf { + description = "atf firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; + uboot { + description = "U-Boot firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; kernel { description = "Vanilla Linux kernel"; type = "kernel"; diff --git a/tools/binman/test/284_fit_fdt_list.dts b/tools/binman/test/284_fit_fdt_list.dts index 8885313f5b8..70cdb326708 100644 --- a/tools/binman/test/284_fit_fdt_list.dts +++ b/tools/binman/test/284_fit_fdt_list.dts @@ -15,6 +15,20 @@ fit,fdt-list-val = "test-fdt1", "test-fdt2"; images { + atf { + description = "atf firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; + uboot { + description = "U-Boot firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; kernel { description = "Vanilla Linux kernel"; type = "kernel"; diff --git a/tools/binman/test/333_fit_fdt_dir.dts b/tools/binman/test/333_fit_fdt_dir.dts index aa778451a4b..71971de4232 100644 --- a/tools/binman/test/333_fit_fdt_dir.dts +++ b/tools/binman/test/333_fit_fdt_dir.dts @@ -15,6 +15,20 @@ fit,fdt-list-dir = "fdts"; images { + atf { + description = "atf firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; + uboot { + description = "U-Boot firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; kernel { description = "Vanilla Linux kernel"; type = "kernel"; diff --git a/tools/binman/test/334_fit_fdt_compat.dts b/tools/binman/test/334_fit_fdt_compat.dts index 3bf45c710db..bf1b5a4a94a 100644 --- a/tools/binman/test/334_fit_fdt_compat.dts +++ b/tools/binman/test/334_fit_fdt_compat.dts @@ -15,6 +15,20 @@ fit,fdt-list = "of-list"; images { + atf { + description = "atf firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; + uboot { + description = "U-Boot firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; kernel { description = "Vanilla Linux kernel"; type = "kernel"; diff --git a/tools/binman/test/335_fit_fdt_phase.dts b/tools/binman/test/335_fit_fdt_phase.dts index f8d0740a394..c20bcad651a 100644 --- a/tools/binman/test/335_fit_fdt_phase.dts +++ b/tools/binman/test/335_fit_fdt_phase.dts @@ -15,6 +15,20 @@ fit,fdt-list = "of-list"; images { + atf { + description = "atf firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; + uboot { + description = "U-Boot firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; kernel { description = "Vanilla Linux kernel"; type = "kernel"; diff --git a/tools/binman/test/345_fit_fdt_name.dts b/tools/binman/test/345_fit_fdt_name.dts index 631a8e5f59b..0ef2e1934a0 100644 --- a/tools/binman/test/345_fit_fdt_name.dts +++ b/tools/binman/test/345_fit_fdt_name.dts @@ -15,6 +15,20 @@ fit,fdt-list = "of-list"; images { + atf { + description = "atf firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; + uboot { + description = "U-Boot firmware"; + type = "firmware"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + }; kernel { description = "Vanilla Linux kernel"; type = "kernel"; From 21f75eb7af40cd22ecee94554b2a226760fe9814 Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Tue, 10 Jun 2025 07:41:20 +0000 Subject: [PATCH 4/5] test: py: add mkimage test for undefined image references in FIT configs Add a test case to verify that mkimage correctly rejects a FIT source that references a non-existent image from a configuration node. This test introduces a minimal ITS that defines a valid kernel image but references a missing "fdt" image under the /configurations section. The test asserts that mkimage fails with a clear error message, as introduced in the new validation logic. This helps ensure the validation logic behaves correctly and prevents regressions in future FIT enhancements. Signed-off-by: Aristo Chen --- test/py/tests/test_fit_mkimage_validate.py | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 test/py/tests/test_fit_mkimage_validate.py diff --git a/test/py/tests/test_fit_mkimage_validate.py b/test/py/tests/test_fit_mkimage_validate.py new file mode 100644 index 00000000000..af56f08ca10 --- /dev/null +++ b/test/py/tests/test_fit_mkimage_validate.py @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2025 +# +# Test that mkimage validates image references in configurations + +import os +import subprocess +import pytest +import fit_util + +@pytest.mark.boardspec('sandbox') +@pytest.mark.requiredtool('dtc') +def test_fit_invalid_image_reference(ubman): + """Test that mkimage fails when configuration references a missing image""" + + its_fname = fit_util.make_fname(ubman, "invalid.its") + itb_fname = fit_util.make_fname(ubman, "invalid.itb") + kernel = fit_util.make_kernel(ubman, 'kernel.bin', 'kernel') + + # Write ITS with an invalid reference to a nonexistent image + its_text = ''' +/dts-v1/; + +/ { + images { + kernel@1 { + description = "Test Kernel"; + data = /incbin/("kernel.bin"); + type = "kernel"; + arch = "sandbox"; + os = "linux"; + compression = "none"; + load = <0x40000>; + entry = <0x40000>; + }; + }; + + configurations { + default = "conf@1"; + conf@1 { + kernel = "kernel@1"; + fdt = "notexist"; + }; + }; +}; +''' + + with open(its_fname, 'w') as f: + f.write(its_text) + + mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage') + cmd = [mkimage, '-f', its_fname, itb_fname] + + result = subprocess.run(cmd, capture_output=True, text=True) + + assert result.returncode != 0, "mkimage should fail due to missing image reference" + assert "references undefined image 'notexist'" in result.stderr + From 647931cfeda1ed594904e7dd4262783a264cb97a Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Tue, 10 Jun 2025 07:41:21 +0000 Subject: [PATCH 5/5] arm: dts: phycore-am62x: Add missing tifsstub image nodes for FIT loadables The phycore-am62x build was broken due to mkimage reporting an undefined 'image "tifsstub-hs"' in the 'loadables' property of the FIT configuration. This occurred because the `loadables` field referenced `tifsstub-hs`, `tifsstub-fs`, and `tifsstub-gp`, but no corresponding nodes were defined under /images. This patch was inspired by commit 622f826bf025704cbcc4f39252d4a83129a9cabb ("arm: dts: phycore-am62x: Package TIFS Stub"). It resolves the issue by adding proper Binman nodes for each TIFS variant (`tifsstub-hs`, `tifsstub-fs`, and `tifsstub-gp`). Signed-off-by: Aristo Chen --- arch/arm/dts/k3-am625-phycore-som-binman.dtsi | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/arch/arm/dts/k3-am625-phycore-som-binman.dtsi b/arch/arm/dts/k3-am625-phycore-som-binman.dtsi index 9b536d679af..b01bc5dfead 100644 --- a/arch/arm/dts/k3-am625-phycore-som-binman.dtsi +++ b/arch/arm/dts/k3-am625-phycore-som-binman.dtsi @@ -405,11 +405,105 @@ }; &binman { + tifsstub-hs { + filename = "tifsstub.bin_hs"; + ti-secure-rom { + content = <&tifsstub_hs_cert>; + core = "secure"; + load = <0x40000>; + sw-rev = ; + keyfile = "custMpk.pem"; + countersign; + tifsstub; + }; + tifsstub_hs_cert: tifsstub-hs-cert.bin { + filename = "ti-sysfw/ti-fs-stub-firmware-am62x-hs-cert.bin"; + type = "blob-ext"; + optional; + }; + tifsstub_hs_enc: tifsstub-hs-enc.bin { + filename = "ti-sysfw/ti-fs-stub-firmware-am62x-hs-enc.bin"; + type = "blob-ext"; + optional; + }; + }; + + tifsstub-fs { + filename = "tifsstub.bin_fs"; + tifsstub_fs_cert: tifsstub-fs-cert.bin { + filename = "ti-sysfw/ti-fs-stub-firmware-am62x-hs-cert.bin"; + type = "blob-ext"; + optional; + }; + tifsstub_fs_enc: tifsstub-fs-enc.bin { + filename = "ti-sysfw/ti-fs-stub-firmware-am62x-hs-enc.bin"; + type = "blob-ext"; + optional; + }; + + }; + + tifsstub-gp { + filename = "tifsstub.bin_gp"; + ti-secure-rom { + content = <&tifsstub_gp>; + core = "secure"; + load = <0x60000>; + sw-rev = ; + keyfile = "ti-degenerate-key.pem"; + tifsstub; + }; + tifsstub_gp: tifsstub-gp.bin { + filename = "ti-sysfw/ti-fs-stub-firmware-am62x-gp.bin"; + type = "blob-ext"; + optional; + }; + }; + ti-spl_unsigned { insert-template = <&ti_spl_unsigned_template>; fit { images { + tifsstub-hs { + description = "TIFSSTUB"; + type = "firmware"; + arch = "arm32"; + compression = "none"; + os = "tifsstub-hs"; + load = <0x9dc00000>; + entry = <0x9dc00000>; + blob-ext { + filename = "tifsstub.bin_hs"; + }; + }; + + tifsstub-fs { + description = "TIFSSTUB"; + type = "firmware"; + arch = "arm32"; + compression = "none"; + os = "tifsstub-fs"; + load = <0x9dc00000>; + entry = <0x9dc00000>; + blob-ext { + filename = "tifsstub.bin_fs"; + }; + }; + + tifsstub-gp { + description = "TIFSSTUB"; + type = "firmware"; + arch = "arm32"; + compression = "none"; + os = "tifsstub-gp"; + load = <0x9dc00000>; + entry = <0x9dc00000>; + blob-ext { + filename = "tifsstub.bin_gp"; + }; + }; + dm { ti-dm { filename = "ti-dm/am62xx/ipc_echo_testb_mcu1_0_release_strip.xer5f";