Hi Tom, út 24. 6. 2025 v 14:59 odesílatel Heinrich Schuchardt <[email protected]> napsal: > > 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 <[email protected]> > > Reviewed-by: Tom Rini <[email protected]> > > --- > > 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. >
I also see random chars printed on the console on SOM with SPL flow. This patch should be reverted. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
