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 <[email protected]>
---
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
