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

Aristo Chen <jj251510319013@gmail.com> 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/20250914110021.4103-1-aristo.chen@canonical.com
This commit is contained in:
Tom Rini
2025-09-23 13:41:16 -06:00
17 changed files with 257 additions and 48 deletions
+11 -11
View File
@@ -234,8 +234,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-fs";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc10000>;
entry = <0x9dc10000>;
blob-ext {
filename = "tifsstub.bin_fs";
};
@@ -247,8 +247,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-gp";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc20000>;
entry = <0x9dc20000>;
blob-ext {
filename = "tifsstub.bin_gp";
};
@@ -322,7 +322,7 @@
description = "k3-am6xx-phycore-disable-spi-nor";
type = "flat_dt";
compression = "none";
load = <0x8F001000>;
load = <0x8F002000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_spi_not_dtbo>;
@@ -337,7 +337,7 @@
description = "k3-am6xx-phycore-disable-eth-phy";
type = "flat_dt";
compression = "none";
load = <0x8F002000>;
load = <0x8F004000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_eth_phy_dtbo>;
@@ -352,7 +352,7 @@
description = "k3-am6xx-phycore-qspi-nor";
type = "flat_dt";
compression = "none";
load = <0x8F003000>;
load = <0x8F006000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_qspi_nor_dtbo>;
@@ -479,8 +479,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-fs";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc10000>;
entry = <0x9dc10000>;
blob-ext {
filename = "tifsstub.bin_fs";
};
@@ -492,8 +492,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-gp";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc20000>;
entry = <0x9dc20000>;
blob-ext {
filename = "tifsstub.bin_gp";
};
+8 -8
View File
@@ -231,8 +231,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-fs";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc10000>;
entry = <0x9dc10000>;
blob-ext {
filename = "tifsstub.bin_fs";
};
@@ -244,8 +244,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-gp";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc20000>;
entry = <0x9dc20000>;
blob-ext {
filename = "tifsstub.bin_gp";
};
@@ -362,8 +362,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-fs";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc10000>;
entry = <0x9dc10000>;
blob-ext {
filename = "tifsstub.bin_fs";
};
@@ -375,8 +375,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-gp";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc20000>;
entry = <0x9dc20000>;
blob-ext {
filename = "tifsstub.bin_gp";
};
@@ -219,8 +219,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-fs";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc10000>;
entry = <0x9dc10000>;
blob-ext {
filename = "tifsstub.bin_fs";
};
@@ -232,8 +232,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-gp";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc20000>;
entry = <0x9dc20000>;
blob-ext {
filename = "tifsstub.bin_gp";
};
@@ -346,8 +346,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-fs";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc10000>;
entry = <0x9dc10000>;
blob-ext {
filename = "tifsstub.bin_fs";
};
@@ -359,8 +359,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-gp";
load = <0x9dc00000>;
entry = <0x9dc00000>;
load = <0x9dc20000>;
entry = <0x9dc20000>;
blob-ext {
filename = "tifsstub.bin_gp";
};
@@ -184,8 +184,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-fs";
load = <0x9ca00000>;
entry = <0x9ca00000>;
load = <0x9ca10000>;
entry = <0x9ca10000>;
blob-ext {
filename = "tifsstub.bin_fs";
};
@@ -260,7 +260,7 @@
description = "k3-am6xx-phycore-disable-spi-nor";
type = "flat_dt";
compression = "none";
load = <0x8F001000>;
load = <0x8F002000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_spi_not_dtbo>;
@@ -275,7 +275,7 @@
description = "k3-am6xx-phycore-disable-eth-phy";
type = "flat_dt";
compression = "none";
load = <0x8F002000>;
load = <0x8F004000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_eth_phy_dtbo>;
@@ -290,7 +290,7 @@
description = "k3-am6xx-phycore-qspi-nor";
type = "flat_dt";
compression = "none";
load = <0x8F003000>;
load = <0x8F006000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_qspi_nor_dtbo>;
+2 -2
View File
@@ -168,8 +168,8 @@
arch = "arm32";
compression = "none";
os = "tifsstub-fs";
load = <0x9ca00000>;
entry = <0x9ca00000>;
load = <0x9ca10000>;
entry = <0x9ca10000>;
blob-ext {
filename = "tifsstub.bin_fs";
};
@@ -371,7 +371,7 @@
description = "k3-am6xx-phycore-disable-spi-nor";
type = "flat_dt";
compression = "none";
load = <0x8F001000>;
load = <0x8F002000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_spi_not_dtbo>;
@@ -386,7 +386,7 @@
description = "k3-am6xx-phycore-disable-eth-phy";
type = "flat_dt";
compression = "none";
load = <0x8F002000>;
load = <0x8F004000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_eth_phy_dtbo>;
@@ -401,7 +401,7 @@
description = "k3-am6xx-phycore-qspi-nor";
type = "flat_dt";
compression = "none";
load = <0x8F003000>;
load = <0x8F006000>;
arch = "arm";
ti-secure {
content = <&am6xx_phycore_disable_qspi_nor_dtbo>;
@@ -103,3 +103,68 @@ def test_fit_invalid_default_config(ubman):
assert result.returncode != 0, "mkimage should fail due to missing default config"
assert re.search(r"Default configuration '.*' not found under /configurations", result.stderr)
def test_fit_load_addr_overlap(ubman):
"""Test that mkimage fails when load address overlap"""
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')
fdt = fit_util.make_dtb(ubman, '''
/dts-v1/;
/ {
model = "Test FDT";
compatible = "test";
};
''', 'test')
# Write ITS with an invalid reference to a nonexistent default config
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>;
};
fdt@1 {
description = "Test FDT";
data = /incbin/("test.dtb");
type = "flat_dt";
arch = "sandbox";
os = "linux";
compression = "none";
load = <0x40000>;
entry = <0x40000>;
};
};
configurations {
default = "conf@1";
conf@1 {
kernel = "kernel@1";
fdt = "fdt@1";
};
};
};
'''
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 memory overlap"
assert "Error: Overlap detected:" in result.stderr
# Check that it identifies the specific overlapping components
assert "kernel@1" in result.stderr and "fdt@1" in result.stderr
+18
View File
@@ -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
+2 -2
View File
@@ -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')
@@ -19,8 +19,8 @@
arch = "arm64";
os = "u-boot";
compression = "none";
load = <0x00000000>;
entry = <0x00000000>;
load = <0x00002000>;
entry = <0x00002000>;
u-boot-nodtb {
};
+2 -2
View File
@@ -20,8 +20,8 @@
arch = "arm64";
os = "u-boot";
compression = "none";
load = <0x00000000>;
entry = <0x00000000>;
load = <0x00002000>;
entry = <0x00002000>;
u-boot-nodtb {
};
+2 -2
View File
@@ -20,8 +20,8 @@
arch = "arm64";
os = "u-boot";
compression = "none";
load = <0x00000000>;
entry = <0x00000000>;
load = <0x00002000>;
entry = <0x00002000>;
u-boot-nodtb {
};
+5 -1
View File
@@ -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)
+1
View File
@@ -0,0 +1 @@
elf_sections.c
+32
View File
@@ -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*)
}
}
+89 -1
View File
@@ -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);
+2 -1
View File
@@ -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);
}