tools: mkimage: detect overlapping load regions in FIT configurations

Add a validation step to mkimage that checks, for every configuration
of a FIT image, whether the images referenced by that configuration
declare overlapping load regions. Images that are loaded together must
not share memory, and such conflicts currently go unnoticed until the
resulting image corrupts memory at runtime.

The check is strictly per configuration. Images that share a load
address while being referenced by different configurations are not
reported, because only one configuration is selected at boot. This
matches the layout used by the TI K3 tispl images since commit
c85bf61b14f9 ("arm: k3: select tifsstub via board_fit_config_name_match"),
where the mutually exclusive tifsstub variants moved into per security
state configurations precisely so that static tooling can reason about
each configuration on its own.

Images without a load address or without data are skipped, since
nothing is copied anywhere for them. Compared to the previously
reverted version of this check, the missing load address case no
longer prints a warning, as FDT images routinely have no load address
and the warning only added noise to every build. An image referenced
through several properties of the same configuration, for example
listed in both firmware and loadables, is only counted once.

When an overlap is found mkimage prints the configuration name
together with both image names and their regions, then fails with
FDT_ERR_BADSTRUCTURE. Teach main() to exit cleanly for that value the
same way it already does for FDT_ERR_NOTFOUND, since the error
message has already been printed.

Signed-off-by: Aristo Chen <aristo.chen@canonical.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

2 files changed, 101 insertions, 1 deletions

diff --git a/tools/fit_image.c b/tools/fit_image.c
index 7e59bc43b77..f0fe68894d6 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -24,6 +24,18 @@
 
 static struct legacy_img_hdr header;
 
+struct fit_region {
+	ulong load;
+	ulong size;
+	const char *name;
+};
+
+static bool fit_regions_overlap(const struct fit_region *a,
+				const struct fit_region *b)
+{
+	return a->load < b->load + b->size && b->load < a->load + a->size;
+}
+
 static int fit_estimate_hash_sig_size(struct image_tool_params *params, const char *fname)
 {
 	bool signing = IMAGE_ENABLE_SIGN &&
@@ -905,6 +917,8 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
 	int images;
 	int confs;
 	int node;
+	struct fit_region *regions = NULL;
+	unsigned int regions_alloc = 0;
 
 	fd = mmap_fdt(params->cmdname, fname, 0, &old_fdt, &sbuf, false, false);
 	if (fd < 0)
@@ -1009,6 +1023,7 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
 
 	fdt_for_each_subnode(node, fdt, confs) {
 		const char *conf_name = fdt_get_name(fdt, node, NULL);
+		unsigned int reg_count = 0;
 
 		for (int i = 0; i < ARRAY_SIZE(props); i++) {
 			int count = fdt_stringlist_count(fdt, node, props[i]);
@@ -1017,6 +1032,11 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
 				continue;
 
 			for (int j = 0; j < count; j++) {
+				const char *img_data;
+				ulong img_load;
+				int img_size;
+				unsigned int k;
+
 				const char *img_name =
 					fdt_stringlist_get(fdt, node, props[i], j, NULL);
 				if (!img_name || !*img_name)
@@ -1031,10 +1051,88 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
 					ret = FDT_ERR_NOTFOUND;
 					goto err_munmap;
 				}
+
+				/*
+				 * Collect the memory region the image is
+				 * loaded to. Images without a load address or
+				 * without data are never copied anywhere, so
+				 * they cannot conflict.
+				 */
+				if (fit_image_get_load(fdt, img, &img_load))
+					continue;
+				img_data = fdt_getprop(fdt, img, FIT_DATA_PROP,
+						       &img_size);
+				if (!img_data || img_size <= 0)
+					continue;
+
+				/*
+				 * A configuration may reference one image
+				 * through several properties, for example
+				 * both "firmware" and "loadables". Count
+				 * each image only once.
+				 */
+				for (k = 0; k < reg_count; k++) {
+					if (!strcmp(regions[k].name, img_name))
+						break;
+				}
+				if (k < reg_count)
+					continue;
+
+				if (reg_count == regions_alloc) {
+					struct fit_region *tmp;
+
+					regions_alloc = regions_alloc ?
+						regions_alloc * 2 : 8;
+					tmp = realloc(regions, regions_alloc *
+						      sizeof(*regions));
+					if (!tmp) {
+						fprintf(stderr,
+							"%s: Out of memory for %u load regions\n",
+							params->cmdname,
+							regions_alloc);
+						ret = -ENOMEM;
+						goto err_munmap;
+					}
+					regions = tmp;
+				}
+
+				regions[reg_count].load = img_load;
+				regions[reg_count].size = img_size;
+				regions[reg_count].name = img_name;
+				reg_count++;
+			}
+		}
+
+		/*
+		 * All images referenced by one configuration are loaded
+		 * together, so no two of them may claim overlapping memory
+		 * regions. Images sharing a load address across different
+		 * configurations are fine, since only one configuration is
+		 * selected at runtime.
+		 */
+		for (unsigned int i = 0; i < reg_count; i++) {
+			for (unsigned int j = i + 1; j < reg_count; j++) {
+				if (!fit_regions_overlap(&regions[i],
+							 &regions[j]))
+					continue;
+				fprintf(stderr,
+					"Error: configuration '%s' has overlapping load regions:\n"
+					"  - %s: [0x%lx - 0x%lx]\n"
+					"  - %s: [0x%lx - 0x%lx]\n",
+					conf_name,
+					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;
 			}
 		}
 	}
 
+	free(regions);
+	regions = NULL;
+
 	munmap(old_fdt, sbuf.st_size);
 
 	/* Close the old fd so we can re-use it. */
@@ -1067,6 +1165,7 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
 err_munmap:
 	munmap(old_fdt, sbuf.st_size);
 err:
+	free(regions);
 	free(fdt);
 	close(fd);
 	return ret;
diff --git a/tools/mkimage.c b/tools/mkimage.c
index 3c43962807d..6b3fd60adbf 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -550,7 +550,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);
 			}