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

[stsp <stsp2@yandex.ru>]

Hi,

13.04.2023 17:38, Szabolcs Nagy пишет:
> it does not allow handling relocation failures sensibly,
> does not specify what happens with dependencies and
> exposes half loaded modules to users with unclear use-case
> or semantics for them.

Thanks, lets update the spec to address this:

RTLD_NORELOC - do not relocate the object, allowing
to perform the relocation later. Ctors are delayed, and
are called immediately after the relocation is done.
Relocation is performed upon the first operation on
the returned handle, before the requested operation
on a handle is performed, with the exception of
dlset_object_baseaddr() which doesn't perform the
relocation. If this flag is used, it is recommended to
use dlrelocate() to explicitly perform the relocation
and check the result of it. This flag doesn't delay
the load of an object deps and the calls to their ctors.
It also doesn't delay the LA_ACT_CONSISTENT
audit event.

dlset_object_baseaddr(handle, addr) - sets the base
address of an unrelocated object. Returns error if
the object is already relocated. The base address
set by this function, will be used when relocation
is performed.

dlrelocate(handle) - perform the object relocation
if the RTLD_NORELOC flag was used, return EINVAL
if already relocated. This function may be omitted
even if RTLD_NORELOC was used, in which case
the relocation will be performed upon the first
operation on a handle, but using that function
allows to handle relocation errors and run ctors
before using the object's handle. If the function
returned success then ctors of an object were
called by it. If it returned error other than EINVAL
(EINVAL means object already relocated), then
relocation error happened and the handle should
be closed with dlclose().


It seems that spec covers all of your concerns.
It does cover even the exposure of "half-loaded
modules" by stating that the relocation may
happen lazily so things would work as expected
in any case.

> if you fix an actual bug that's accepted.

I believe this classifies as a fix:
https://patchwork.ozlabs.org/project/glibc/patch/20230301150818.31815-2-stsp2@yandex.ru/

> you are adding a new interface without gathering consensus
> that it's a good idea at all or how it should work.

I can get such a consensus only if people,
like you do (many thanks for that!), explain
the problems to me and suggest an amendments.
Unfortunately, its not the case. People are
saying that my proposal is bad, and the only
suggested amendment is to drop it immediately. :)

But I am very thankful to those who at least did
some side-activities for me, like discussing my
use-case and reviewing part of the patches.
These activities were very valuable, but the real
problems were never discussed before, proposed
API was not reviewed, and so on.

And with your input I already advanced to the
point where the API can be split into 2 parts,
whereas before I thought of dlmem() as a
thing in itself that can't be split. Now I see
that RTLD_NORELOC can solve the biggest part
of the puzzle alone.


> when i said you should write your own loader i expected
> you to create simple stubs in memory at the location
> you want them to be and bind them to a library that
> is loaded by libc (so you can connect your weird world
> with the libc world).
>
> it looked like a simple solution to me, but i still dont
> know exactly your requirements so in a sense we still
> don't have a well understood use-case for any of your
> proposed changes. so maye the use-case is where you
> should start.
You mean "restart". :)
As I already explained it very thoroughly in the
bugzilla, but you missed that and now its better
to repeat, since the bugzilla entry had a very
lengthy discussions afterwards.

The use-case is a VM with 32bit code inside.
It has a 4Gb window mapped somewhere in
a 64bit space (call it VM_window_start).
The task is very simple: first I find the suitable
mapping address within a VM address space.
Such address is within the 0-4Gb boundaries.
I relocate the solib to that "low" address.
Unfortunately this is still not visible to the VM,
because VM's window is at VM_window_start,
as we remember. So in order for VM-running
32bit code to access the solib's data (like .bss,
.rodata) when it gets a 32bit pointer to such
data, I also need to map the full duplicate of
the solib object into the address that looks like:

dupe_addr = VM_window_start + reloc_addr

For that I invented the optional dlmem flag
that doesn't destroy the destination mapping.
But such flag doesn't survive your requirement
to get rid of a call-back, so in the next prop
I'll pass a shm fd to dlmem instead, and an
optional flag will ask dlmem() to mmap that
fd as a destination backing-store.

So I am sure all problems can be solved!
But of course I can't do that alone, if everyone
just keeps saying "drop your patches immediately".