On Fri, Jun 06, 2025 at 10:35:22PM +0300, 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.
>
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevets...@iopsys.eu>
> ---
> 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++;
> }Looking at the loop, I'm not sure why we're doing what we're doing here. It's possible I'm missing something else here, but, fdt_getprop is going to return us the data part of an fdt_property struct. When index is 0, the normal case, we don't loop here at all? Then if the index is 1, my head starts hurting. It's unclear if the case where we don't have a string value returned but find the node works, or maybe it does because we always try the next hunk of code? But I think we should try and clean up what's done here now that we're looking at it right now. Thanks. -- Tom
signature.asc
Description: PGP signature
