[PATCH v1 1/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[PATCH v1 1/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Aristo Chen]

The get_basename() helper in tools/fit_image.c searches the entire input
path for the last '/' and the last '.' independently. 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 subsequent size check uses signed comparison so
the negative value passes through unchanged, and memcpy() is then called
with that length implicitly cast to size_t, which segfaults.

Restrict the dot search to the substring that follows the last slash so
that only an extension in the filename component can become the end of
the basename. This matches the function's stated intent of stripping an
extension from the leaf, and keeps the existing behaviour for typical
inputs such as "arch/arm/dts/foo.dtb".

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

Signed-off-by: Aristo Chen <aristo.chen@canonical.com>
---
 tools/fit_image.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index 1dbc14c63e4..6c129117297 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -265,8 +265,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;
-- 
2.43.0

[PATCH v1 2/2] test/py: cover get_basename crash on paths with dotted directories

[Aristo Chen]

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 <aristo.chen@canonical.com>
---
 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)
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.requiredtool('dtc')
+@pytest.mark.parametrize('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)
+
+    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}"
+    )
-- 
2.43.0

Re: [PATCH v1 2/2] test/py: cover get_basename crash on paths with dotted directories

[Quentin Schulz]

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 <aristo.chen@canonical.com>
> ---
>   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)
> +
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.requiredtool('dtc')
> +@pytest.mark.parametrize('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

Re: [PATCH v1 1/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Quentin Schulz]

Hi Aristo,

On 5/21/26 4:34 AM, Aristo Chen wrote:
> The get_basename() helper in tools/fit_image.c searches the entire input
> path for the last '/' and the last '.' independently. 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 subsequent size check uses signed comparison so
> the negative value passes through unchanged, and memcpy() is then called
> with that length implicitly cast to size_t, which segfaults.
> 
> Restrict the dot search to the substring that follows the last slash so
> that only an extension in the filename component can become the end of
> the basename. This matches the function's stated intent of stripping an
> extension from the leaf, and keeps the existing behaviour for typical
> inputs such as "arch/arm/dts/foo.dtb".
> 
> 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
> 
> Signed-off-by: Aristo Chen <aristo.chen@canonical.com>

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

Re: [PATCH v1 2/2] test/py: cover get_basename crash on paths with dotted directories

[Aristo Chen]

Hi Quentin,

On Thu, May 21, 2026 at 5:10 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
> 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 <aristo.chen@canonical.com>
> > ---
> >   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)
> > +
> > +@pytest.mark.boardspec('sandbox')
> > +@pytest.mark.requiredtool('dtc')
> > +@pytest.mark.parametrize('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?

Thanks for the feedback! That makes sense to me, and I will prepare a v2

>
> Looks good to me otherwise.
>
> Cheers,
> Quentin

Best regards,
Aristo

Re: [PATCH v1 1/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Marek Vasut]

On 5/21/26 4:34 AM, Aristo Chen wrote:
> The get_basename() helper in tools/fit_image.c searches the entire input
> path for the last '/' and the last '.' independently. 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 subsequent size check uses signed comparison so
> the negative value passes through unchanged, and memcpy() is then called
> with that length implicitly cast to size_t, which segfaults.
> 
> Restrict the dot search to the substring that follows the last slash so
> that only an extension in the filename component can become the end of
> the basename. This matches the function's stated intent of stripping an
> extension from the leaf, and keeps the existing behaviour for typical
> inputs such as "arch/arm/dts/foo.dtb".
> 
> 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
> 
> Signed-off-by: Aristo Chen <aristo.chen@canonical.com>
> ---
>   tools/fit_image.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index 1dbc14c63e4..6c129117297 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -265,8 +265,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;

Why not call basename(3) directly in here ? Why reimplement it ?

Re: [PATCH v1 1/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Quentin Schulz]

On 5/21/26 6:09 PM, Marek Vasut wrote:
> On 5/21/26 4:34 AM, Aristo Chen wrote:
>> The get_basename() helper in tools/fit_image.c searches the entire input
>> path for the last '/' and the last '.' independently. 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 subsequent size check uses signed comparison so
>> the negative value passes through unchanged, and memcpy() is then called
>> with that length implicitly cast to size_t, which segfaults.
>>
>> Restrict the dot search to the substring that follows the last slash so
>> that only an extension in the filename component can become the end of
>> the basename. This matches the function's stated intent of stripping an
>> extension from the leaf, and keeps the existing behaviour for typical
>> inputs such as "arch/arm/dts/foo.dtb".
>>
>> 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
>>
>> Signed-off-by: Aristo Chen <aristo.chen@canonical.com>
>> ---
>>   tools/fit_image.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/fit_image.c b/tools/fit_image.c
>> index 1dbc14c63e4..6c129117297 100644
>> --- a/tools/fit_image.c
>> +++ b/tools/fit_image.c
>> @@ -265,8 +265,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;
> 
> Why not call basename(3) directly in here ? Why reimplement it ?

"""
Warning: there are two different functions basename() - see below.

[...]

Notes

There are two different versions of basename() - the POSIX version 
described above, and the GNU version, which one gets after

     #define _GNU_SOURCE         /* See feature_test_macros(7) */
     #include <string.h>

The GNU version never modifies its argument, and returns the empty 
string when path has a trailing slash, and in particular also when it is 
"/". There is no GNU version of dirname().

With glibc, one gets the POSIX version of basename() when <libgen.h> is 
included, and the GNU version otherwise.

Bugs

In the glibc implementation of the POSIX versions of these functions 
they modify their argument, and segfault when called with a static 
string like "/usr/". Before glibc 2.2.1, the glibc version of dirname() 
did not correctly handle pathnames with trailing '/' characters, and 
generated a segfault if given a NULL argument.
"""

Not very reassuring, tbh.

Quentin

Re: [PATCH v1 1/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Marek Vasut]

On 5/21/26 6:12 PM, Quentin Schulz wrote:

Hello Quentin,

>>> +    p = strrchr(start, '.');
>>> +    end = p ? p : start + strlen(start);
>>>       len = end - start;
>>>       if (len >= size)
>>>           len = size - 1;
>>
>> Why not call basename(3) directly in here ? Why reimplement it ?
> 
> """
> Warning: there are two different functions basename() - see below.
> 
> [...]
> 
> Notes
> 
> There are two different versions of basename() - the POSIX version 
> described above, and the GNU version, which one gets after
> 
>      #define _GNU_SOURCE         /* See feature_test_macros(7) */
>      #include <string.h>
> 
> The GNU version never modifies its argument, and returns the empty 
> string when path has a trailing slash, and in particular also when it is 
> "/". There is no GNU version of dirname().
> 
> With glibc, one gets the POSIX version of basename() when <libgen.h> is 
> included, and the GNU version otherwise.
> 
> Bugs
> 
> In the glibc implementation of the POSIX versions of these functions 
> they modify their argument, and segfault when called with a static 
> string like "/usr/". Before glibc 2.2.1, the glibc version of dirname() 
> did not correctly handle pathnames with trailing '/' characters, and 
> generated a segfault if given a NULL argument.
> """
> 
> Not very reassuring, tbh.

Writing our own variant with its own set of bugs is worse than using a 
common implementation which has the bugs removed over time due to effort 
of the various users.

If basename(3) is not an option, is there another common alternative ?

Re: [PATCH v1 1/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Aristo Chen]

On Fri, May 22, 2026 at 12:17 AM Marek Vasut <marek.vasut@mailbox.org> wrote:
>
> On 5/21/26 6:12 PM, Quentin Schulz wrote:
>
> Hello Quentin,
>
> >>> +    p = strrchr(start, '.');
> >>> +    end = p ? p : start + strlen(start);
> >>>       len = end - start;
> >>>       if (len >= size)
> >>>           len = size - 1;
> >>
> >> Why not call basename(3) directly in here ? Why reimplement it ?
> >
> > """
> > Warning: there are two different functions basename() - see below.
> >
> > [...]
> >
> > Notes
> >
> > There are two different versions of basename() - the POSIX version
> > described above, and the GNU version, which one gets after
> >
> >      #define _GNU_SOURCE         /* See feature_test_macros(7) */
> >      #include <string.h>
> >
> > The GNU version never modifies its argument, and returns the empty
> > string when path has a trailing slash, and in particular also when it is
> > "/". There is no GNU version of dirname().
> >
> > With glibc, one gets the POSIX version of basename() when <libgen.h> is
> > included, and the GNU version otherwise.
> >
> > Bugs
> >
> > In the glibc implementation of the POSIX versions of these functions
> > they modify their argument, and segfault when called with a static
> > string like "/usr/". Before glibc 2.2.1, the glibc version of dirname()
> > did not correctly handle pathnames with trailing '/' characters, and
> > generated a segfault if given a NULL argument.
> > """
> >
> > Not very reassuring, tbh.
>
> Writing our own variant with its own set of bugs is worse than using a
> common implementation which has the bugs removed over time due to effort
> of the various users.
>
> If basename(3) is not an option, is there another common alternative ?

I agree with the principle, but IMO get_basename() is not really a
reimplementation of basename(3). It does two things, and basename(3)
covers only one of them.

get_basename() strips the directory and the extension. Per its own comment:

"arch/arm/dts/sun7i-a20-bananapro.dtb"
becomes "sun7i-a20-bananapro"

basename(3) on that path returns "sun7i-a20-bananapro.dtb"; the
extension stays on. So basename(3) would replace only the directory
half, the strrchr('/') scan. The extension half, the strrchr('.') scan
that this crash actually came from, still has to be done by hand.

There is no libc one-shot for "basename without the extension".
U-Boot's own host tooling reflects that: binman does it in two steps
wherever it needs a stem, for example in tools/binman/control.py:

os.path.splitext(os.path.basename(item))[0]
basename() followed by splitext(), because no single call does both.

basename(3) also carries the two-version problem Quentin already
quoted. For mkimage specifically: tools/Makefile builds every host
tool with -D_GNU_SOURCE, and fit_image.c includes <string.h> but not
<libgen.h>. That combination resolves to the GNU basename(), the
const-safe one that leaves its argument alone. But the selection is
implicit: if anyone later adds #include <libgen.h> to this file,
basename() silently becomes the POSIX version that modifies its
argument and segfaults on string literals. That is a sharper footgun
than the small hand-rolled helper, and it would sit in the very
function we are trying to make safe.

So my preference is to keep get_basename() hand-rolled. This patch is
not new logic; it only bounds the existing strrchr('.') scan to the
part after the last '/', which is the minimal fix for the
negative-length crash.

If you would still rather use basename(3), the least fragile form is
GNU basename() for the leaf, then strrchr('.') on that leaf to drop
the extension, with a comment pinning the GNU-version dependency so
nobody includes <libgen.h> later. Either way I will send a v2 (there
are review comments on patch 2/2 to address as well), so just let me
know which direction you prefer for get_basename() and I will include
it there.

Thanks,
Aristo

[PATCH v2 0/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Aristo Chen]

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.

Changes in v2:

Patch 1/2 is unchanged code-wise and picks up Quentin Schulz's
Reviewed-by from the v1 thread. Marek Vasut asked on that thread whether
basename(3) could replace the helper; the in-thread response noted that
get_basename() also strips the extension and therefore only half of it
overlaps with basename(3), and that the GNU vs POSIX basename() selection
in tools/ is implicit (it relies on _GNU_SOURCE being defined globally
and on <libgen.h> not being included), so v2 keeps the hand-rolled
approach.

Patch 2/2 now reads the fdt sub-image description back from the produced
FIT via fdtget instead of regex-matching mkimage's console output, and
invokes ./tools/mkimage rather than building an absolute path now that
the test runs with cwd=build_dir.

v1: https://patchwork.ozlabs.org/project/uboot/patch/20260521023503.29315-1-aristo.chen@canonical.com/

Aristo Chen (2):
  tools: mkimage: fix get_basename crash on paths with dotted
    directories
  test/py: cover get_basename crash on paths with dotted directories

 test/py/tests/test_fit_mkimage_validate.py | 57 ++++++++++++++++++++++
 tools/fit_image.c                          | 10 +++-
 2 files changed, 65 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Aristo Chen]

