[BUG] fdt_pack_reg in common/fdt_support.c can cause crash from unaligned access

[Simon Glass]

Hi,

On Mon, 10 Jul 2023 at 14:13, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Jul 10, 2023 at 01:45:46PM -0600, Simon Glass wrote:
> > Hi David,
> >
> > On Sun, 9 Jul 2023 at 19:11, David Virag <virag.david003 at gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > I'm trying to port U-Boot to a new board (Samsung JACKPOTLTE, ARMv8,
> > > Exynos7885) but when CONFIG_ARCH_FIXUP_FDT_MEMORY is enabled, the bootm
> > > command leads to an unaligned memory access, which results in a
> > > synchronous abort.
> > >
> > > After a long debugging session, I concluded that fdt_pack_reg in
> > > common/fdt_support.c writes to unaligned addresses in its for loop.
> > > In the case of address_cells being 2, and size_cells being 1, the
> > > buffer pointer gets incremented by 12 in each loop, making the second
> > > iteration (i=1) write a 64bit value to a non 64bit aligned address.
> > >
> > > Turning the alignment check enable bit (A) off in SCTLR makes the
> > > function work as intended. I couldn't find code that touches this bit,
> > > but I may have missed something. I don't think writing in two parts
> > > should be the fix, but something should be done about this. As far as I
> > > understand, any arm64 board that has this bit turned on, either from
> > > previous code or just the initial status of the bit after power on,
> > > could crash here.
> > >
> > > This is on top of the latest commit as of now
> > > (0beb649053b86b2cfd5cf55a0fc68bc2fe91a430)
> > >
> > > What should be done here?
> >
> > +Tom Rini
>
> ... I was hoping you had an idea Simon. Is this part of the code we
> share with libfdt itself, or one of the helpers we made?

Hmmm, is the DT itself 64-bit aligned? It needs to be.

Looking at fdt_find_separate() it needs _end

Looking at arch/arm/cpu/armv8/u-boot.lds I don't see an ALIGN before _end.

So perhaps that is the problem?

Regards,
Simon