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 !
