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

[Jonas Karlman]

Hi Mikhail,

On 2025-06-07 00:31, 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 | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 783bb84bdb5..438d1ecf124 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -703,13 +703,29 @@ 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)

Are we sure 0x0 is not a valid load address on all platforms? On what
platform are you seeing this being an issue on?

On e.g. Rockchip 0x0 is typically a valid DRAM address and could
theoretically be used as the load address in xPL, it is highly unlikely
because SPL is typically loaded to start of DRAM at 0x0.

> +		return -EIO;
>  
>  	count = info->read(info, offset, size, buf);
> +	if (!count) {
> +		/*
> +		 * The memory allocated by board_spl_fit_buffer_addr()
> +		 * should be freed. Unfortunately, we don't know what
> +		 * memory allocation mechanism was used, so we'll hope
> +		 * for the best and leave it as is.
> +		 *
> +		 * 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
> +		 */

For platforms using SPL_SYS_MALLOC_SIMPLE this comment does not fully
make sense. With SPL_SYS_MALLOC_SIMPLE no allocated memory is re-used.

To my knowledge xPL is typically running with size constrains and memory
leaking and re-use of freed memory is typically not really an issue as
main purpose of xPL is to load and jump to next phase.

On what platform have you seen the need for this memory to be freed?

Regards,
Jonas

> +		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)