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

[Marek Vasut]

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 <james.hilliard1 at gmail.com>
> ---
> 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 !