On Tue, 2012-07-17 at 16:54 +0100, Nick Alcock wrote:
On 17 Jul 2012, Mark Wielaard outgrape:
> 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.
The asserts are testing that the code path will only be hit for the case
where the data isn't properly aligned or the byte order is different.
> /* 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?)
It does. But it sees elf->map_address != NULL and assumes that means all
data has already been processed (assuming alignment and byte order match
up).
Cheers,
Mark