[PATCH v1 4/5] tools: mkimage: detect overlapping load regions in FIT configurations

[Yannic Moog]

Am Dienstag, dem 29.07.2025 um 12:48 +0000 schrieb Aristo Chen:
> This patch adds a validation step in mkimage to detect memory region
> overlaps between images specified in the same configuration of a
> FIT image. If any overlaps are found, the tool prints an error and
> aborts the build.
> 
> This helps prevent runtime memory corruption caused by conflicting
> load addresses between images.
> 
> Signed-off-by: Aristo Chen <aristo.chen at canonical.com>
> ---
>  tools/fit_image.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++
>  tools/mkimage.c   |  3 +-
>  2 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index 331be5ae71d..d0061adf051 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -22,6 +22,22 @@
>  #include <version.h>
>  #include <u-boot/crc.h>
>  
> +#define MAX_IMAGES_PER_CONFIG 16

How did you get this number?

If this is arbitrary, add to the error message below that if someone needs more
images, they can increase the size.

> +
> +struct fit_region {
> +	ulong load;
> +	ulong size;
> +	const char *name;
> +};
> +
> +static int regions_overlap(struct fit_region *a, struct fit_region *b)

Nit: can you constify these?

> +{
> +	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)
> @@ -775,6 +791,8 @@ static int fit_import_data(struct image_tool_params
> *params, const char *fname)
>  	}
>  
>  	fdt_for_each_subnode(node, fdt, confs) {
> +		struct fit_region regions[MAX_IMAGES_PER_CONFIG];

init to {0}?

> +		int img_count = 0;

Nit: use unsigned

>  		const char *conf_name = fdt_get_name(fdt, node, NULL);
>  
>  		for (int i = 0; i < ARRAY_SIZE(props); i++) {
> @@ -798,6 +816,66 @@ 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 == 0)

be consistent, use !img_size

> +					continue;
> +
> +				// Check if we've already added this image to
> avoid duplicates
> +				bool already_added = false;
> +
> +				for (int k = 0; k < img_count; k++) {
> +					if (strcmp(regions[k].name, img_name)
> == 0) {

same here, !strcmp should do, right?

> +						already_added = true;

You can simplify and do continue; here?
Avoids the already_added variable.

> +						break;
> +					}
> +				}
> +				if (already_added)
> +					continue;
> +
> +				if (img_count >= MAX_IMAGES_PER_CONFIG) {
> +					fprintf(stderr, "Too many images in
> config %s\n",
> +						fdt_get_name(fdt, node,
> NULL));
> +					break;
> +				}
> +
> +				regions[img_count].load = img_load;
> +				regions[img_count].size = img_size;
> +				regions[img_count].name = img_name;
> +				img_count++;
> +			}
> +		}
> +
> +		// Check for overlap within this config only
> +		for (int i = 0; i < img_count; i++) {
> +			for (int j = i + 1; j < img_count; j++) {

Nit: unsigned here as well

Yannic

> +				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;
> +					goto err_munmap;
> +				}
>  			}
>  		}
>  	}
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index 361711c53b2..3f28918f5cf 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -520,7 +520,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);
>  			}