Hi Marek,
On Sun, 9 Aug 2026 at 22:42, Marek Vasut <[email protected]> wrote:
>
> On 8/9/26 6:33 PM, Simon Glass wrote:
>
> Hello Simon,
>
> >>> On Tue, 21 Jul 2026 at 13:48, Marek Vasut
> >>> <[email protected]> wrote:
> >>>>
> >>>> In case MULTI_DTB_FIT_GZIP is enabled, fdtdec_setup() does uncompress
> >>>> the compressed DTs in uncompress_blob() using gunzip(), which invokes
> >>>> malloc() internally. The early simple malloc is initialized in board_f
> >>>> initf_malloc() call, which sets up the early simple malloc limit and
> >>>> offset pointer in global data. Currently, the initf_malloc() is called
> >>>> after fdtdec_setup(), which leads to malloc failure in fdtdec_setup()
> >>>> during the gzip decompression, because the early simple malloc is not
> >>>> initialized yet.
> >>>>
> >>>> Call initf_malloc() before fdtdec_setup() to assure fdtdec_setup() can
> >>>> use malloc() during gzip decompression of the DTs.
> >>>>
> >>>> The impact of this change on boot time is negligible, because the
> >>>> initf_malloc() only assigns two fields in global data.
> >>>>
> >>>> Signed-off-by: Marek Vasut <[email protected]>
> >>>> ---
> >>>> Cc: Ilias Apalodimas <[email protected]>
> >>>> Cc: Simon Glass <[email protected]>
> >>>> Cc: Tom Rini <[email protected]>
> >>>> Cc: [email protected]
> >>>> ---
> >>>> common/board_f.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> I'm not keen on reordering this list...
> >>>
> >>> The offending call is inside uncompress_blob(), which already has a
> >>> non-malloc path - MULTI_DTB_FIT_USER_DEFINED_AREA with
> >>> MULTI_DTB_FIT_USER_DEF_ADDR.
> >>
> >> It isn't the allocation of the decompress target that is the problem, it
> >> is the gunzip() call which internally calls malloc(), cf. commit message
> >> and lib/gunzip.c gzalloc() usage.
> >>
> >>> That is the pattern most boards using
> >>> compressed multi-DTB FIT already use, and it avoids early malloc
> >>> altogether. Could the BTT config not just switch to that and drop the
> >>> board-local initf_malloc() workaround at the same time?
> >>
> >> This is unrelated to BTT config.
> >>
> >>> Failing that, the per-board workaround in board/liebherr/btt/btt.c is
> >>> ugly but localised. If we really want a generic fix, I would rather
> >>> see uncompress_blob() call initf_malloc() itself when it needs the
> >>> heap, so the ordering constraint stays local to the code that needs
> >>> it. We would need to ensure that malloc() isn't then inited a second
> >>> time. We could always add a flag to gd->boardf, I suppose.
> >>>
> >>> The reordering also means that malloc cannot be traced - the idea with
> >>> trace is that it is enabled as early as possible. Finally (that I can
> >>> think of), it means that early malloc can never be configured by the
> >>> devicetree (although that is not something we have needed yet).
> >> [...]
> >
> > Ah OK, I see. So in U-Boot proper, before relocation, you have a FIT
> > containing multiple gzip-compressed DTBs and you want to select the
> > correct one (presumably with a compatible string), then decompress and
> > use it.
> >
> > Is it possible to do this in SPL instead?
>
> No, there is no SPL on the device I have here.
>
> > If not, it looks like there are two allocations in gzip. One is just
> > its state (fixed size so we could pass it in or pass a pointer to a
> > local var). The other is its context buffer, which might be 64K or
> > more. Did you see my suggested workaround above (call initf_malloc()
> > itself)?
> I believe there are more than 2 mallocs in the gzip code. Search for
> ZALLOC() macro, that is invoked during decompression, that invokes the
> gzalloc() function from lib/gunzip.c which invokes malloc() too.
Yes you're right.
Anyway, I've spent an hour or so digging through this and looking at
the various board-specific implementations of board_fdt_blob_setup()
and I believe your change is safe. In fact board/liebherr/btt/btt.c
does something similar and with your change we can remove that
workaround. Since, as you say, initf_malloc() really only writes a few
things to gd, tracing is no great loss (really tracing should go above
fdtdec_setup() but I suspect I found that too hard when I tried). So I
agree with what Tom said too - this is a reasonable tradeoff.
I suggest adding a tag for this, or at least a link to the commit
which makes this change necessary, e.g.:
Fixes: 95f4bbd581cf ("lib: fdt: Allow LZO and GZIP DT compression in U-Boot")
Reviewed-by: Simon Glass <[email protected]>
I'm sure we could create a sandbox_noinst test for all this logic, BTW.
Regards,
Simon
