Merge patch series "mkimage: Detect FIT image load address overlaps and fix related test/DTS issues"

Aristo Chen <[email protected]> says:

This patch series enhances FIT image robustness by adding **memory
region overlap detection** to `mkimage` and fixing existing overlaps
in DTS files and `binman` tests.

The primary goal is to prevent runtime memory corruption from
conflicting load addresses in FIT images.

Key Changes:

1.  `mkimage` Overlap Detection: A new validation in
    `tools/fit_image.c` checks for overlapping load addresses
    within FIT configurations. `mkimage` now errors out with
    detailed info on conflicts, preventing bad FIT image creation.

2.  New Test Case: A Python test verifies the new detection.
    It intentionally creates an overlap (kernel and FDT)
    to confirm correct error handling.

3.  Fixes for Existing Overlaps:
    * Board DTS (k3-am6xx): Adjusted load addresses for TI
      firmware stubs to prevent conflicts. This resolves
      previously undetected overlaps.
    * `binman` Tests: Fixed several tests. U-Boot load
      addresses were shifted to avoid ATF conflicts. A new
      linker script for TEE ELF sections ensures distinct
      memory layouts.

4.  Documentation: Added guidance for developers on how to
    determine ELF load addresses using readelf, linker scripts,
    and objdump when working with binman FIT images.

Impact:

This series improves FIT image reliability by catching overlaps
at build time, helping developers resolve issues before runtime
failures.

Link: https://lore.kernel.org/r/[email protected]

10 files changed, 155 insertions, 11 deletions

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 8922d6cd070..173b7eef6cc 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1050,6 +1050,24 @@ split-elf
         Generates a `load = <...>` property with the load address of the
         segment
 
+        Note: The load address comes from the ELF file's program header or
+        linker script. To determine where an ELF file will be loaded, you can:
+
+        1. Use readelf to examine the program headers:
+           ``readelf -l your_elf_file.elf``
+           Look for the LOAD segments and their VirtAddr (Virtual Address)
+
+        2. Check the linker script (.lds file) used to build the ELF:
+           Look for the `. = <address>;` statements which set the location
+           counter and determine load addresses for different sections
+
+        3. Use objdump to see section addresses:
+           ``objdump -h your_elf_file.elf``
+
+        For example, in binman tests, elf_sections.lds sets ATF load address
+        to 0x00000010, while elf_sections_tee.lds sets TEE load address to
+        0x00100010 to avoid memory overlap conflicts.
+
     fit,entry
         Generates a `entry = <...>` property with the entry address of the
         ELF. This is only produced for the first entry
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 925c39a530e..0c2dbf333c0 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -252,7 +252,7 @@ class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('bl31.elf',
             tools.read_file(cls.ElfTestFile('elf_sections')))
         TestFunctional.tee_elf_path = TestFunctional._MakeInputFile('tee.elf',
-            tools.read_file(cls.ElfTestFile('elf_sections')))
+            tools.read_file(cls.ElfTestFile('elf_sections_tee')))
 
         # Newer OP_TEE file in v1 binary format
         cls.make_tee_bin('tee.bin')
@@ -7997,7 +7997,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
             'Node \'/binman/fit\': multiple key paths found',
             str(e.exception))
 
-    def testFitSignNoSingatureNodes(self):
+    def testFitSignNoSignatureNodes(self):
         """Test that fit,sign doens't raise error if no signature nodes found"""
         if not elf.ELF_TOOLS:
             self.skipTest('Python elftools not available')
diff --git a/tools/binman/test/276_fit_firmware_loadables.dts b/tools/binman/test/276_fit_firmware_loadables.dts
index 2f79cdc9bb8..d344036a11a 100644
--- a/tools/binman/test/276_fit_firmware_loadables.dts
+++ b/tools/binman/test/276_fit_firmware_loadables.dts
@@ -19,8 +19,8 @@
 					arch = "arm64";
 					os = "u-boot";
 					compression = "none";
-					load = <0x00000000>;
-					entry = <0x00000000>;
+					load = <0x00002000>;
+					entry = <0x00002000>;
 
 					u-boot-nodtb {
 					};
