Hi Aristo,
On 5/21/26 4:35 AM, Aristo Chen wrote:
Add a parametrized regression test for the fix in the previous commit.
The test invokes mkimage in auto-FIT mode (-f auto) with a -b argument
whose directory component contains a '.' and whose leaf either lacks an
extension or is a plain identifier. Before the fix these inputs caused
get_basename() to compute a negative length and segfault inside memcpy.
The test asserts that mkimage exits successfully and that the fdt
sub-image description matches the expected stripped basename, covering
"./mydt", "./sub.d/leaf", and "./a.b/c". A control input of "./mydt.dtb"
is also exercised to confirm normal extension stripping still works.
Signed-off-by: Aristo Chen <[email protected]>
---
test/py/tests/test_fit_mkimage_validate.py | 55 ++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/test/py/tests/test_fit_mkimage_validate.py
b/test/py/tests/test_fit_mkimage_validate.py
index 170b2a8cbbb..0a1cc5963a6 100644
--- a/test/py/tests/test_fit_mkimage_validate.py
+++ b/test/py/tests/test_fit_mkimage_validate.py
@@ -103,3 +103,58 @@ def test_fit_invalid_default_config(ubman):
assert result.returncode != 0, "mkimage should fail due to missing default config"
assert re.search(r"Default configuration '.*' not found under
/configurations", result.stderr)
+
[email protected]('sandbox')
[email protected]('dtc')
[email protected]('dtb_relpath,expected_desc', [
+ # Crash triggers: last '.' precedes last '/', or leaf has no extension.
+ ('./mydt', 'mydt'),
+ ('./sub.d/leaf', 'leaf'),
+ ('./a.b/c', 'c'),
+ # Control case: extension lives in the leaf, no dotted directory.
+ ('./mydt.dtb', 'mydt'),
+])
+def test_fit_auto_basename_dotted_directory(ubman, dtb_relpath, expected_desc):
+ """Regression test: mkimage -f auto must not crash when a -b path has a
+ '.' in its directory portion.
+
+ Before the fix, get_basename() in tools/fit_image.c searched the whole
+ path for both the last '/' and the last '.'. When the '.' fell before
+ the '/', the computed length went negative and was passed unchanged to
+ memcpy(), which segfaulted. This test exercises three crashing paths
+ plus one control input.
+ """
+ build_dir = ubman.config.build_dir
+ kernel = fit_util.make_kernel(ubman, 'kernel.bin', 'kernel')
+ itb_fname = fit_util.make_fname(ubman, 'auto_basename.itb')
+
+ # Materialize the dtb at the requested relative path inside build_dir.
+ dtb_abs = os.path.join(build_dir, dtb_relpath)
+ os.makedirs(os.path.dirname(dtb_abs), exist_ok=True)
+ with open(dtb_abs, 'wb') as f:
+ f.write(b'dummy')
+
+ mkimage = os.path.join(build_dir, 'tools/mkimage')
+ cmd = [mkimage, '-f', 'auto',
+ '-A', 'arm', '-O', 'linux', '-T', 'kernel', '-C', 'none',
+ '-a', '0x80000000', '-e', '0x80000000', '-n', 'test',
+ '-d', kernel,
+ '-b', dtb_relpath,
+ itb_fname]
+ # Run with cwd=build_dir so the relative path resolves the same way
+ # the bug originally reproduced.
+ result = subprocess.run(cmd, capture_output=True, text=True,
+ cwd=build_dir)
Considering we set cwd to build_dir, can't we simply use ./tools/mkimage
instead of prepending build_dir to it? It's unclear to me if it's an
absolute path, but I'm assuming it is otherwise the test wouldn't run.
+
+ assert result.returncode == 0, (
+ f"mkimage crashed or failed on -b {dtb_relpath!r}: "
+ f"rc={result.returncode}\nstdout:\n{result.stdout}\n"
+ f"stderr:\n{result.stderr}"
+ )
+ # The fdt sub-image description is set from get_basename(); confirm it
+ # matches the expected stripped basename.
+ assert re.search(rf"Image 1
\(fdt-1\)\s+Description:\s+{re.escape(expected_desc)}\b",
+ result.stdout), (
+ f"Expected fdt-1 description {expected_desc!r} in mkimage output, "
+ f"got:\n{result.stdout}"
+ )
I can't help but wonder if we shouldn't make this less dependent on
mkimage's console output (and thus requiring a regex). FIT data
structure is simply a device tree so why not parse it to look for the
description property of the /images/fdt-1 node?
What do you think?
Looks good to me otherwise.
Cheers,
Quentin
