Hi, On Tue, 10 Feb 2026 at 13:04, Marek Vasut <[email protected]> wrote: > > On 2/10/26 7:54 PM, James Hilliard wrote: > > libfdt expects FDT/DTO blobs to be 8-byte aligned. When loading the > > base FDT or overlays from a FIT, the mapped buffer may be unaligned, > > which can break fdt_open_into() on strict-alignment architectures. > > > > boot_get_fdt_fit() relocates the base FDT with boot_relocate_fdt() > > before applying overlays. That uses the bootm memory map and can > > overlap with the FIT buffer when the FIT is loaded into RAM, > > corrupting data needed to load the kernel and ramdisk. > > > > Allocate writable, 8-byte aligned copies of the base FDT and overlays > > with memalign() and fdt_open_into(). Grow the base buffer as needed, > > apply overlays to it and pack the final tree. Free each temporary > > overlay copy after application and check fdt_pack() errors. > > > > Fixes: 8fbcc0e0e839 ("boot: Assure FDT is always 8-byte aligned") > > Fixes: 881f0b77dc8c ("image: apply FDTOs on FDT image node") > > Signed-off-by: James Hilliard <[email protected]> > > --- > > Changes v1 -> v2: > > - also fix alignment issues > > Please keep the Jamie on CC > > > --- > > boot/image-fit.c | 158 +++++++++++++++++++++++++++++++++++------------ > > 1 file changed, 118 insertions(+), 40 deletions(-) > > > > diff --git a/boot/image-fit.c b/boot/image-fit.c > > index 41ab1f552b0..68fb75c8389 100644 > > --- a/boot/image-fit.c > > +++ b/boot/image-fit.c > > @@ -2344,6 +2344,52 @@ int boot_get_setup_fit(struct bootm_headers *images, > > uint8_t arch, > > } > > > > #ifndef USE_HOSTCC > > +#ifdef CONFIG_OF_LIBFDT_OVERLAY > > +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; > > + ulong fdtdstlen; > > + 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; > > + } > > + > > + fdtdstlen = ALIGN(fdt_totalsize(fdtsrcbuf) + extra, SZ_4K); > > + min_dstlen = ALIGN(min_dstlen, SZ_4K); > > + if (fdtdstlen < min_dstlen) > > + fdtdstlen = min_dstlen; > > + > > + *fdtdstbuf = memalign(8, fdtdstlen); > > + if (!*fdtdstbuf) { > > + free(tmp); > > + return -ENOMEM; > > + } > > + > > + err = fdt_open_into(fdtsrcbuf, *fdtdstbuf, fdtdstlen); > > + free(tmp); > > + if (err < 0) { > > + free(*fdtdstbuf); > > + *fdtdstbuf = NULL; > > + return err; > > + } > > + > > + *fdtdstlenp = fdtdstlen; > > + > > + return 0; > > +} > > +#endif > > + > > int boot_get_fdt_fit(struct bootm_headers *images, ulong addr, > > const char **fit_unamep, const char **fit_uname_configp, > > int arch, ulong *datap, ulong *lenp) > > @@ -2356,18 +2402,12 @@ int boot_get_fdt_fit(struct bootm_headers *images, > > ulong addr, > > char *next_config = NULL; > > ulong load, len; > > #ifdef CONFIG_OF_LIBFDT_OVERLAY > > - ulong ovload, ovlen, ovcopylen; > > + ulong ovload, ovlen, ovcopylen, need; > > const char *uconfig; > > const char *uname; > > - /* > > - * of_flat_tree is storing the void * returned by map_sysmem, then its > > - * address is passed to boot_relocate_fdt which expects a char ** and > > it > > - * is then cast into a ulong. Setting its type to void * would require > > - * to cast its address to char ** when passing it to > > boot_relocate_fdt. > > - * Instead, let's be lazy and use void *. > > - */ > > - char *of_flat_tree; > > - void *base, *ov, *ovcopy = NULL; > > + void *ovcopy = NULL; > > + void *base_buf = NULL; > > + ulong base_buf_size = 0; > > int i, err, noffset, ov_noffset; > > #endif > > > > @@ -2410,18 +2450,27 @@ int boot_get_fdt_fit(struct bootm_headers *images, > > ulong addr, > > /* we need to apply overlays */ > > > > #ifdef CONFIG_OF_LIBFDT_OVERLAY > > - /* Relocate FDT so resizing does not overwrite other data in FIT. */ > > - of_flat_tree = map_sysmem(load, len); > > - len = ALIGN(fdt_totalsize(load), SZ_4K); > > - err = boot_relocate_fdt(&of_flat_tree, &len); > > - if (err) { > > - printf("Required FDT relocation for applying DTOs failed: > > %d\n", > > - err); > > - fdt_noffset = -EBADF; > > + /* > > + * Make a writable copy of the base FDT for applying overlays. > > + * > > + * Do not use boot_relocate_fdt() here: it allocates from the bootm > > map and > > + * may overlap with the FIT buffer (still needed to load the kernel / > > + * ramdisk) when the FIT is loaded into RAM. > > + */ > > + err = boot_get_fdt_fit_into_buffer(map_sysmem(load, len), len, > > + CONFIG_SYS_FDT_PAD, 0, &base_buf, > > + &base_buf_size); > > + if (err < 0) { > > + if (err == -ENOMEM) > > + printf("Required FDT copy for applying DTOs failed: > > out of memory\n"); > > If this returns ENOMEM, you will likely not be able to print anything, > so don't bother. Bail with a return value and be done with it, i.e. drop > the error prints here. > > > + else > > + printf("Required FDT copy for applying DTOs failed: > > %s\n", > > + fdt_strerror(err)); > > + fdt_noffset = err; > > goto out; > > } > > > > - load = (ulong)of_flat_tree; > > + len = fdt_off_dt_strings(base_buf) + fdt_size_dt_strings(base_buf); > > > > /* apply extra configs in FIT first, followed by args */ > > for (i = 1; ; i++) { > > @@ -2465,46 +2514,73 @@ int boot_get_fdt_fit(struct bootm_headers *images, > > ulong addr, > > } > > debug("%s loaded at 0x%08lx len=0x%08lx\n", > > uname, ovload, ovlen); > > - ov = map_sysmem(ovload, ovlen); > > - > > - ovcopylen = ALIGN(fdt_totalsize(ov), SZ_4K); > > - ovcopy = malloc(ovcopylen); > > - if (!ovcopy) { > > - printf("failed to duplicate DTO before > > application\n"); > > - fdt_noffset = -ENOMEM; > > - goto out; > > - } > > - > > - err = fdt_open_into(ov, ovcopy, ovcopylen); > > + err = boot_get_fdt_fit_into_buffer(map_sysmem(ovload, ovlen), > > + ovlen, 0, 0, &ovcopy, > > + &ovcopylen); > > if (err < 0) { > > - printf("failed on fdt_open_into for DTO\n"); > > + if (err == -ENOMEM) > > + printf("failed to duplicate DTO before > > application\n"); > > + else > > + printf("failed on fdt_open_into for DTO: > > %s\n", > > DTTO > > > + fdt_strerror(err)); > > fdt_noffset = err; > > goto out; > > } > > > > - base = map_sysmem(load, len + ovlen); > > - err = fdt_open_into(base, base, len + ovlen); > > - if (err < 0) { > > - printf("failed on fdt_open_into\n"); > > - fdt_noffset = err; > > - goto out; > > + /* > > + * Ensure the base FDT buffer is open and has enough room for > > + * the overlay. Grow it on demand. > > + */ > > + need = ALIGN(len + ovcopylen + CONFIG_SYS_FDT_PAD, SZ_4K); > > + if (need > base_buf_size) { > > Doesn't boot_get_fdt_fit_into_buffer() do the necessary > rounding/alignment/... already , so shouldn't it be enough to simply > call it here, without this conditional ? > > > + void *new_base; > > + ulong new_size; > > + > > + err = boot_get_fdt_fit_into_buffer(base_buf, > > base_buf_size, > > + 0, need, &new_base, > > + &new_size); > > + if (err < 0) { > > + if (err == -ENOMEM) > > + printf("failed to expand FDT for DTO > > application\n"); > > + else > > + printf("failed on fdt_open_into while > > expanding FDT\n"); > > + fdt_noffset = err; > > + goto out; > > + } > > + > > + free(base_buf); > > + base_buf = new_base; > > + base_buf_size = new_size; > > } > The rest looks good, thanks !
At some point I requested or suggested that we add a function to allocate aligned space for a devicetree. If that was done then this function should not be needed. If not, then the other code (which does this in an ad-hoc manner) should be cleaned up to use this function. Regards, Simon
