[PATCH v2] tools: ensure zeroed padding in external FIT images

[Simon Glass]

On Fri, 25 Aug 2023 at 02:10, Roman Azarenko <roman.azarenko at iopsys.eu> wrote:
>
> Padding the header of an external FIT image is achieved by truncating
> the existing temporary FIT file to match the required alignment before
> appending image data. Reusing an existing file this way means that the
> padding will likely contain a portion of the original data not
> overwritten by the new header.
>
> Zero out any data past the end of the new header, and stop at either
> the end of the desired padding, or the end of the old FIT file,
> whichever comes first.
>
> Fixes: 7946a814a319 ("Revert "mkimage: fit: Do not tail-pad fitImage with external data"")
> Signed-off-by: Roman Azarenko <roman.azarenko at iopsys.eu>
> ---
> v2:
>   - Use memset() on `fdt` instead of ftruncate() on `fd`, zero only
>     the part that will become padding in the new FIT file.
> v1: https://lists.denx.de/pipermail/u-boot/2023-August/528213.html
>
> I'm trying to be a bit smart here and only zero out the amount of
> padding that we actually need, but no more than the total length of the
> old FIT file (not to cross the boundary of the mmaped region).
>
> See v1 for info on how to reproduce the original issue.
> ---
>  tools/fit_image.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index 9fe69ea0d9f8..d9da0ce8388f 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -497,7 +497,7 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname)
>  {
>         void *buf = NULL;
>         int buf_ptr;
> -       int fit_size, new_size;
> +       int fit_size, unpadded_size, new_size, pad_boundary;
>         int fd;
>         struct stat sbuf;
>         void *fdt;
> @@ -564,9 +564,13 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname)
>         /* Pack the FDT and place the data after it */
>         fdt_pack(fdt);
>
> -       new_size = fdt_totalsize(fdt);
> -       new_size = ALIGN(new_size, align_size);
> +       unpadded_size = fdt_totalsize(fdt);
> +       new_size = ALIGN(unpadded_size, align_size);
>         fdt_set_totalsize(fdt, new_size);

I didn't know that was allowed...I thought it needed fdt_open_into() ?

> +       if (unpadded_size < fit_size) {
> +               pad_boundary = new_size < fit_size ? new_size : fit_size;
> +               memset(fdt + unpadded_size, 0, pad_boundary - unpadded_size);
> +       }
>         debug("Size reduced from %x to %x\n", fit_size, fdt_totalsize(fdt));
>         debug("External data size %x\n", buf_ptr);
>         munmap(fdt, sbuf.st_size);
> --
> 2.42.0
>

Reviewed-by: Simon Glass <sjg at chromium.org>