[PATCH v4 1/1] image: fit: Apply overlays using aligned writable FDT copies

[James Hilliard]

On Thu, Feb 12, 2026 at 1:05 PM Marek Vasut <marek.vasut at mailbox.org> wrote:
>
> On 2/12/26 8:52 PM, James Hilliard wrote:
> > On Thu, Feb 12, 2026 at 8:26 AM Marek Vasut <marek.vasut at mailbox.org> wrote:
> >>
> >> On 2/11/26 7:06 PM, James Hilliard wrote:
> >>
> >>> +static int boot_get_fdt_fit_into_buffer(const void *src, ulong srclen,
> >>> +                                     ulong extra, ulong min_dstlen,
> >>> +                                     void **fdtdstbuf, ulong *fdtdstlenp)
> >>> +{
> >>> +     const void *fdtsrcbuf;
> >>> +     void *tmp = NULL;
> >>> +     void *dstbuf, *newdstbuf;
> >>> +     ulong dstlen, newdstlen;
> >>> +     int err;
> >>> +
> >>> +     /* Make sure the source FDT/DTO is 8-byte aligned for libfdt. */
> >>> +     fdtsrcbuf = src;
> >>> +     if (!IS_ALIGNED((uintptr_t)src, 8)) {
> >>> +             tmp = memalign(8, srclen);
> >>> +             if (!tmp)
> >>> +                     return -ENOMEM;
> >>> +
> >>> +             memcpy(tmp, src, srclen);
> >>> +             fdtsrcbuf = tmp;
> >>> +     }
> >>> +
> >>> +     newdstlen = ALIGN(fdt_totalsize(fdtsrcbuf) + extra, SZ_4K);
> >>> +     min_dstlen = ALIGN(min_dstlen, SZ_4K);
> >>> +     if (newdstlen < min_dstlen)
> >>> +             newdstlen = min_dstlen;
> >>> +
> >>> +     dstbuf = *fdtdstbuf;
> >>> +     dstlen = dstbuf ? *fdtdstlenp : 0;
> >>> +
> >>> +     /*
> >>> +      * If the caller already provided a large enough writable buffer,
> >>> +      * and we're not moving the FDT, nothing to do.
> >>> +      */
> >>> +     if (dstbuf && dstlen >= newdstlen && dstbuf == fdtsrcbuf) {
> >>
> >> Can $dstbuf ever be NULL ?
> >>
> >>> +             free(tmp);
> >>> +             return 0;
> >>
> >> You could try something like this here, to reduce the duplicate
> >> free(tmp) in the code:
> >>
> >> "
> >> err = 0;
> >> goto exit;
> >>
> >> ...
> >>
> >> exit:
> >>     free(tmp);
> >>     return err;
> >> "
> >>
> >>> +     }
> >>> +
> >>
> >> [...]
> >>
> >>> -     load = (ulong)of_flat_tree;
> >>> +     len = fdt_off_dt_strings(base_buf) + fdt_size_dt_strings(base_buf);
> >> How does this new length calculation work, can you please clarify that ?
> >
> >  From my understanding, that len is tracking the current packed DTB data
> > length (up to the end of the strings block), not the allocated buffer size. We
> > recompute it after each overlay so need = len + ovcopylen
> > CONFIG_SYS_FDT_PAD grows from real used bytes instead of
> > accumulated slack. This is roughly the same idea as libfdt’s internal
> > fdt_data_size_(), and the final returned size is still taken after fdt_pack()
> > with fdt_totalsize().
>
> Ah I see, thank you for the clarification, this helped. It would be nice
> to include that as code comment too.

I'll add a comment.

>
> But don't you have to account for the FDT headers too, so wouldn't
> fdt_totalsize() be more appropriate here ?

>From my understanding, fdt_off_dt_strings() + fdt_size_dt_strings() is the
packed DTB data size we need for growth calculations, and it already
covers data from the blob start (so headers are implicitly accounted for
via the offset). fdt_totalsize() includes spare/free space, so using it there
would overestimate need and cause unnecessary buffer growth.

>
> >> I don't think this will work for fitImages with external data (generated
> >> using mkimage -E) , but I might be wrong.
> >
> >  From my understanding, this should still work with mkimage -E, external
> > data only changes where FIT payload bytes are stored, and
> > fit_image_load()/fit_image_get_data() already resolves that before overlay
> > application. At that point we’re operating on a normal copied DTB buffer,
> > and the length math is based on DTB-internal layout, not FIT
> > external-data placement.
>
> Clear, you're right.