On 10.06.25 11:56, Mikhail Kshevetskiy wrote:
The current code have two issues:
1) ineffective NULL pointer check
str = strchr(str, '\0') + 1
if (!str || ...
The str here will never be NULL (because we add 1 to result of strchr())
2) strchr() may go out of the buffer for the special forms of name variable.
It's better use memchr() function here.
According to the code the property is a sequence of C-string like
shown below:
'h', 'e', 'l', 'l', 'o', '\0', 'w', 'o', 'r', 'l', 'd', '\0', '!', '\0'
index is the string number we are interested, so
index = 0 => "hello",
index = 1 => "world",
index = 2 => "!"
The issue will arrise if last string for some reason have no terminating
'\0' character. This can happen for damaged or specially crafted dtb.
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevets...@iopsys.eu>
Reviewed-by: Tom Rini <tr...@konsulko.com>
---
common/spl/spl_fit.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 86506d6905c..ab277bb2baa 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -86,11 +86,12 @@ static int spl_fit_get_image_name(const struct spl_fit_info
*ctx,
str = name;
for (i = 0; i < index; i++) {
- str = strchr(str, '\0') + 1;
- if (!str || (str - name >= len)) {
+ str = memchr(str, '\0', name + len - str);
+ if (!str) {
found = false;
break;
}
+ str++;
}
if (!found && CONFIG_IS_ENABLED(SYSINFO) && !sysinfo_get(&sysinfo)) {
Since this patch (commit 3704b888a4cabac8) on the Milk-V Mars board I
see a message "cannot find image node ''" and on some builds I have seen
boot failures.
With your patch and some debug messages:
U-Boot SPL 2025.07-rc4-00017-g3704b888a4ca-dirty (Jun 24 2025 - 14:45:11
+0200)
DDR version: dc2e84f0.
Trying to boot from MMC2
spl_fit_get_image_name(): index = 0, name = 'opensbi' len = 8
spl_fit_get_image_name(): outname = 'opensbi'
spl_fit_get_image_name(): index = 0, name = 'uboot' len = 6
spl_fit_get_image_name(): outname = 'uboot'
spl_fit_get_image_name(): index = 0, name = 'fdt-2' len = 6
spl_fit_get_image_name(): outname = 'fdt-2'
spl_fit_get_image_name(): index = 0, name = 'uboot' len = 6
spl_fit_get_image_name(): outname = 'uboot'
spl_fit_get_image_name(): index = 0, name = 'uboot' len = 6
spl_fit_get_image_name(): outname = 'uboot'
spl_fit_get_image_name(): index = 1, name = 'uboot' len = 6
spl_fit_get_image_name(): outname = ''
cannot find image node '': -1
Before that patch:
U-Boot SPL 2025.07-rc4-00017-g3704b888a4ca-dirty (Jun 24 2025 - 14:34:50
+0200)
DDR version: dc2e84f0.
Trying to boot from MMC2
spl_fit_get_image_name(): index = 0, name = 'opensbi' len = 8
spl_fit_get_image_name(): outname = 'opensbi'
spl_fit_get_image_name(): index = 0, name = 'uboot' len = 6
spl_fit_get_image_name(): outname = 'uboot'
spl_fit_get_image_name(): index = 0, name = 'fdt-2' len = 6
spl_fit_get_image_name(): outname = 'fdt-2'
spl_fit_get_image_name(): index = 0, name = 'uboot' len = 6
spl_fit_get_image_name(): outname = 'uboot'
spl_fit_get_image_name(): index = 0, name = 'uboot' len = 6
spl_fit_get_image_name(): outname = 'uboot'
spl_fit_get_image_name(): index = 1, name = 'uboot' len = 6
no string for index 1
With your patch spl_fit_get_image_name() does not detect that there is
no property at index 1 and does not return -E2BIG. Instead it signals
the property is an empty string and returns 0 which is not expected.
Best regards
Heinrich