diff --git a/tools/binman/test/340_fit_signature.dts b/tools/binman/test/340_fit_signature.dts
index 9dce62e52de..1c25d52cba4 100644
--- a/tools/binman/test/340_fit_signature.dts
+++ b/tools/binman/test/340_fit_signature.dts
@@ -20,8 +20,8 @@
 					arch = "arm64";
 					os = "u-boot";
 					compression = "none";
-					load = <0x00000000>;
-					entry = <0x00000000>;
+					load = <0x00002000>;
+					entry = <0x00002000>;
 
 					u-boot-nodtb {
 					};
diff --git a/tools/binman/test/342_fit_signature.dts b/tools/binman/test/342_fit_signature.dts
index 267105d0f68..2ac600b1c70 100644
--- a/tools/binman/test/342_fit_signature.dts
+++ b/tools/binman/test/342_fit_signature.dts
@@ -20,8 +20,8 @@
 					arch = "arm64";
 					os = "u-boot";
 					compression = "none";
-					load = <0x00000000>;
-					entry = <0x00000000>;
+					load = <0x00002000>;
+					entry = <0x00002000>;
 
 					u-boot-nodtb {
 					};
diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile
index 4d152eee9c0..66279e0e207 100644
--- a/tools/binman/test/Makefile
+++ b/tools/binman/test/Makefile
@@ -30,12 +30,13 @@ LDS_BINMAN_BAD := -T $(SRC)u_boot_binman_syms_bad.lds
 LDS_BINMAN_X86 := -T $(SRC)u_boot_binman_syms_x86.lds
 LDS_BINMAN_EMBED := -T $(SRC)u_boot_binman_embed.lds
 LDS_EFL_SECTIONS := -T $(SRC)elf_sections.lds
+LDS_EFL_SECTIONS_TEE := -T $(SRC)elf_sections_tee.lds
 LDS_BLOB := -T $(SRC)blob_syms.lds
 
 TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data bss_data_zero \
 	u_boot_binman_syms u_boot_binman_syms.bin u_boot_binman_syms_bad \
 	u_boot_binman_syms_size u_boot_binman_syms_x86 embed_data \
-	u_boot_binman_embed u_boot_binman_embed_sm elf_sections blob_syms.bin
+	u_boot_binman_embed u_boot_binman_embed_sm elf_sections elf_sections_tee blob_syms.bin
 
 all: $(TARGETS)
 
@@ -84,6 +85,9 @@ blob_syms: blob_syms.c
 elf_sections: CFLAGS += $(LDS_EFL_SECTIONS)
 elf_sections: elf_sections.c
 
+elf_sections_tee: CFLAGS += $(LDS_EFL_SECTIONS_TEE)
+elf_sections_tee: elf_sections_tee.c
+
 clean:
 	rm -f $(TARGETS)
 
diff --git a/tools/binman/test/elf_sections_tee.c b/tools/binman/test/elf_sections_tee.c
new file mode 120000
index 00000000000..01b200a365e
--- /dev/null
+++ b/tools/binman/test/elf_sections_tee.c
@@ -0,0 +1 @@
+elf_sections.c
\ No newline at end of file
diff --git a/tools/binman/test/elf_sections_tee.lds b/tools/binman/test/elf_sections_tee.lds
new file mode 100644
index 00000000000..97e5e5f5d94
--- /dev/null
+++ b/tools/binman/test/elf_sections_tee.lds
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2016 Google, Inc
+ * Copyright (c) 2025 Canonical Ltd.
+ */
+
+OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386")
+OUTPUT_ARCH(i386)
+ENTRY(_start)
+
+SECTIONS
+{
+	. = 0x00100010;
+	_start = .;
+
+	. = ALIGN(4);
+	.text :
+	{
+		*(.text*)
+	}
+
+	. = 0x00101000;
+	.sram :
+	{
+		*(.sram*)
+	}
+
+	/DISCARD/ : {
+		*(.comment)
+		*(.dyn*)
+	}
+}
diff --git a/tools/fit_image.c b/tools/fit_image.c
index 10849733816..12f4cdb2875 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -22,6 +22,20 @@
 #include <version.h>
 #include <u-boot/crc.h>
 
