fit_import_data() reads each image's external data from the mapped FIT
file at the offset and size the FIT itself declares, without checking
that the region lies inside the file. A data-offset or data-position
pointing past the end of the file, or a size reaching beyond it, makes
mkimage read outside the mapping, crashing on re-process or silently
importing unrelated memory as image data. An authored .its that
declares external data properties hits the same path, since dtc
produces no payload beyond the device tree.
Read the properties through fit_image_get_data_position(),
fit_image_get_data_offset() and fit_image_get_data_size(), which return
u32 since commit c4d81d062790 ("image-fit: Use unsigned types for
external data properties"), so a value of 0xffffffff is no longer
mistaken for a missing property. They fail for a property that is not
exactly one cell, and the import then skips the image as if the
property were missing, as it already did for a shorter one. Compute the
file offset in 64 bits and require the whole region to lie within the
mapped file before importing it, with an error naming the image and the
offending values. An empty region reads nothing, so it is not checked:
mkimage -E -p places one past the end of the file when every image is
empty, and such a FIT must still re-process.
Signed-off-by: Aristo Chen <[email protected]>
Reviewed-by: Simon Glass <[email protected]>
---
v2: no code changes, collected Reviewed-by
tools/fit_image.c | 41 +++++++++++++++++++++++++++++------------
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/tools/fit_image.c b/tools/fit_image.c
index b648e3e2514..f0f30e06bd9 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -1033,9 +1033,9 @@ static int fit_import_data(struct image_tool_params
*params, const char *fname)
node >= 0;
node = fdt_next_subnode(fdt, node)) {
const char *ext_data_prop = NULL;
- void *data = NULL;
- int buf_ptr;
- int len;
+ uint64_t offset = 0;
+ u32 val;
+ u32 len;
/*
* A source file must not combine image-data with external
@@ -1059,22 +1059,39 @@ static int fit_import_data(struct image_tool_params
*params, const char *fname)
* FIT_DATA_OFFSET_PROP and FIT_DATA_POSITION_PROP are never
both present,
* but if they are, prefer FIT_DATA_OFFSET_PROP as it was
there first
*/
- buf_ptr = fdtdec_get_int(fdt, node, FIT_DATA_POSITION_PROP, -1);
- if (buf_ptr != -1) {
+ if (!fit_image_get_data_position(fdt, node, &val)) {
ext_data_prop = FIT_DATA_POSITION_PROP;
- data = old_fdt + buf_ptr;
+ offset = val;
}
- buf_ptr = fdtdec_get_int(fdt, node, FIT_DATA_OFFSET_PROP, -1);
- if (buf_ptr != -1) {
+ if (!fit_image_get_data_offset(fdt, node, &val)) {
ext_data_prop = FIT_DATA_OFFSET_PROP;
- data = old_fdt + data_base + buf_ptr;
+ offset = data_base + (uint64_t)val;
}
- len = fdtdec_get_int(fdt, node, FIT_DATA_SIZE_PROP, -1);
- if (!data || len == -1)
+ if (!ext_data_prop || fit_image_get_data_size(fdt, node, &len))
continue;
+
+ /*
+ * The offset and size come from the FIT itself, so make
+ * sure the region lies within the mapped file before
+ * reading it. An empty region reads nothing and may lie
+ * past the end, as mkimage -E -p places it there when every
+ * image is empty.
+ */
+ if (len && (offset > (uint64_t)sbuf.st_size ||
+ len > sbuf.st_size - offset)) {
+ fprintf(stderr,
+ "Error: image '%s': external data at file
offset 0x%llx size 0x%x is outside the FIT file (size 0x%llx)\n",
+ fdt_get_name(fdt, node, NULL),
+ (unsigned long long)offset, len,
+ (unsigned long long)sbuf.st_size);
+ ret = -EINVAL;
+ goto err_munmap;
+ }
+
debug("Importing data size %x\n", len);
- ret = fdt_setprop(fdt, node, FIT_DATA_PROP, data, len);
+ ret = fdt_setprop(fdt, node, FIT_DATA_PROP, old_fdt + offset,
+ len);
if (ret) {
debug("%s: Failed to write property: %s\n", __func__,
fdt_strerror(ret));
--
2.43.0
