Re: [PATCH v9 0/13] implement dlmem() function

[Szabolcs Nagy <szabolcs.nagy@arm.com>]

The 04/03/2023 15:43, stsp via Libc-alpha wrote:
> 03.04.2023 15:04, Szabolcs Nagy пишет:
> > the segments of an elf file can be mmapped in many ways,
> > there is no such thing as initial mmap (there need not
> > be a mapping that covers all segments and there is no
> > ordering requirement for the mappings or which mapping
> > is file backed etc) your callback exposes a particular
> > behaviour.
> 
> I think this is a misunderstanding.
> Even before my patch there is this code in dl-load.c:

these are all *internals*, there is nothing in the
public api (or even elf spec) that says this is how
this should be done.

> So I am working on an existing frame-work only.

no, you are exposing implementation internal details.

> > and libc apis are exposed to not use locks that have
> > lock ordering requirement wrt the dynamic linker
> > internal lock. (e.g. we could add such a lock in mmap
> > or shm_open since they are not required to be as-safe.
> > so your callback can deadlock in a future glibc).
> 
> Well you probably can't add such a lock into mmap(),
> as the loader calls mmap() anyway. Premap callback
> just does the same thing.

no, the loader does not call the public mmap libc api.
it calls internal __mmap that may behave differently.

the current implementation does not impose any
requirement on the public mmap symbol.

> Do you really think some lock can be added to
> ftruncate()?

we have to reason about the constraints not second
guess what others think, we have plenty syscalls
emulated in the libc for posix conformance reasons
doing non-trivial things and various libc apis are
interposed by user code (see asan) and then they can
do whatever (e.g. asan interposes mmap, but cannot
interpose the internal __mmap).

> > and it's not just locks but any unusual libc internal
> > state that becomes observable via the callback.
> > (the callback runs when the dynamic linker may be in
> > inconsistent internal state. why do you assume that
> > shm_open or other libc apis work in that case? this
> > is a new requirement that has to be documented.)
> 
> Mainly mmap(), and the loader does mmap()s by
> himself, so I quite assume they work from the
> callback.

again, this is wrong.

> > it does not matter if it's 99% or 99.999%, what matters
> > is that if we expose an api then that has to work
> > *forever* under all potential usage and we have to
> > keep *supporting* it with all the api&abi constraints.
> 
> Would it be acceptable to settle on an agreement
> to not add a lock to ftruncate() that would prevent
> its use from a premap callback? If its not possible,
> of course there is still an option to document the
> premap callback in a way so it will have to do the
> syscalls directly. Which doesn't look like a very
> good choice, but possible.

a user callback must not run in a context where libc
internals are exposed (e.g. under internal locks).
if you do that you need very strong justification
and very careful specification of the api. (moving
the callback outside the dynlinker locks or passing
down flags instead of a callback would solve this
particular issue.)

> > > Unfortunately my setup is much more
> > > complicated than that: I need to force
> > > the mapping under 4Gb AND I need to
> > > force it into the shared buffer which I
> > > then mmap() to another address...
> > you didnt explain this in the rationale of the patches.
> 
> I mentioned such a use-case actually in the
> rationale, here's the quote:
> 
> More so, if DLMEM_DONTREPLACE flag is used, then the mapping
> established by the premap callback, will not be replaced with the
> file-backed mapping. In that case dlmem() have to use memcpy(), which
> is likely even faster than mmaps() but doesn't end up with the proper
> /proc/self/map_files or /proc/self/maps entries. So for example if the
> premap callback uses MAP_SHARED, then with the use of the DLMEM_DONTREPLACE
> flag you can get your solib relocated into a shared memory buffer.

there are so many unexplained details here.. we have
to guess what you have in mind. (should tls also end
up in shared memory? what if the libc wants to generate
executable code per module for instrumentation? e.g.
for PLT hooking, should that go to the shared mapping
too? what if the malloc implementation reuses bits of
memory from shared libs they waste due to page alignment
is that legal? we can answer these questions if you
either specify the interface contracts or the point of
all of this, but right now it's anybody's guess.)

libc will likely not include an api to load libs into
shared memory. (e.g. aarch64 has features that work on
anon or file backed memory but not on shared memory and
some systems may prevent it for security reasons, so
this has at least portability problems, but it also
clearly constrains the implementation.)

> > but it seems what you want is not suitable for libc.
> > you have to write your own loader to do this.
> I very much wish to do that!
> But so far I failed with a few naive attempts,
> like building the patched glibc statically, or
> loading custom libc.so into a separate name-space...

you can load a shared lib / generate code that does
not use normal libc shared library abi, but can
call into libc (or some other library) or can be
called from normal libraries. this is kind of what
jits do.

> I don't know what interface should be exposed
> by glibc to let my own loader to create a new
> link-map and bind symbols to it. Maybe even
> none, maybe I can do that via r_debug for example.
> So I very much wish to work in that direction, if you
> think it is a more appropriate solution that
> can be accepted into glibc (if any changes are
> at all needed).
> 
> However, dlmem() can be used for Android's
> dlopen_with_offset(), which was also rejected
> from glibc. With dlmem() its just a dozen of
> lines that never need to be in glibc itself. So
> for example the problem with dlopen_with_offset()
> could be easily settled with my dlmem().

there is probably a reason why that api got rejected
and likely the same reason applies to dlmem.