On Tue Jun 10, 2025 at 1:16 AM IST, Mikhail Kshevetskiy wrote:
> This fix a possible NULL pointer dereference.
>
> There is also a risk of memory leaking within the same portion of code.
> The leak will happen if loaded image is bad or damaged. In this case
> u-boot-spl will try booting from the other available media. Unfortunately
> resources allocated for previous boot media will NOT be freed.
>
> We can't fix that issue as the memory allocation mechanism used here
> is unknown. It can be different kinds of malloc() or something else.
>
> To somewhat reduce memory consumption, one can try to reuse previously
> allocated memory as it's done in board_spl_fit_buffer_addr() from
> test/image/spl_load.c.
>
> The corresponding comment was put to the code as well.
>
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevets...@iopsys.eu>
> ---
> common/spl/spl_fit.c | 35 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 321954a1547..3a7d753edcf 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -703,13 +703,46 @@ static int spl_simple_fit_read(struct spl_fit_info *ctx,
> */
> size = get_aligned_image_size(info, size, 0);
> buf = board_spl_fit_buffer_addr(size, size, 1);
> + if (!buf) {
> + /*
> + * We assume that non of the board will ever use 0x0 as a
s/non/none
Everything else looks good to me, thanks for the patch Mikhail.
Reviewed-by: Anshul Dalal <ansh...@ti.com>
> + * valid load address. Theoretically some board could use it,
> + * but this is extremely unlikely.
> + */
> + return -EIO;
> + }
>
> count = info->read(info, offset, size, buf);
> + if (!count) {
> + /*
> + * FIT could not be read. This means we should free the
> + * memory allocated by board_spl_fit_buffer_addr().
> + * Unfortunately, we don't know what memory allocation
> + * mechanism was used:
> + * - For the SPL_SYS_MALLOC_SIMPLE case nothing could
> + * be done. The memory just could not be freed.
> + * - For statically allocated memory buffer we can try
> + * to reuse previously allocated memory (example:
> + * board_spl_fit_buffer_addr() function from the
> + * file test/image/spl_load.c).
> + * - For normall malloc() -- the memory will be lost!
> + *
> + * Please note:
> + * - FIT images with data placed outside of the FIT
> + * structure will cause small memory leak (several
> + * kilobytes),
> + * - FIT images with data placed inside to the FIT
> + * structure may cause huge memory leak (up to
> + * several megabytes). Do NOT use such images!
> + */
> + return -EIO;
> + }
> +
> ctx->fit = buf;
> debug("fit read offset %lx, size=%lu, dst=%p, count=%lu\n",
> offset, size, buf, count);
>
> - return (count == 0) ? -EIO : 0;
> + return 0;
> }
>
> static int spl_simple_fit_parse(struct spl_fit_info *ctx)
