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

[Marek Vasut]

On 2/12/26 10:32 PM, James Hilliard wrote:
> 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.
Thank you for spelling that out. Please also add that as a code comment, 
this really helps understand the code.

With that in place, for V6, add my:

Reviewed-by: Marek Vasut <marek.vasut at mailbox.org>

Thank you for your hard work.