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

Simon Glass sjg at chromium.org
Mon Aug 28 19:54:48 CEST 2023


Hi Roman,

On Mon, 28 Aug 2023 at 02:00, Roman Azarenko <roman.azarenko at iopsys.eu> wrote:
>
> On Fri, 2023-08-25 at 12:06 -0600, Simon Glass wrote:
> > > @@ -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() ?
>
> The introduction of fdt_set_totalsize() comes from commit ebfe611be91e
> ("mkimage: fit_image: Add option to make fit header align"). The commit
> message doesn't describe the choice of this function vs fdt_open_into().
>
> Personally I'm unable to definitively comment on it. I can only blindly
> guess, that because we're only changing the total length of the fdt
> struct, and keeping all other fields the same, we don't need to allocate
> a new fdt struct with a different size.

I am not sure if it would cause problems. I do understand that you
didn't write the code. You could copy the people who did (and those
that reviewed it) for their input.

But I think it should change to call fdt_open_into()...if you look at
that function it does extra things. Unfortunately, the function has no
comment in libfdt.h.

Could you add another patch before or after this one?

Regards,
Simon


More information about the U-Boot mailing list