This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] ld.so: Enable preloading of new symbol versions [BZ #24974]


On 9/9/19 8:01 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> (a) Not a tunable, but use some tunable framework.
>>
>> I assume this isn't a tunable because it changes the semantics of the
>> dynamic loader.
>>
>> I would like us to avoid adding another env var since it will become
>> another variable that downstream needs to learn to "clean" from the
>> environment if they want to manage runtime behaviour.
> 
> We have basically reserved the LD_ prefix for these knobs.  It is
> relatively easy to strip all environment variables which start with
> "LD_" (very easy if you use posix_spawn or execve with an explicit
> envp).

Let me explore the solution space a bit here, I don't disagree with you, but
I'm playing a bit of devil's advocate. My own opinion is that changes of this
nature need more public discourse on the pros and cons. Sot let me lecture a
bit.

Yes, users can do `unset "${!LD@}"` to unset all such environment variables,
and in all the years I've been working with downstream packages I've seen
this construct, and its variants, used very few times. 

It is not used, IMO, because it is not a conservative action to take on the
part of the ISV.

Example from OpenEmbedded:

106 # autotools.bbclass sets the _FOR_BUILD variables, but for some reason we need
107 # to unset LD_LIBRARY_PATH.
108 export CC_FOR_BUILD = "LD_LIBRARY_PATH= ${BUILD_CC}"

Developers tend to remove env vars one at a time when they impact their use
cases, rather than blanket removal of all LD_* env vars.

I also do not think that LD_* is reserved for glibc, we now have shared
semantics with musl, and soon llvm's libc, and I fully expect they will create
their own environment variables here with an LD_* prefix. This means that we
might be removing env vars that are used by musl or llvm-libc and we should not
do that.

We did the right thing with tunables to use a single variable with N=V pairs,
but in that case we envisioned a *huge* number of tunables that were set as a
collection, and could be unset without impacting the ability to run the program.

We have more isolation using a GLIBC_* prefix which is clearly the variables
used by only glibc.

The counter argument here is that GLIBC_LDSO is a single monolithic variable
that is either set or unset, and that could be problematic for developers who
need to alter only part of it via the command line.

If I have to weigh between the two:

(a) One env var for multiple variables.

vs.

(b) One env var per variable.

I think it comes out slightly in favour of (b), one env var per variable,
as you suggest, but for the following reasons:

(1) These variables have direct influence over the operation of the software
    and there is an easy shell-based mechanism for setting or unsetting the
    values.

(2) The variables can be set or unset one at a time allowing easy and fine
    grained control. As is seen in the OE example. The variables are orthogonal
    to each other.

(3) We have no expectation that we will grow the list of LD_* vars in any
    appreciable way, compared to tunables which can change with each release.

So I think we are on the same page here, use one env var for this behaviour.

However, we should be cautious of musl and llvm-libc to ensure that we don't
have problems with the LD_* namespace.
 
> If we stuff everything into a single environment variable, applications
> would have to write a parser for the variable contents even if they just
> want to add a single directory to LD_LIBRARY_PATH.  This doesn't strike
> me as a good idea.

One already has to have a sufficiently complex parser to handle : splitting,
along with spaces and escaping, extending the parser to handle N=V is no
that much additional complication. In-line editing of LD_LIBRARY_PATH via
shell is not trivial, but setting and unsetting is easy.

I would *support* making APIs to properly handle env-var => array, and
array => env-var support, so one can insert into LD_LIBRARY_PATH or
GLIBC_TUNABLES easily from a C API.

e.g. https://github.com/sloria/environs
     https://12factor.net/config

In summary:

- Unsetting a single env var is easy.
- Setting it to an entirely new value is easy.
- Appending to the env var is easy.
- Rewriting the env var is hard.

This still weighs in favour of a single env var per control.

>> (b) When does a user know to use this env var?
> 
> When application developers who ship a glibc compatibility shim tell
> them to.

We should call this out in the NEWS entry then to make it clear how the
new feature is used.

>> - Is one scenario like this:
>>
>>   - They have an old library that doesn't provide all the symbols they need.
>>   - They implement a preload that provides the missing functionality.
>>   - They disable the check with an env var and run a newer program using 
>>     a mix of the old library and preload to provide all the required symbols.
>>
>> Is that a valid scenario? We should document something like this in
>> the NEWS entry.
> 
> The NEWS entry already references this scenario (LD_PRELOAD of new
> library versions).  If we had documentation for the dynamic loader, I
> would have added it there.

Thanks, I was just practicing 3 way communication here to try and
rewrite in my own words what I understood. Given that you seem to agree
I guess I understood it correctly.

We have proposed initial documentation for the dynamic loader:
https://sourceware.org/ml/libc-alpha/2017-10/msg00582.html

Someone needs to review it, and integrate it, and then we'd have a
place to put this.

I do not block your suggested fix, but it would be nice, as a senior
developer, to help us make incremental progress on this issue.

> We could also add a new DT_ tag to the LD_PRELOAD library and disable
> the check for the process if we encounter such a library.  That would
> simplify things for users.

Exactly! I was just going to suggest this, because if the use case is
designed for this exact scenario then the developer of the shim can
build the shim in a certain way to make it easier for the user.

The semantics of the new DT_ tag would be interesting, because we could
do something like:

DT_PROVIDE_VERSIONS="libc.so.6=GLIBC_2.18,GLIBC_2.19:libpthread=GLIBC_2.18,GLIBC_2.19"

Encoding at least some of the .gnu.version_r information in DT_VERSIONS
to allow us to make the error checking more robust.

This is just a thought.

>> What other useful scenarios are supported by the change?
> 
> I don't know of anything else.

Fine by me, the use case we already have is quite useful.

>> (c) Why not remove the check entirely?
> 
> I don't know if anyone uses Solaris-style versions (without associated
> symbols).  They would completely break as a result, in the sense that
> the version check for them is completely gone.

I don't follow, could you expand on this a bit, or verify that I understand
you correctly?

A Solaris-style version is where we have possibly one version recorded
per library, and it's the oldest version that covers the called APIs.

Which could be implemented as just having .gnu.version_r entries without
the associated symbol versions? Right?

I have never seen a Linux-based implementation of this, but I agree that
removing the check entirely would not support such an alternative use.

> Even without that, with lazy binding, users will start to see crashes in
> programs if their glibc (or libstdc++) is too old, instead of them
> failing to start at all.  I don't think that would be an improvement.

I agree. I wanted to ask the question because I think it's an interesting
thought experiment.

In summary:

- It is a conscious choice to use a new env var for the reasons outlined
  above.

- We should investigate using a new DT_* tag to remove the env var and
  make the solution transparent to users.

- The check should not be removed entirely because it is useful for
  normal builds and checking (error is up front at startup), and it
  supports Solaris-style implementations.

I think the real question is this:

Do we avoid the env var and try to implement it all as a DT_* tag?

-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]