Merge patch series "tools: mkimage: fix stale data pointer in fit_import_data()"

Aristo Chen <[email protected]> says:

fit_import_data() in tools/fit_image.c declares the data pointer and
the name of the external data property outside its loop over the
/images subnodes, so both values leak from one image into the next. An
image node that carries data-size but neither data-offset nor
data-position then reuses the pointer left behind by the previously
imported image: the previous image's data is written into the node,
after which the import aborts trying to delete an external data
property the node never had. Since that abort path only prints a
debug() message, a regular mkimage build fails with nothing but the
generic usage text. The failure mode also depends on the order of the
image nodes: when no externally stored image precedes the malformed
node, the pointer is still NULL, the node is skipped, and the hashing
stage reports a proper error instead.

A FIT authored from a .its cannot hit this, because dtc-authored
images carry inline data. It takes re-processing an external-data FIT
in which an image has lost its data-offset, for example one edited
with fdtput -d or produced by another tool:

  mkimage -E -f demo.its demo.itb
  fdtput -d demo.itb /images/kernel-2 data-offset
  mkimage -F demo.itb

There is no silent-success path, because the failing property delete
always aborts the run before the file is written back, but the user is
left without any indication of what is wrong.

Patch 1 moves the declarations into the loop so that each image starts
from a clean state. A node without an external data reference is now
skipped consistently regardless of node order, and the later
processing stages report the malformed node with a proper error
message (Can't get image data/size).

Patch 2 adds a regression test that builds an external-data FIT,
deletes the data-offset property of the second image and re-processes
the result with mkimage -F. It asserts that mkimage fails and that the
diagnostic is present on stderr; the unfixed tool fails the second
assertion since it prints nothing beyond the usage text.

Link: https://lore.kernel.org/r/[email protected]

2 files changed, 91 insertions, 2 deletions

diff --git a/test/py/tests/test_fit_import_data.py b/test/py/tests/test_fit_import_data.py
new file mode 100644
index 00000000000..efbbad14262
--- /dev/null
+++ b/test/py/tests/test_fit_import_data.py
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2026 Canonical Ltd.
+#
+# Test mkimage import of external data in fit_import_data()
+
+"""Regression test for stale per-image state in fit_import_data().
+
+The import loop used to keep the data pointer and external property name
+of the previous image, so an image node carrying data-size but neither
+data-offset nor data-position imported the previous image's data and
+then made mkimage abort without printing any diagnostic. Such a node
+must be skipped by the import and reported by the later processing
+stages instead.
+"""
+
+import os
+import subprocess
+
+import pytest
+
+import fit_util
+
+BASE_ITS = '''
+/dts-v1/;
+
+/ {
+    description = "import-data test";
+
+    images {
+        kernel-1 {
+            description = "first kernel";
+            data = /incbin/("%(kernel1)s");
+            type = "kernel";
+            arch = "sandbox";
+            os = "linux";
+            compression = "none";
+            load = <0x40000>;
+            entry = <0x40000>;
+        };
+        kernel-2 {
+            description = "second kernel";
+            data = /incbin/("%(kernel2)s");
+            type = "kernel";
+            arch = "sandbox";
+            os = "linux";
+            compression = "none";
+            load = <0x80000>;
+            entry = <0x80000>;
+        };
+    };
+
+    configurations {
+        default = "conf-1";
+        conf-1 {
+            kernel = "kernel-1";
+        };
+    };
+};
+'''
+
+
[email protected]('sandbox')
[email protected]('dtc')
[email protected]('fdtput')
+def test_fit_import_data_missing_offset(ubman):
+    """An image with data-size but no data-offset must not inherit data"""
+    mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage')
+    params = {
+        'kernel1': fit_util.make_kernel(ubman, 'imp-kernel1.bin', 'first'),
+        'kernel2': fit_util.make_kernel(ubman, 'imp-kernel2.bin', 'second'),
+    }
+    its = fit_util.make_its(ubman, BASE_ITS, params, 'imp.its')
+    itb = fit_util.make_fname(ubman, 'imp.itb')
+
+    result = subprocess.run([mkimage, '-E', '-f', its, itb],
+                            capture_output=True, text=True)
+    assert result.returncode == 0, result.stderr
+
+    # Remove the offset so that only data-size is left on kernel-2
+    subprocess.run(['fdtput', '-d', itb, '/images/kernel-2', 'data-offset'],
+                   check=True)
+
+    # Re-processing must skip the malformed image in the import, so that
+    # the hashing stage reports it; previously the stale pointer made the
+    # import write kernel-1's data into kernel-2 and abort silently
+    result = subprocess.run([mkimage, '-F', itb],
+                            capture_output=True, text=True)
+    assert result.returncode != 0
+    assert "Can't get image data/size" in result.stderr
diff --git a/tools/fit_image.c b/tools/fit_image.c
index 5831b07c090..7e59bc43b77 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -898,8 +898,6 @@ err:
 static int fit_import_data(struct image_tool_params *params, const char *fname)
 {
 	void *fdt, *old_fdt;
-	void *data = NULL;
-	const char *ext_data_prop = NULL;
 	int fit_size, new_size, size, data_base;
 	int fd;
 	struct stat sbuf;
@@ -941,6 +939,8 @@ static int fit_import_data(struct image_tool_params *params, const char *fname)
 	for (node = fdt_first_subnode(fdt, images);
 	     node >= 0;
 	     node = fdt_next_subnode(fdt, node)) {
+		const char *ext_data_prop = NULL;
+		void *data = NULL;
 		int buf_ptr;
 		int len;