The get_basename() helper in tools/fit_image.c searches the entire input
path for the last '/' and the last '.' independently. 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 subsequent size check uses signed comparison so
the negative value passes through unchanged, and memcpy() is then called
with that length implicitly cast to size_t, which segfaults.

Restrict the dot search to the substring that follows the last slash so
that only an extension in the filename component can become the end of
the basename. This matches the function's stated intent of stripping an
extension from the leaf, and keeps the existing behaviour for typical
inputs such as "arch/arm/dts/foo.dtb".

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

Signed-off-by: Aristo Chen <aristo.chen@canonical.com>
Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 tools/fit_image.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index 1dbc14c63e4..6c129117297 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -265,8 +265,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;
-- 
2.43.0

[PATCH v2 2/2] test/py: cover get_basename crash on paths with dotted directories

[Aristo Chen]

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 <aristo.chen@canonical.com>
---
 test/py/tests/test_fit_mkimage_validate.py | 57 ++++++++++++++++++++++
 1 file changed, 57 insertions(+)

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)
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.requiredtool('dtc')
+@pytest.mark.requiredtool('fdtget')
+@pytest.mark.parametrize('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}"
+    )
-- 
2.43.0

Re: [PATCH v2 0/2] tools: mkimage: fix get_basename crash on paths with dotted directories

[Tom Rini]

On Tue, 26 May 2026 07:03:31 +0000, Aristo Chen wrote:

> 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.
> 
> [...]

Applied to u-boot/next, thanks!

[1/2] tools: mkimage: fix get_basename crash on paths with dotted directories
      commit: 759968136d68ba178904313c38ad1003525c58ac
[2/2] test/py: cover get_basename crash on paths with dotted directories
      commit: 4a4452a03916d8687442b1d0af5098be986e439e
-- 
Tom