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

[Quentin Schulz <quentin.schulz@cherry.de>]

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