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

[Nix <nix at esperi dot org dot uk>]

On 17 Jul 2012, Mark Wielaard outgrape:

> On Tue, 2012-07-03 at 16:17 +0100, Nick Alcock wrote:
>> 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:

I hoped this was true, but precisely what the assertions were testing
was quite unclear to me, so I wasn't sure if removing them might not
cause different bugs to slip through which would now be detected.

>       /* 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.

Right. I wasn't sure if there was some obscure path that ensured this
would happen or not.

> Or maybe __libelf_readall () should do some more/extra work?

This would seem best if we were writing it from scratch, but since
load_shdr_wrlock() already knows how to read in the necessary data,
I don't see any reason not to allow it to do that. (At least I think
it knows how even in the non-mmap()ed case, doesn't it?)

-- 
NULL && (void)