From eecc4148fe61a53fa1f4d16ffb453c3901fc704f Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Fri, 10 Jul 2026 15:33:39 +0000 Subject: tools: mkimage: fix stale data pointer in fit_import_data() The data pointer and the name of the external data property are declared outside the loop over the image nodes, so their 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 of the previously imported image: the previous image's data is written into the node before the import aborts when it tries to delete an external data property the node does not have. Since that abort path only prints a debug message, mkimage fails without any indication of what is wrong. The failure mode also depends on the order of the image nodes: when no externally stored image precedes the malformed node, the stale pointer is still NULL, so the import skips the node and the hashing stage reports a proper error instead. Move 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 a malformed node is always reported by the later processing stages with a proper error message. Signed-off-by: Aristo Chen Reviewed-by: Simon Glass --- tools/fit_image.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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; -- cgit v1.3.1 From 7551ce097779014405bd4976e0f9f1246dbb80d7 Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Fri, 10 Jul 2026 15:33:40 +0000 Subject: test: py: add regression test for fit_import_data() stale state Build an external-data FIT, remove the data-offset property from the second image so that only its data-size remains, and re-process the result with mkimage -F. mkimage must reject the malformed FIT with a clear diagnostic from the hashing stage. Previously the stale per-image state in fit_import_data() made the import copy the first image's data into the second image and abort without printing anything. Signed-off-by: Aristo Chen Reviewed-by: Simon Glass --- test/py/tests/test_fit_import_data.py | 89 +++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 test/py/tests/test_fit_import_data.py 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"; + }; + }; +}; +''' + + +@pytest.mark.boardspec('sandbox') +@pytest.mark.requiredtool('dtc') +@pytest.mark.requiredtool('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 -- cgit v1.3.1