Re: "ELF-Symbols" tag for relocatable images

[Daniel Kiper]

On Wed, Mar 22, 2017 at 03:50:44PM +0300, Andrei Borzenkov wrote:
> On Wed, Mar 22, 2017 at 3:07 PM, Daniel Kiper <address@hidden> wrote:
> > Hi,
> >
> > Sorry for late reply but I am busy.
> >
> > On Sun, Mar 19, 2017 at 12:02:38PM +0300, Andrei Borzenkov wrote:
> >> 17.03.2017 22:53, Ahmed, Safayet (GE Global Research, US) ??????????:
> >> > Hello again,
> >> >
> >> > I had a question on the function, "grub_multiboot_load_elf(32/64)".
> >> > (grub/grub_core/loader/multiboot_elfxx.c: line 54)
> >> >
> >> > As a part of parsing an ELF image, the above-named function copies
> >> > the section header table into memory, and copies "unloaded" sections
> >> > into memory (lines 199 - 269). The section table may be passed to an
> >> > OS image as the "ELF-Symbols" tag of the multiboot2 information
> >> > structure.
> >> >
> >> > Section 2.6.7 of the specification states that "the physical address
> >> > fields of the ELF section header then refer to where the sections are
> >> > in memory".
> >> >
> >> > Sections that are loaded are handled differently in the code from
> >> > sections that are not loaded. This distinction is made at line  234.
> >> > The loaded sections are ignored.
> >> >
> >> > The "sh_addr" field of entries in the table for not-loaded sections
> >> > are explicitly updated to point to the address where those sections
> >> > are copied (line 265).
> >> >
> >> > For "loaded" sections loaded to a fixed address, the "sh_addr" field
> >> > of the section header table entries should be accurate without any
> >> > updates. However, if the image is relocated, the "sh_addr" field of
> >> > the entries for relocated sections are not necessarily accurate.
> >
> > I am not sure that I understand this correctly. Do you mean that sh_addr
> > for a given section in memory differs from its sh_addr in the image? I think
> > that it is OK.
>
> Specification claims that sh_addr is physical section address in
> memory. If you agree that sh_addr may not match physical address,
> specification must be updated to reflect reality.

You are right, implementation deviates from spec. We can fix it but it is
not one liner fix. And, IMO, this is not critical bug. So, we should not
change this before GRUB2 2.02 release.

> > We care more about what is in the mem then in the image here.
>
> You seem to misunderstand. Before introduction of relocatable images
> ELF image was loaded at (segment) physical address, which presumably
> matches (segment) virtual address; sh_addr is virtual section address

IIRC, I have not seen physical address == virtual address assumption in
multiboot2 spec. At least in regards to ELF itself. So, I think this
should be fixed in spec.

> which in this case matches physical address memory.
>
> Relocatable image can presumably be loaded everywhere, so its load
> address != segment physical address anymore, so sh_addr also do not
> match.

Right. This begs code fix.

> They also may not match if virtual address != physical address, but as
> we do not establish any address translation when launching image, this
> probably is going to fail. Still would be good to have this assumption
> explicitly listed in multiboot2 manual.

I think that we should state in multiboot2 spec that physical address ==
virtual address in ELF.

> >> > Is this a legitimate concern?
> >>
> >> Yes. @Daniel, note that tags 9, 10 are not even documented.
> >
> > Do you mean MULTIBOOT_TAG_TYPE_ELF_SECTIONS and MULTIBOOT_TAG_TYPE_APM?
>
> No, I mean
>
> #define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64  9
> #define MULTIBOOT_HEADER_TAG_RELOCATABLE  10

They are documented in line 563 and 686.

Daniel