Page MenuHomeFreeBSD

rtld: Always check LD_64_/LD_32_ environment variables
Needs ReviewPublic

Authored by arichardson on Jan 18 2021, 11:11 AM.

Details

Summary

With this change a 64-bit RTLD will check LD_64_* first and if those are
not set it will fall back to checking LD_*. Similarly, the 32-bit RTLDs
check LD_32_*, and then fall back to LD_*.

The main motivation here is CheriBSD, where we have an additional rtld for
CHERI binaries that uses LD_CHERI_*. Therefore, if you have existing 64-bit
software package that ships with a script that sets LD_LIBRARY_PATH to find
the local libraries, this would not work since currently LD_* is only used
by the native RTLD (i.e. CHERI pure-capability for us). To handle this case
this change makes RTLD check the ABI-suffixed variables first and if not
set falls back to LD_*. This still allows for different paths for different
ABI, but handles the program with bundled libraries + launcher script case
better.

Obtained from: CheriBSD (https://github.com/CTSRD-CHERI/cheribsd/pull/874)

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 36322
Build 33211: arc lint + arc unit

Event Timeline

  • fix warning building rtld-elf32
libexec/rtld-elf/paths.h
33

We use ILP32__/LP64__ for these checks.

libexec/rtld-elf/rtld.1
138

Perhaps note that LD_64_ have a priority over LD_

libexec/rtld-elf/rtld.c
351

Wouldn't this case break after your changes ?

358

style: char *

arichardson marked 2 inline comments as done.

update manpage and fix style

libexec/rtld-elf/paths.h
33

The reason that I did it this way is that the use of LP64 causes issues for CHERI (we use 64-bit virtual addresses and integer registers, so generally do want the LP64 branch, but pointers are 128-bits so LP64 is not defined).
But actually, the SIZEOF_POINTER checks also don't work for CHERI. In the CheriBSD patch I have an additional ifdef first so it does matter what the other branches do.

Since your suggestion also works for all currently supported architectures, and I can make the change if you prefer.

libexec/rtld-elf/rtld.1
138

Sounds good, will update.

libexec/rtld-elf/rtld.c
351

Do you mean LD_SOFT_* won't be handled properly?
The get_ld_env() helper checks whatever _LD(x) returns, so it should check LD_SOFT_* first followed by LD_*.

I haven't tested those variants, so not sure if we want the fallback behaviour there too.

libexec/rtld-elf/paths.h
33

Yes I prefer standard ABI designators. We have around 450 checks with them, not counting contrib.

libexec/rtld-elf/rtld.1
144

There is something wrong with this sentence. It says that LD_64 has priority over LD_64.

libexec/rtld-elf/rtld.c
351

May be LD_SOFT should be somehow integrated into get_ld_env. Right now it looks like as two levels of archeology put one over another.

libexec/rtld-elf/paths.h
33

Well, a lot of those are wrong for CHERI and should become more precise in what their requirements are; most of the time it's really __SIZEOF_LONG__ (or, even more precisely, "how big is my virtual address space", but the two are equivalent if you don't care about Window's awful LLP64). However, __ILP32__ and __LP64__ are fine to be checking here, as pure-capability CHERI C is a different ABI with a different environment variable prefix.

arichardson marked 2 inline comments as done.
  • use LP64
  • fix typo in manpage
libexec/rtld-elf/rtld.c
351

I agree it would be nice to merge the LD_SOFT prefix here. However that is determined dynamically on startup based on auxargs so we can't do it using the pre-processor. In the common case where we don't need this (i.e. all non-ARM architectures) we can avoid the strlcpy.

I think cleaning up ld_env_prefix should probably be a separate commit.

So one question is: how far makes sense to take this? Should rtld always read the ABI-specific libmap+hints and search in lib32/lib64? Only handling the ABI-specific environment variables is inconsistent.

libexec/rtld-elf/paths.h
69

This should be LD_FALLBACK_ with no argument to match the others.

69–70

This was only here because of COMPAT_32BIT.

libexec/rtld-elf/rtld.c
355

32,64 looks more normal

359

Both are prefixed.

arichardson marked 3 inline comments as done.
  • Address more review comments

So one question is: how far makes sense to take this? Should rtld always read the ABI-specific libmap+hints and search in lib32/lib64? Only handling the ABI-specific environment variables is inconsistent.

I'd prefer if that was done using symlinks to avoid having to stat twice as many files/directories.

libexec/rtld-elf/paths.h
69

good catch, it was required in the initial version for CheriBSD, but is no longer needed.

Does this mean that /libexec/ld-elf32.so.1 starts accepting LD_ vars, without _32_ part?

In D28220#631389, @kib wrote:

Does this mean that /libexec/ld-elf32.so.1 starts accepting LD_ vars, without _32_ part?

Yes. In the original version of this change (https://github.com/CTSRD-CHERI/cheribsd/pull/874) I disabled this behaviour for 32-bit compat RTLD. I can restore it if you prefer to avoid behaviour changes.

In D28220#631389, @kib wrote:

Does this mean that /libexec/ld-elf32.so.1 starts accepting LD_ vars, without _32_ part?

Yes. In the original version of this change (https://github.com/CTSRD-CHERI/cheribsd/pull/874) I disabled this behaviour for 32-bit compat RTLD. I can restore it if you prefer to avoid behaviour changes.

I am not opposing it, and think that my personal life would even become somewhat easier. But I do think that the change should be discussed on arch/current before applying it, and not in context of the change introducing LD_64_.