Re: [PATCH] libfdt: Add FDT alignment check to fdt_check_header()

[Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>]

On Thu, Apr 08, 2021 at 06:36:53AM +1200, Simon Glass wrote:
> Hi Tom & Rob,
> 
> On Thu, 8 Apr 2021 at 06:09, Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> >
> > On Thu, Apr 08, 2021 at 03:35:35AM +1200, Simon Glass wrote:
> > > Hi Rob,
> > >
> > > On Wed, 7 Apr 2021 at 07:07, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > >
> > > > Only checking the FDT alignment in fdt_ro_probe_() means that
> > > > fdt_check_header() can pass, but then subsequent API calls fail on
> > > > alignment checks. Let's add an alignment check to fdt_check_header() so
> > > > alignment errors are found up front.
> > > >
> > > > Cc: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> > > > Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > > ---
> > > > For background, the new alignment check triggered a crash in the
> > > > linux kernel. Yes, we should fix the error handling, but
> > > > fdt_check_header() shouldn't tell us the FDT is valid only to fail
> > > > later on.
> > > >
> > > > Maybe we should move the check instead, but fdt_ro_probe_() and
> > > > fdt_check_header() already have a lot of the same checks.
> > > >
> > > >  libfdt/fdt.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > >
> > > At present U-Boot uses a 4-byte alignment, so far as I know, so this
> > > will break things.
> >
> > It's 8 byte, not 4 byte and I have nothing good to say about places that
> > get by with 4-and-not-8 alignment.
> 
> I am thinking of this in arch/arm/cpu/u-boot.lds :
> 
> . = ALIGN(4);
> 
> .image_copy_end : {
> 
> At least for now, we use 4-byte alignment on 32-bit ARM, for example.

Lets move that over to the U-Boot list and see what needs whacking where
then, keeping in mind that it's where we use the DTB once it's in memory
that needs 8 byte alignment, not where it might be placed in a blob for
storage.

-- 
Tom