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

  This commit adds the --no-version-coverage-check option to the
dynamic loader, and the LD_NO_VERSION_COVERAGE_CHECK environment
variable.

The new _dl_no_version_coverage_check field in struct rtld_global_ro
lands in previously-unused padding (on x86-64).

2019-09-06  Florian Weimer  <fweimer@redhat.com>

	[BZ #24974]
	* elf/rtld.c (dl_main): Add --no-version-coverage-check option.
	(process_envvars): Handle LD_NO_VERSION_COVERAGE_CHECK.
	* sysdeps/generic/ldsodefs.h (struct rtld_global_ro): Add
	_dl_no_version_coverage_check field.
	* elf/dl-version.c (match_symbol): Check it before reporting an
	error.
	* elf/Makefile [$(build-shared)] (tests): Add
	tst-no-version-coverage-lazy, tst-no-version-coverage-now.
	(modules-names): Add tst-no-version-coverage-linkmod,
	tst-no-version-coverage-runmod, tst-no-version-coverage-preloadmod.
	(LDFLAGS-tst-no-version-coverage-linkmod.so): Set version map and
	soname.
	(LDFLAGS-tst-no-version-coverage-runmod.so): Likewise.
	(LDFLAGS-tst-no-version-coverage-preloadmod.so): Likewise.
	(LDFLAGS-tst-no-version-coverage-lazy): Enable lazy binding.
	(tst-no-version-coverage-lazy): Link with
	tst-no-version-coverage-linkmod.so.
	(tst-no-version-coverage-lazy-ENV): Disable symbol version
	coverage check.
	(tst-no-version-coverage-lazy.out): Depend on
	tst-no-version-coverage-runmod.so.
	(LDFLAGS-tst-no-version-coverage-now): Disable lazy binding.
	(tst-no-version-coverage-now): Link with
	tst-no-version-coverage-linkmod.so.
	(tst-no-version-coverage-now-ENV): Preload
	tst-no-version-coverage-preloadmod.so.  Disable symbol version
	coverage check.
	(tst-no-version-coverage-now.out): Depend on
	tst-no-version-coverage-preloadmod.so,
	tst-no-version-coverage-runmod.so.
	* elf/tst-no-version-coverage-lazy.c: New file.
	* elf/tst-no-version-coverage-linkmod.c:: Likewise.
	* elf/tst-no-version-coverage-linkmod.map: Likewise.
	* elf/tst-no-version-coverage-now.c: Likewise.
	* elf/tst-no-version-coverage-preloadmod.c: Likewise.
	* elf/tst-no-version-coverage-preloadmod.map: Likewise.
	* elf/tst-no-version-coverage-runmod.c: Likewise.
	* elf/tst-no-version-coverage-runmod.map: Likewise.
  

On 9/6/19 11:59 AM, Florian Weimer wrote:
> This commit adds the --no-version-coverage-check option to the
> dynamic loader, and the LD_NO_VERSION_COVERAGE_CHECK environment
> variable.
> 
> The new _dl_no_version_coverage_check field in struct rtld_global_ro
> lands in previously-unused padding (on x86-64).

At a high level I like where you are going with this design.

I have some high level questions to ask, that way we can set the tone
for the overall design of the dynamic loader.

(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.

My idea is that if we are going to add a new env var that it should
be the *last* one we add.

We should add a GLIBC_LDSO="glibc.ld.no_version_coverage_check=1" and
then extend GLIBC_LDSO to contain all existing legacy env vars etc.
You can reuse all of the tunables code to manage *one* env var that
can alter the behaviour of the dynamic loader, and this includes
reusing the well tested handling of security context and clearing.

There are ~20 env vars that can be cleaned up and put into legacy
handling like this.

(b) When does a user know to use this env var?

- 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.

What other useful scenarios are supported by the change?

(c) Why not remove the check entirely?

What are the pros/cons of keeping or removing the check?

Cheers,
Carlos.
  

* 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).

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.

> (b) When does a user know to use this env var?

When application developers who ship a glibc compatibility shim tell
them to.

> - 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.

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.

> What other useful scenarios are supported by the change?

I don't know of anything else.

> (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.

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.

Thanks,
Florian
  

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?
  

* Carlos O'Donell:

> 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.

Sure, it's a shared namespace, like pretty much everything else.  I'd
love to coordinate such changes across libcs, but we don't have such a
forum today.

>> 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.

We'll need string quoting because any option separator we choose can in
paths.  At that point, you will be hard-pressed to implement this
processin using shell scripts.

> 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

What's missing from the envz functions?

>> 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.

I'll have to verify that this actually works out with the order things
are processed in the dynamic linker.

If it's more than a flag, I think we'd need a new data structure because
we have two arbitrary strings, soname and symbol version, and no clear
way to separate them.

>>> (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.

binutils has some remnants there.  I have never used it.

> I think the real question is this:
>
> Do we avoid the env var and try to implement it all as a DT_* tag?

For something that's just a hack to avoid rebasing glibc?  I don't think
so.  I suspect everyone would be better off if we could deliver
functionality provided by newer glibc versions in more direct ways.

A mere flag in DT_* might hit a sweet spot in terms of implementation
complexity and user convenience, though.

Thanks,
Florian
  

* Florian Weimer:

> * Carlos O'Donell:
>
>> 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.
>
> Sure, it's a shared namespace, like pretty much everything else.  I'd
> love to coordinate such changes across libcs, but we don't have such a
> forum today.
>
>>> 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.
>
> We'll need string quoting because any option separator we choose can
> [occur] in paths.  At that point, you will be hard-pressed to
> implement this processin[g] using shell scripts.

With “this processing”, I mean adding new entries to LD_LIBRARY_PATH,
for example.

>> 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
>
> What's missing from the envz functions?

Can we please move the discussion forward?

Thanks,
Florian
  

On 10/2/19 7:36 AM, Florian Weimer wrote:
> Can we please move the discussion forward?

I think we've settled on:

* No env var.
* Add a DT_ tag to disable the check.
* Users building the special preload DSOs need to set the
  new DT_ tag.
* Everything else is OK.

The DT_ tag prevents the user from having to do anything
other than just preload the library which is something
that users have experience doing.

What's the next step? New patch with new DT_ entries for
use with this?
  

* Carlos O'Donell:

> On 10/2/19 7:36 AM, Florian Weimer wrote:
>> Can we please move the discussion forward?
>
> I think we've settled on:
>
> * No env var.
> * Add a DT_ tag to disable the check.
> * Users building the special preload DSOs need to set the
>   new DT_ tag.
> * Everything else is OK.

I have no recollection of that decision, or anyone else requesting that
we follow this approach.  I expect that this will require a more
invasive change to the dynamic linker.

Alan Modra pointed out how to avoid binutils changes for this:

  <https://sourceware.org/ml/binutils/2019-09/msg00287.html>

I don't know if we can get a tag reservation for GNU gABI for this
feature.  I couldn't get one the last time, for .got.plt isolation (aka
safe lazy binding), so that project is now essentially dead.

I'm slightly concerned that what started out as a simple change is now
something that needs coordination between glibc, binutils (at least for
readelf), elfutils, and the GNU generic ABI.  Is this really the
consensus on this list?

Thanks,
Florian
  

On 10/2/19 1:00 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> On 10/2/19 7:36 AM, Florian Weimer wrote:
>>> Can we please move the discussion forward?
>>
>> I think we've settled on:
>>
>> * No env var.
>> * Add a DT_ tag to disable the check.
>> * Users building the special preload DSOs need to set the
>>   new DT_ tag.
>> * Everything else is OK.
> 
> I have no recollection of that decision, or anyone else requesting that
> we follow this approach.  I expect that this will require a more
> invasive change to the dynamic linker.

Sorry, I thought that was what we were talking about when hashing out
the details to avoid a new env var. Always good to practice 3 way communication :-)
 
> Alan Modra pointed out how to avoid binutils changes for this:
> 
>   <https://sourceware.org/ml/binutils/2019-09/msg00287.html>
> 
> I don't know if we can get a tag reservation for GNU gABI for this
> feature.  I couldn't get one the last time, for .got.plt isolation (aka
> safe lazy binding), so that project is now essentially dead.

