Re: [PATCH] ELF program header

[Stefan Reinauer]

* Robert Millan <address@hidden> [071012 18:19]:
> It seems that grub-mkimage generates awkward ELF files, in which the Program
> header table is at the end of the file instead of right after the ELF header.
> 
> 
>   - Our own ELF loader doesn't like phdroff > 0x2000 either, from
>     loader/i386/pc/multiboot.c:

I think such a fixed address makes little sense, however there are
reasons to put the phdr in front.

>   /* FIXME: Should we support program headers at strange locations?  */
>   if (ehdr->e_phoff + ehdr->e_phnum * ehdr->e_phentsize > MULTIBOOT_SEARCH)
>     return grub_error (GRUB_ERR_BAD_OS, "program header at a too high 
> offset");
> 
> This breaks self-boot in the LinuxBIOS target.  Moving the Program header
> (see attached patch) fixed it, with no apparent drawbacks or regressions in
> any of the ELF loaders around (tested on LinuxBIOS ELF loader and Efika OF).

I assume this was with LinuxBIOSv3 and Qemu?

> I'm not completely sure of its correctness though, and would appreciate if
> someone with a better understanding of ELF can comment on it.  

There's one good reason to put ELF headers in the beginning, that is
streaming of ELF files. If you get your ELF from a streaming
decompression algorithm you have to 
- either copy your elf to memory completely before loading it a second
  time
- or decompress it twice
- or you put the phdr in the beginning :-)

> I don't know if my proposed solution could overwrite valid data.  Are the
> segments garanteed to always leave room for the program header, do we
> have to explicitly check for that, or perhaps we need to relocate the segments
> when needed?

We tried the same but the patch was a lot bigger, trying to actually
leave space in the beginning. Your patch might be enough, that would be
really nice. I didn't really get the sense behind the layers of grub
magic around read() and write(). Looking at the code, it looks
completely wrong, but it surprisingly seemed to work in LBv3.

>    /* Append entire segment table to the file.  */
> -  phdroff = ALIGN_UP (grub_util_get_fp_size (out), sizeof (long));
> +  phdroff = ALIGN_UP (sizeof (ehdr), sizeof (long));
>    grub_util_write_image_at (phdrs, grub_target_to_host16 (ehdr.e_phentsize)
>                           * grub_target_to_host16 (ehdr.e_phnum), phdroff,
>                           out);

Stefan

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: address@hidden  • http://www.coresystems.de/