[PATCH v3 3/3] common/spl: improve error handling in spl_fit

[Anshul Dalal]

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.kshevetskiy at 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 <anshuld at 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)