Previous rejection doesn't mean you will get rejected again?

I know it's a platitude, but "if at first you don't succeed..." keep
trying until you understand who the key decision makers are and their
particular requirements.

> I'm slightly concerned that what started out as a simple change is now
> something that needs coordination between glibc, binutils (at least for
> readelf), elfutils, and the GNU generic ABI.  Is this really the
> consensus on this list?

Right now it's just me and you talking about this feature.

*I* was hoping to avoid an env var that the user has to set to magically
disable the check. With the DT_ tag you could disable the check just
for one DSO, and keep the check for all others?

Your suggestion of a DT_ tag seemed like a really nice idea. Do you dislike
the idea for some reason? Only because it's more work and might fail?
  

* Carlos O'Donell:

>> Alan Modra pointed out how to avoid binutils changes for this:
>> 
>>   <https://sourceware.org/ml/binutils/2019-09/msg00287.html>
>> 
>> I don't know if we can get a tag reservation for GNU gABI for this
>> feature.  I couldn't get one the last time, for .got.plt isolation (aka
>> safe lazy binding), so that project is now essentially dead.
>
> Previous rejection doesn't mean you will get rejected again?
>
> I know it's a platitude, but "if at first you don't succeed..." keep
> trying until you understand who the key decision makers are and their
> particular requirements.

I didn't receive any replies whatsoever this time:

  <https://sourceware.org/ml/gnu-gabi/2019-q4/msg00000.html>

What's the next step here?  Declare our copy of <elf.h> the primary
source for GNU, and suggest that changes to it are cross-posted to the
relevant lists?

This way, if glibc needs something, we can post a patch for review, wait
some time, and then allocate the number to ourselves.

Thanks,
Florian
  

On 10/28/19 11:02 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>>> Alan Modra pointed out how to avoid binutils changes for this:
>>>
>>>   <https://sourceware.org/ml/binutils/2019-09/msg00287.html>
>>>
>>> I don't know if we can get a tag reservation for GNU gABI for this
>>> feature.  I couldn't get one the last time, for .got.plt isolation (aka
>>> safe lazy binding), so that project is now essentially dead.
>>
>> Previous rejection doesn't mean you will get rejected again?
>>
>> I know it's a platitude, but "if at first you don't succeed..." keep
>> trying until you understand who the key decision makers are and their
>> particular requirements.
> 
> I didn't receive any replies whatsoever this time:
> 
>   <https://sourceware.org/ml/gnu-gabi/2019-q4/msg00000.html>
> 
> What's the next step here?  Declare our copy of <elf.h> the primary
> source for GNU, and suggest that changes to it are cross-posted to the
> relevant lists?
> 
> This way, if glibc needs something, we can post a patch for review, wait
> some time, and then allocate the number to ourselves.

I have responded to your comment, and suggested a course of action.

In summary: Go ahead and use the new DT_* tag as-if it were part of the GNU gABI.

We should make sure that binutils can process this new tag and display something sensible.

Will you be doing any binutils work for this?