[PATCH v3 09/11] boot: fit: share the external-data offset calculation

Daniel Golle daniel at makrotopia.org
Sat Oct 31 04:12:02 PDT 2026


fit_image_load_storage() repeats the data-position / data-offset /
totalsize-alignment sequence that fit_image_get_data() performs, and
writes the result through an int pointer where the getters expect a u32.
The copy also omits the range check on the sum, so an offset that the
embedded path rejects is accepted here.

Extract fit_image_get_ext_data_offset() and call it from both. The
storage path gains the range check and both agree on one expression for
the alignment. The signed comparison of the offset and size is
meaningless once both are u32, so it is dropped in favour of the shared
range check the helper provides.

Reviewed-by: Simon Glass <sjg at chromium.org>
Signed-off-by: Daniel Golle <daniel at makrotopia.org>
---
v3: No changes since v2.
v2:
 - Rename the helper to fit_image_get_ext_data_offset() to match the
   sibling fit_image_get_*() getters.
 - fit_image_get_data() folds the embedded case into an early return
   on -ENOENT, dropping the external_data local.
 - Note in the message that the signed-value guard in the storage
   path is dead once data_off and data_sz are u32.

 boot/image-fit.c | 166 ++++++++++++++++++++++++++---------------------
 1 file changed, 91 insertions(+), 75 deletions(-)

diff --git a/boot/image-fit.c b/boot/image-fit.c
index 6ab6c6c7da9..0367331c25d 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1056,6 +1056,47 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
 	return 0;
 }
 
+/**
+ * fit_image_get_ext_data_offset - locate a sub-image's external data
+ * @fit: pointer to the FIT format image header
+ * @noffset: component image node offset
+ * @offsetp: holds the data offset from the start of the FIT, on success
+ *
+ * returns:
+ *     0, if the sub-image has external data
+ *     -ENOENT, if its data is embedded in the FIT
+ *     -EINVAL, if the recorded offset is out of range
+ */
+static int fit_image_get_ext_data_offset(const void *fit, int noffset, u32 *offsetp)
+{
+	size_t fdt_total_size_aligned;
+	u32 offset;
+
+	if (!fit_image_get_data_position(fit, noffset, &offset)) {
+		*offsetp = offset;
+
+		return 0;
+	}
+
+	if (fit_image_get_data_offset(fit, noffset, &offset))
+		return -ENOENT;
+
+	/*
+	 * data-offset counts from the end of the FDT, so that is the base for
+	 * every image; the sum has to stay within a u32.
+	 */
+	fdt_total_size_aligned = ALIGN(fdt_totalsize(fit), 4);
+	if (fdt_total_size_aligned > UINT32_MAX - offset) {
+		printf("Invalid external data offset: %u\n", offset);
+
+		return -EINVAL;
+	}
+
+	*offsetp = offset + fdt_total_size_aligned;
+
+	return 0;
+}
+
 /**
  * fit_image_get_data - get data and its size including
  *				 both embedded and external data
@@ -1075,82 +1116,63 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
 int fit_image_get_data(const void *fit, int noffset, const void **data,
 		       size_t *size)
 {
-	bool external_data = false;
 	int ret;
-	size_t fdt_total_size_aligned;
 	u32 offset;
 	u32 len;
 	uintptr_t max_offset;
 
-	if (!fit_image_get_data_position(fit, noffset, &offset)) {
-		external_data = true;
-	} else if (!fit_image_get_data_offset(fit, noffset, &offset)) {
-		/*
-		 * For FIT with external data, figure out where
-		 * the external images start. This is the base
-		 * for the data-offset properties in each image.
-		 */
-		fdt_total_size_aligned = ((fdt_totalsize(fit) + 3) & ~3);
-		/* The resulting offset cannot exceed UINT32_MAX */
-		if (fdt_total_size_aligned > UINT32_MAX - offset) {
-			printf("Invalid external data offset: %u\n", offset);
-			return -EINVAL;
-		}
-		offset += fdt_total_size_aligned;
-
-		external_data = true;
-	}
+	ret = fit_image_get_ext_data_offset(fit, noffset, &offset);
+	if (ret == -ENOENT)
+		return fit_image_get_emb_data(fit, noffset, data, size);
+	if (ret)
+		return ret;
 