+struct fit_region {
+	ulong load;
+	ulong size;
+	const char *name;
+};
+
+static int regions_overlap(const struct fit_region *a, const struct fit_region *b)
+{
+	ulong a_end = a->load + a->size;
+	ulong b_end = b->load + b->size;
+
+	return !(a_end <= b->load || b_end <= a->load);
+}
+
 static struct legacy_img_hdr header;
 
 static int fit_estimate_hash_sig_size(struct image_tool_params *params, const char *fname)
@@ -823,9 +837,12 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
 	}
 
 	fdt_for_each_subnode(node, fdt, confs) {
+		struct fit_region *regions = NULL;
+		unsigned int img_count = 0;
+		unsigned int regions_allocated = 0;
 		const char *conf_name = fdt_get_name(fdt, node, NULL);
 
-		for (int i = 0; i < ARRAY_SIZE(props); i++) {
+		for (unsigned int i = 0; i < ARRAY_SIZE(props); i++) {
 			int count = fdt_stringlist_count(fdt, node, props[i]);
 
 			if (count < 0)
@@ -846,8 +863,79 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
 					ret = FDT_ERR_NOTFOUND;
 					goto err_munmap;
 				}
+
+				ulong img_load = 0;
+				int img_size = 0;
+
+				if (fit_image_get_load(fdt, img, &img_load)) {
+					fprintf(stderr,
+						"Warning: not able to get `load` of node '%s'\n",
+						img_name);
+					// Skip checking the components that do not have a
+					// definition for `load`
+					continue;
+				}
+				const char *img_data = fdt_getprop(fdt, img,
+								   FIT_DATA_PROP,
+								   &img_size);
+
+				if (!img_data || !img_size)
+					continue;
+
+				// Check if we've already added this image to avoid duplicates
+				for (unsigned int k = 0; k < img_count; k++) {
+					if (!strcmp(regions[k].name, img_name))
+						goto next_node;
+				}
+
+				// Expand regions array if needed
+				if (img_count >= regions_allocated) {
+					unsigned int new_size = regions_allocated ?
+								regions_allocated * 2 : 8;
+					struct fit_region *new_regions = realloc(regions,
+							new_size * sizeof(struct fit_region));
+					if (!new_regions) {
+						fprintf(stderr,
+							"Failed to allocate memory for regions in config %s\n",
+							fdt_get_name(fdt, node, NULL));
+						free(regions);
+						ret = -ENOMEM;
+						goto err_munmap;
+					}
+					regions = new_regions;
+					regions_allocated = new_size;
+				}
+
+				regions[img_count].load = img_load;
+				regions[img_count].size = img_size;
+				regions[img_count].name = img_name;
+				img_count++;
+next_node:;
+			}
+		}
+
+		// Check for overlap within this config only
+		for (unsigned int i = 0; i < img_count; i++) {
+			for (unsigned int j = i + 1; j < img_count; j++) {
+				if (regions_overlap(&regions[i], &regions[j])) {
+					fprintf(stderr,
+						"[Config: %s] Error: Overlap detected:\n"
+						"  - %s: [0x%lx - 0x%lx]\n"
+						"  - %s: [0x%lx - 0x%lx]\n",
+						fdt_get_name(fdt, node, NULL),
+						regions[i].name, regions[i].load,
+						regions[i].load + regions[i].size,
+						regions[j].name, regions[j].load,
+						regions[j].load + regions[j].size);
+					ret = FDT_ERR_BADSTRUCTURE;
+					free(regions);
+					goto err_munmap;
+				}
 			}
 		}
+
+		// Clean up allocated memory for this configuration
+		free(regions);
 	}
 
 	munmap(old_fdt, sbuf.st_size);
diff --git a/tools/mkimage.c b/tools/mkimage.c
index 12183270776..e96fb7e42db 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -533,7 +533,8 @@ int main(int argc, char **argv)
 			retval = tparams->fflag_handle(&params);
 
 		if (retval != EXIT_SUCCESS) {
-			if (retval == FDT_ERR_NOTFOUND) {
+			if (retval == FDT_ERR_NOTFOUND ||
+			    retval == FDT_ERR_BADSTRUCTURE) {
 				// Already printed error, exit cleanly
 				exit(EXIT_FAILURE);
 			}