Re: [PATCH 12/13] dlfcn,elf: implement dlmem() [BZ #11767]

[Jonathon Anderson <janderson@rice.edu>]

On Wed, 2023-03-29 at 18:51 +0500, stsp via Libc-alpha wrote:
> ```
> 
> 29.03.2023 18:45, Carlos O'Donell пишет:
> On 3/18/23 12:51, Stas Sergeev via Libc-alpha wrote:
> This patch adds the following function:
> > > void *dlmem(const unsigned char *buffer, size_t size, int flags,
> > >              struct dlmem_args *dlm_args);
> 
> > I am raising a sustained objection to including dlmem() in glibc.
> > 
> > I appreciate your effort in working on this serious, and I think *many* of
> > the core changes you propose are good cleanups.
> > 
> > In my experience it is the wrong level of abstraction.
> > 
> > To implement fdlopen on top of dlmem requires PT_LOAD processing and that
> > will duplicate into userspace a significant part of the complexity of ELF
> > loading and segment handling. The test case you include below is incomplete
> > when it comes to implementing a functional fdlopen() across the supported
> > architectures and toolchain variants.
> 
> 
> Carlos, please, be reasonable, I don't understand
> what do you want from me. :)
> Why my test-cases are incomplete?
> Why some "complete" test-case needs an elf
> parsing? Will this ever be demonstrated with
> some example or anything?
> I am really a bit tired of that permanently
> recurring argument about some elf parsing.
> I don't even know what are you talking about.
> 
> Where, just where have you seen it in my
> patch? Or if its not there, why do you mention
> it?

Stas,

Please do some research into the ELF file format. Neither your fdlopen implementation in the test cases nor your dlopen_with_offset implementation in the email chain implement it correctly.

AFAICT, the first glaring issue with both of your implementations is that you have neglected the case where p_offset != p_vaddr, i.e. a segment is mmapped to a different location than its layout in the file. There are a LOT of binaries out in the wild where this is the case. Here's a quick one-liner to help you find some on your own box, I have 11712 such binaries on my Debian system:

    find /usr/lib -type f -exec grep -aq '^.ELF' {} \; -print 2>/dev/null | while read bin; do if readelf -l $bin | grep LOAD | grep -vE 'LOAD[[:space:]]+(0x[0-9a-f]+) (\1)'; then echo $bin; fi; done

The second glaring issue (from my perspective) is that you are mmapping the entire file, instead of just the executable code. I have personally compiled up binaries where the DWARF debugging information was far larger than the code, one extreme case was a ~7.7GB binary of which merely ~130MB was .text. It is critical for performance that only that ~130MB is loaded from disk in the nominal case.

There are likely more glaring issues, these are just the two that first came to mind.

Needless to say, both of these issues require the user of dlmem() to parse the program headers of the binary and mmap() the binary according to the ELF standard. This is a complex and delicate process, and is already implemented in full in Glibc. The fact that users have to duplicate this implementation makes dlmem() a terrible API abstraction and, as others have asserted, unsuitable for inclusion Glibc.

-Jonathon