-	if (external_data) {
-		debug("External Data\n");
+	debug("External Data\n");
 
-		max_offset = UINTPTR_MAX - (uintptr_t)fit;
-		/* Check that external data offset is within the addressable range */
-		if (offset > max_offset) {
-			printf("Invalid external data offset: %u\n", offset);
-			return -EINVAL;
-		}
+	max_offset = UINTPTR_MAX - (uintptr_t)fit;
+	/* Check that external data offset is within the addressable range */
+	if (offset > max_offset) {
+		printf("Invalid external data offset: %u\n", offset);
+		return -EINVAL;
+	}
 
-		ret = fit_image_get_data_size(fit, noffset, &len);
-		if (!ret) {
-			/*
-			 * For non-signed FIT images, we can only check that
-			 * (offset + len) doesn't exceed the addressable range.
-			 * For signed FITs, we can additionally check that
-			 * (offset + len) doesn't exceed the allowed FIT image
-			 * maximum size.
-			 */
-			if (len > max_offset - offset
-			/*
-			 * #if (not a runtime if) is required: FIT_SIGNATURE_MAX_SIZE
-			 * depends on FIT_SIGNATURE, so CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)
-			 * is undefined when signing is disabled and referencing it
-			 * here would fail to compile.
-			 */
+	ret = fit_image_get_data_size(fit, noffset, &len);
+	if (!ret) {
+		/*
+		 * For non-signed FIT images, we can only check that
+		 * (offset + len) doesn't exceed the addressable range.
+		 * For signed FITs, we can additionally check that
+		 * (offset + len) doesn't exceed the allowed FIT image
+		 * maximum size.
+		 */
+		if (len > max_offset - offset
+		/*
+		 * #if (not a runtime if) is required: FIT_SIGNATURE_MAX_SIZE
+		 * depends on FIT_SIGNATURE, so CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)
+		 * is undefined when signing is disabled and referencing it
+		 * here would fail to compile.
+		 */
 #if CONFIG_IS_ENABLED(FIT_SIGNATURE)
-			    || offset > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) ||
-			    len > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) - offset
+		    || offset > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) ||
+		    len > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) - offset
 #endif
-			) {
-				printf("FIT external data is out of bounds (offset=%u, size=%u)\n",
-				       offset, len);
-				return -EINVAL;
-			}
-			*data = fit + offset;
+		) {
+			printf("FIT external data is out of bounds (offset=%u, size=%u)\n",
+			       offset, len);
+			return -EINVAL;
+		}
+		*data = fit + offset;
 #if !defined(USE_HOSTCC) && CONFIG_IS_ENABLED(IMAGEMAP)
-			if (images.imagemap) {
-				void *mapped;
+		if (images.imagemap) {
+			void *mapped;
 
-				mapped = imagemap_lookup(images.imagemap,
-							 offset, len);
-				if (mapped)
-					*data = mapped;
-			}
-#endif
-			*size = len;
+			mapped = imagemap_lookup(images.imagemap,
+						 offset, len);
+			if (mapped)
+				*data = mapped;
 		}
-	} else {
-		ret = fit_image_get_emb_data(fit, noffset, data, size);
+#endif
+		*size = len;
 	}
 
 	return ret;
@@ -2185,12 +2207,12 @@ static int fit_image_load_storage(struct bootm_headers *images, const void *fit,
 				  int noffset, enum fit_load_op load_op,
 				  ulong *datap, ulong *lenp, bool *early)
 {
-	int data_off = 0, data_sz = 0;
-	bool external = false;
+	u32 data_off = 0, data_sz = 0;
 	ulong img_load;
 	u8 img_comp = IH_COMP_NONE;
 	void *mapped;
 	void *dst;
+	int ret;
 
 	if (CONFIG_IS_ENABLED(FIT_VERITY)) {
 		u8 img_type;
@@ -2207,19 +2229,13 @@ static int fit_image_load_storage(struct bootm_headers *images, const void *fit,
 		}
 	}
 
-	if (!fit_image_get_data_position(fit, noffset, &data_off)) {
-		external = true;
-	} else if (!fit_image_get_data_offset(fit, noffset, &data_off)) {
-		external = true;
-		data_off += ALIGN(fdt_totalsize(fit), 4);
-	}
+	ret = fit_image_get_ext_data_offset(fit, noffset, &data_off);
+	if (ret)
+		return ret == -ENOENT ? 0 : ret;
 
-	if (!external || fit_image_get_data_size(fit, noffset, &data_sz))
+	if (fit_image_get_data_size(fit, noffset, &data_sz))
 		return 0;
 
-	if (data_off < 0 || data_sz < 0)
-		return -EINVAL;
-
 	fit_image_get_comp(fit, noffset, &img_comp);
 
 	if (img_comp == IH_COMP_NONE && load_op != FIT_LOAD_IGNORED &&
-- 
2.56.0



More information about the openwrt-devel mailing list