Re: Multiboot2 on aarch64: Alignment of ELF Headers

[Chris Plant]

On Tue, 2020-09-22 at 08:33 +0200, Krystian Hebel wrote:
> On 21.09.2020 21:14, Chris Plant wrote:
> > On Sat, 2020-05-23 at 14:33 +0200, Hans Ulrich Niedermann wrote:
> > > On Sat, 23 May 2020 12:21:27 +0100
> > > Chris Plant via Grub-devel <grub-devel@gnu.org> wrote:
> > > 
> > > > On Sat, 2020-05-23 at 12:43 +0200, Hans Ulrich Niedermann
> > > > wrote:
> > > > > On Fri, 22 May 2020 17:23:35 +0100
> > > > > Chris Plant via Grub-devel <grub-devel@gnu.org> wrote:
> > > > >    
> > > > > > I'm continuing to work on Multiboot2 support on aarch64,
> > > > > > and
> > > > > > I'm
> > > > > > looking at the alignment of the ELF headers which are
> > > > > > passed
> > > > > > through
> > > > > > MB2.
> > > > > At the risk of me sounding stupid...
> > > > > 
> > > > > Having read the MB2 specs quite thoroughly in the past few
> > > > > months, I
> > > > > still have no idea what "ELF headers" being "passed through
> > > > > MB2"
> > > > > could
> > > > > be about. The MB2 spec defines a MB2 header consisting of a
> > > > > four
> > > > > MB2
> > > > > header magic fields and a set of MB2 header tags.
> > > > > 
> > > > > The only thing ELF related I could find there is the MB2
> > > > > header
> > > > > address
> > > > > tag which duplicates some information from ELF (if OS image
> > > > > is in
> > > > > ELF format) or contains address information (if OS image is
> > > > > in
> > > > > non-ELF format). Is that what you are talking about?
> > > > Yes, this is what I'm talking about.  I'm using the MB2 tag
> > > > (type
> > > > 9)
> > > > to see where the ELF section headers are.
> > > Ah. So the type 9 you are referring to is the "ELF-Symbols" boot
> > > information tag (i.e. information the bootloader communicates to
> > > the
> > > OS
> > > image), not an MB2 header tag (information the OS image
> > > communicates
> > > to
> > > the bootloader).
> > > 
> > > I am catching up with you, finally.
> > > 
> > > > Apologies, I've not been clear enough and rambled too
> > > > much.  The
> > > > issue
> > > > I see is nothing to do with grub's MB2 implementation, but
> > > > instead
> > > > in
> > > > the way grub is loading the ELF section headers.
> > > > 
> > > > The ELF section headers appear to be loaded on a 4 byte
> > > > alignment
> > > > not
> > > > an 8 byte alignment.  You can read the 4 byte members of the
> > > > ELF
> > > > header structure fine, but not the 8 byte members as they are
> > > > aligned
> > > > to 4 bytes.
> > > > 
> > > > So, my question is, should grub load the ELF headers to an 8
> > > > byte
> > > > alignment, or should the OS be prepared for them to be aligned
> > > > to 4
> > > > byte?
> > > The 64bit part of the ELF spec appears to specifically made such
> > > that
> > > 64bit values are aligned to 64bit boundaries (I have taken a look
> > > at
> > > https://en.wikipedia.org/wiki/Executable_and_Linkable_Format for
> > > an
> > > overview).
> > > 
> > > Therefore, loading an ELF64 image with its many 64bit values to
> > > an
> > > address not aligned to 8 byte boundaries sounds to me like a
> > > probable
> > > oversight going back to ELF32 days and thus a bug for ELF64,
> > > unless
> > > someone has a very good argument to the contrary.
> > On this basis, is the logical change to fix the definition of the
> > "ELF-
> > Symbols" boot information tag, which is currently:
> > 
> > struct multiboot_tag_elf_sections
> > {
> >    multiboot_uint32_t type;
> >    multiboot_uint32_t size;
> >    multiboot_uint32_t num;
> >    multiboot_uint32_t entsize;
> >    multiboot_uint32_t shndx;
> >    char sections[0];
> > };
> > 
> > grub aligns this to an 8 byte boundary, but then the odd number of
> > uint32_t mean that sections is on a 4 byte boundary and is designed
> > within it's ELF defintion to be on 8 byte boundary.
> > 
> > So I would propose adding a 4 byte pad between shndx and sections,
> > so
> > that sections on an 8 byte boundary.  This might break things until
> > they're recompiled but won't require any changes to code elsewhere
> > and
> > I can't see how else to fix it without a new information defintion.
> This WILL break things until they are recompiled in every MB2 kernel,
> in addition to grub, every other MB2-capable bootloader and
> documentation.
> Until then, the kernel would have to somewhat guess whether it was
> given
> the old (not aligned) tag or the new one. While it may be possible as
> long
> as the padding is filled with a value that cannot appear in the first
> bytes
> of sections, the users would have to add the discovery code into
> kernels.
> MB2 is one of the most commonly used (if not the most common) boot
> protocols
> for hobby OSes.

Understood.


> 
> IMHO the only way out of it would be to create a new tag type and
> leave the
> old one as it is right now. This won't break current implementations,
> though
> it will pass redundant information. After some transition period we
> may drop
> the old format.

I'll create a new tag, struct multiboot_tag_elf64_sections which is
correctly aligned.

> > I can include this in my patch (first one, taking time and will be
> > terrible) to enable multiboot2 on ARM64 if this is the approach
> > required/desired?
> > 
> > Chris
> > 
> > > Uli
> > > 
> > > _______________________________________________
> > > Grub-devel mailing list
> > > Grub-devel@gnu.org
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 
>