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

[Mark Wielaard <mjw at redhat dot com>]

On Thu, 2012-07-19 at 11:56 +0200, Mark Wielaard wrote:
> On Wed, 2012-07-18 at 16:17 -0700, Roland McGrath wrote:
> > This seems vaguely reasonable to me, but OTOH since in the mmap case we do
> > eagerly set up the shdr pointers in file_read_elf, I think it makes sense
> > to be consistent and do that in __libelf_readall too.  It should also save
> > a little memory to not malloc the ehdr and shdr copies.  That means that
> > __libelf_readall really ought to free the old ehdr copy (which is always
> > cached) and the shdr copies if they were cached before.  Most of the code
> > can just be broken out of file_read_elf into a subroutine,
> > e.g. __libelf_maybe_precache_headers.  __libelf_readall will just need to
> > free the old state and clear the shdr_malloced flag.
> 
> I thought about that, but was afraid about other (concurrent) users of
> the already allocated data if we freed it and swapped in a new mmaped
> version. So I decided to just keep what we have and only copy in the new
> shdr data in load_shdr_wrlock () when requested. Do you think the memory
> savings are significant enough to risk data races? It can probably be
> done safely, but does require careful analysis for IMHO little gain.

Also note the somewhat related item in the TODO file:

** shdrs in read-only files

   When reading (ELF_C_READ*) then there is no need to malloc Shdr
   structure in elfXX_getshdr if file is mmaped and unaligned access
   is allowed or the structure is aligned.  Use ELF_F_MALLOCED flag
   to differentiate.