Hi Quentin,
On Mon, Sep 21, 2026 at 7:27 PM Quentin Schulz <[email protected]> wrote:
>
> Hi Alexey,
>
> On 9/18/26 12:52 PM, Alexey Charkov wrote:
> > 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
> > 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.
> >
> > Signed-off-by: Alexey Charkov <[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
> > index e02c746af5d3..9a93487bf0b2 100644
> > --- a/common/spl/spl_fit.c
> > +++ b/common/spl/spl_fit.c
> > @@ -6,7 +6,6 @@
> >
> > #include <errno.h>
> > #include <fpga.h>
> > -#include <gzip.h>
> > #include <image.h>
> > #include <log.h>
> > #include <memalign.h>
> > @@ -260,7 +259,7 @@ static int load_simple_fit(struct spl_load_info *info,
> > ulong fit_offset,
> > ulong load_addr;
> > void *load_ptr;
> > void *src;
> > - uint8_t image_comp = -1, type = -1;
> > + u8 image_comp = IH_COMP_NONE, type = IH_TYPE_INVALID;
> > const void *data;
> > const void *fit = ctx->fit;
> > bool external_data = false;
> > @@ -364,8 +363,7 @@ static int load_simple_fit(struct spl_load_info *info,
> > ulong fit_offset,
> > }
> > read_offset = fit_offset + aligned_offset;
> >
> > - if (spl_decompression_enabled() &&
>
> You remove this function in the previous patch so the previous patch is
> breaking bisectability. I'm guessing you want to replace it in the
> previous patch with CONFIG_IS_ENABLED(DECOMPRESSION).
Right, thanks for spotting that. That was a somewhat careless
fixup/autosquash combo.
> > - (image_comp == IH_COMP_GZIP || image_comp ==
> > IH_COMP_LZMA))
> > + if (image_comp != IH_COMP_NONE)
> > src_ptr = map_sysmem(ALIGN(CONFIG_SYS_LOAD_ADDR,
> > ARCH_DMA_MINALIGN), len);
> > else
> > src_ptr = map_sysmem(ALIGN(load_addr,
> > ARCH_DMA_MINALIGN), len);
> > @@ -429,25 +427,18 @@ static int load_simple_fit(struct spl_load_info
> > *info, ulong fit_offset,
> > if (CONFIG_IS_ENABLED(FIT_IMAGE_POST_PROCESS))
> > board_fit_image_post_process(fit, node, &src, &length);
> >
> > - load_ptr = map_sysmem(load_addr, length);
> > - if (IS_ENABLED(CONFIG_SPL_GZIP) && image_comp == IH_COMP_GZIP) {
> > - size = length;
> > - if (gunzip(load_ptr, CONFIG_SYS_BOOTM_LEN, src, &size)) {
> > - puts("Uncompressing error\n");
> > - return -EIO;
> > - }
> > - length = size;
> > - } else if (IS_ENABLED(CONFIG_SPL_LZMA) && image_comp == IH_COMP_LZMA)
> > {
> > - size = CONFIG_SYS_BOOTM_LEN;
> > - ulong loadEnd;
> > + if (image_comp != IH_COMP_NONE) {
> > + ulong load_end;
> >
> > - if (image_decomp(IH_COMP_LZMA, CONFIG_SYS_LOAD_ADDR, 0, 0,
> > - load_ptr, src, length, size, &loadEnd)) {
> > + load_ptr = map_sysmem(load_addr, max_size);
> > + if (image_decomp(image_comp, load_addr, (ulong)src, type,
>
> Because type is only assigned when
> if (CONFIG_IS_ENABLED(FPGA) ||
> (CONFIG_IS_ENABLED(OS_BOOT) &&
> CONFIG_IS_ENABLED(DECOMPRESSION)))
>
> we may pass type=IH_TYPE_INVALID (as initialized at the top of the
> function) to that function. I know because I reverted patch 1 in this
> series to see what it would print and it printed Invalid Image instead
> of Standalone Program. I'm not sure this is an issue, but it's for sure
> a bit odd.
I believe switching the initializer from -1 to IH_TYPE_INVALID is not
warranted after all. Without that, your test should print "Unknown
Image" instead of "Invalid Image", which probably makes more sense
(nobody said the image is invalid, because nobody checked it without
the CONFIG_IS_ENABLED(FPGA) || (CONFIG_IS_ENABLED(OS_BOOT) &&
CONFIG_IS_ENABLED(DECOMPRESSION)) combination).
I'll revert the type initialization to -1 in v3.
Best regards,
Alexey
