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.c...@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(®ions[i],
> ®ions[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(¶ms);
>
> 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);
> }
