Merge patch series "tools: mkimage: fix get_basename crash on paths with dotted directories"

Aristo Chen <[email protected]> says:

The get_basename() helper in tools/fit_image.c searches the entire input
path independently for the last '/' and the last '.'. When the last '.'
falls at an offset earlier than the last '/', for example "./mydt",
"a.b/c" or "sub.d/leaf", 'end' points before 'start' and the computed
length is negative. The size check uses signed comparison so the negative
value flows unchanged into memcpy() (cast to size_t there) and mkimage
segfaults during -f auto FIT generation. The helper is reached on every
auto-FIT build via the -b, --fit-tee and --fit-tfa-bl31 file arguments.

The first patch restricts the dot search to the substring that follows
the last slash, which is the minimal fix and preserves the existing
behaviour for typical inputs such as "arch/arm/dts/foo.dtb".

The second patch adds a parametrized sandbox test under
test/py/tests/test_fit_mkimage_validate.py that drives mkimage -f auto
with each of the crashing inputs ("./mydt", "./sub.d/leaf", "./a.b/c")
plus one control input ("./mydt.dtb"). The test reads the resulting
/images/fdt-1 description back from the produced FIT via fdtget to verify
get_basename()'s output matches the expected stripped basename.

Reproducer that previously segfaulted and now produces a valid image:

  echo dummy > kernel.bin
  echo dummy > ./mydt
  ./tools/mkimage -f auto -A arm -O linux -T kernel -C none \
                  -a 0x80000000 -e 0x80000000 -n test \
                  -d kernel.bin -b ./mydt out.itb

Verified by rebuilding tools/mkimage on master and running the command
above with each of the four parametrized inputs. The three crash triggers
all segfault before the fix and now produce the expected fdt-1
descriptions ("mydt", "leaf", "c"); the control input "./mydt.dtb"
continues to produce "mydt" as before.

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

2 files changed, 65 insertions, 2 deletions

diff --git a/test/py/tests/test_fit_mkimage_validate.py b/test/py/tests/test_fit_mkimage_validate.py
index 170b2a8cbbb..5922f071dd8 100644
--- a/test/py/tests/test_fit_mkimage_validate.py
+++ b/test/py/tests/test_fit_mkimage_validate.py
@@ -7,6 +7,7 @@ import os
 import subprocess
 import pytest
 import fit_util
+import utils
 import re
 
 @pytest.mark.boardspec('sandbox')
@@ -103,3 +104,59 @@ 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]('fdtget')
[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')
+
+    cmd = ['./tools/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 both ./tools/mkimage and the relative -b
+    # path resolve the same way the bug originally reproduced.
+    result = subprocess.run(cmd, capture_output=True, text=True,
+                            cwd=build_dir)
+
+    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(). Read it back
+    # from the produced FIT (a device tree) rather than parsing mkimage's
+    # console output.
+    desc = utils.run_and_log(
+        ubman, ['fdtget', itb_fname, '/images/fdt-1', 'description']).strip()
+    assert desc == expected_desc, (
+        f"Expected /images/fdt-1 description {expected_desc!r}, got {desc!r}"
+    )
diff --git a/tools/fit_image.c b/tools/fit_image.c
index b53088bf783..5831b07c090 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -281,8 +281,14 @@ static void get_basename(char *str, int size, const char *fname)
 	 */
 	p = strrchr(fname, '/');
 	start = p ? p + 1 : fname;
-	p = strrchr(fname, '.');
-	end = p ? p : fname + strlen(fname);
+	/*
+	 * Search for the extension dot only within the basename. Searching
+	 * the whole path would let a dot in the directory part (for example
+	 * "./mydt" or "a.b/c") place 'end' before 'start' and produce a
+	 * negative length, which the size check below does not catch.
+	 */
+	p = strrchr(start, '.');
+	end = p ? p : start + strlen(start);
 	len = end - start;
 	if (len >= size)
 		len = size - 1;