Hi Alexey,

On 2026-09-18T10:52:27, Alexey Charkov <[email protected]> wrote:
> spl: fit: Decompress any supported algorithm
>
> load_simple_fit() open-codes gzip and lzma in two places, so a FIT built
> with any of the other five algorithms image_decomp() implements is

I count four others (bzip2, lzo, lz4 and zstd), and bzip2 is still not
reachable here since SPL_BZIP2 does not select SPL_DECOMPRESSION

> silently copied to its load address without decompression and jumped into,
> causing a hang with nothing on the console to aid debugging.
>
> Call image_decomp() instead, which can handle all compression algorigthms
> enabled in the config, and will abort on an unknown one instead of trying
> to silently jump into it.

'algorithms'

This only holds when the phase has at least one decompressor enabled.
Without CONFIG_IS_ENABLED(DECOMPRESSION) the 'compression' property is
never read, so image_comp stays at IH_COMP_NONE and a compressed image
is still copied and jumped into with nothing on the console. Please
can you mention that limitation here? Another option would be to check
the property in that case and fail if it is anything other than
'none', although that costs a little code size. What do you think?

>
> Signed-off-by: Alexey Charkov <[email protected]>
> Reviewed-by: Tom Rini <[email protected]>
>
> common/spl/spl_fit.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)

> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> @@ -429,25 +427,18 @@ static int load_simple_fit(struct spl_load_info *info, 
> ulong fit_offset,
> +             load_ptr = map_sysmem(load_addr, max_size);
> +             if (image_decomp(image_comp, load_addr, (ulong)src, type,
> +                              load_ptr, src, length, max_size, &load_end)) {

The old code passed CONFIG_SYS_BOOTM_LEN as the output limit for both
gzip and lzma, regardless of the caller. With max_size, a compressed
overlay is now limited to CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY_BUF_SZ
rather than being able to run off the end of the heap buffer. That is
a good fix, but it is a behaviour change which the commit message does
not mention. Please can you add a sentence about it?

The image_start parameter is an address, not a pointer, and src is a
host pointer on sandbox. It is only used to decide whether to print
'XIP' for the uncompressed case, so it has no effect here. Please can
you use map_to_sysmem(src), or just pass 0 as the old code did?

Regards,
Simon

Reply via email to