Re: elf_cntl(ELF_C_FDREAD) breaks elf64_getshdr() with non-mmap()ed ELF files

[Mark Wielaard <mjw at redhat dot com>]

On Tue, 2012-07-03 at 16:17 +0100, Nick Alcock wrote:
> I'm seeing a problem with all versions of elfutils from 0.152 up to
> latest git head (a problem probably originating in 2007).
> 
> The symptoms are that using elf_cntl(..., ELF_C_READ, NULL) to pull a
> whole ELF file into memory so you can close it causes assertion failures
> inside elf64_getshdr() and gelf_getshdr():
> 
> scnwalk: elf32_getshdr.c:95: load_shdr_wrlock: Assertion `ehdr->e_ident[5] != 1 || (! 1 && ((uintptr_t) file_shdr & (__alignof__ (Elf64_Shdr) - 1)) != 0)' failed.
> 
> These failures do not occur if you haven't used elf_cntl(), or if you
> have used ELF_C_READ_MMAP rather than ELF_C_READ at elf_begin() time.
> 
> Testcase below. It looks to me like elf_begin.c:file_read_elf() is
> concluding that the ELF file will require increased alignment to use (so
> cannot be directly mapped), following which
> elf32_getshdr.c:load_shdr_wrlock() concludes that it does *not* require
> increased alignment, thus the assertion failure. These are perfectly
> normal object files compiled on the same architecture, no byteswapping
> needed and one would assume no alignment increase either: indeed you can
> see this failure when you try to work over the very object files that
> comprise the version of elfutils being tested.

The asserts seem too aggressive/wrong in this case. The following
comment in elf32_getshdr.c (load_shdr_wrlock) explains them:

      /* All the data is already mapped.  If we could use it
         directly this would already have happened.  */

But in the case of elf_cntl (ELF_CREAD) this has not yet happened,
unless you already accessed the shdrs before that call.

So just removing the asserts will make everything work magically. But
maybe there is an opportunity for some optimization if we detect
elf->map_address != NULL && elf->flags & ELF_F_MALLOCED.
Or maybe __libelf_readall () should do some more/extra work?

Thanks,

Mark