Page MenuHomeFreeBSD

rtld: Switch to the standard symbol lookup behavior if LD_DYNAMIC_WEAK=0
ClosedPublic

Authored by kib on Sep 7 2020, 5:30 PM.
Tags
None
Referenced Files
F89255941: D26352.diff
Thu, Jul 25, 6:47 PM
F89236120: D26352.id93671.diff
Thu, Jul 25, 11:27 AM
Unknown Object (File)
Mon, Jul 8, 4:30 PM
Unknown Object (File)
Sun, Jul 7, 8:56 AM
Unknown Object (File)
Wed, Jul 3, 2:16 PM
Unknown Object (File)
Mon, Jul 1, 8:03 PM
Unknown Object (File)
Fri, Jun 28, 9:30 AM
Unknown Object (File)
Jun 25 2024, 8:31 AM
Subscribers

Details

Summary

The current lookup prefers a strong definition to a STB_WEAK definition (similar
to glibc pre-2.2 behavior) which does not conform to the ELF specification.

The non-compliant behavior provoked https://reviews.llvm.org/D4418 which was
intended to fix -shared-libasan but introduced new problems (and caused some
sanitizer tests (e.g. test/asan/TestCases/interception_failure_test.cpp) to
fail): sanitizer interceptors are STB_GLOBAL instead of STB_WEAK, so defining a
second STB_GLOBAL interceptor can lead to a multiple definition linker error.
For example, in a -fsanitize={address,memory,...} build, libc functions like
malloc/free/strtol/... cannot be provided by user object files.

See
https://docs.freebsd.org/cgi/getmsg.cgi?fetch=16483939+0+archive/2014/freebsd-current/20140716.freebsd-current
for discussions.

This patch implements the ELF-compliant behavior under LD_DYNAMIC_WEAK=0.
As advised by kib, STB_WEAK wrestling in symbol lookups in `Search the dynamic
linker itself` are untouched.

You may use git commit --amend --author='Fangrui Song <i@maskray.me>'

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 41037
Build 37926: arc lint + arc unit

Event Timeline

fbsd-phab_maskray.me edited the summary of this revision. (Show Details)
fbsd-phab_maskray.me edited the summary of this revision. (Show Details)
fbsd-phab_maskray.me edited the summary of this revision. (Show Details)

This breaks FreeBSD. If you look at the deleted block, the deleted comment explains why (and this is only one instance of it, there are more).

In D26352#585872, @kib wrote:

This breaks FreeBSD. If you look at the deleted block, the deleted comment explains why (and this is only one instance of it, there are more).

I can indeed find many instances of Search the dynamic linker itself in the file. Is this unfixable now?

(The special case for the ld-elf.so.1 is strange: in glibc, the dynamic linker needs to be in a DT_NEEDED entry to be searchable, so there is no such special case.)

After make buildworld, how can I rebuild just libexec/rtld-elf? make -C libexec/rtld-elf does not work

% make -C libexec/rtld-elf                                                                                                                       [41/30860]
cc  -O2 -pipe -fno-common   -Wall -DFREEBSD_ELF -DIN_RTLD -ffreestanding -I/usr/home/ray/freebsd/lib/csu/common -I/usr/home/ray/freebsd/libexec/rtld-elf/amd64 
-I/usr/home/ray/freebsd/libexec/rtld-elf -fpic -DPIC  -I/usr/home/ray/freebsd/libexec/rtld-elf/rtld-libc -mno-mmx -mno-sse -mno-avx -mno-avx2 -msoft-float -fvi
sibility=hidden -g -MD  -MF.depend.rtld.o -MTrtld.o -std=gnu99 -Wno-format-zero-length -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter 
-Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wchar-subscripts -Winl
ine -Wnested-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -Wformat=2 -Wno-format-extra-args -Werror -Wmissing-variable-declarations -Wthr
ead-safety -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable  -Qunused-arguments    -c /usr/home/ray/freebsd/libexec/rtld-elf/rtld.c -o rtld.o   
/usr/home/ray/freebsd/libexec/rtld-elf/rtld.c:450:18: error: use of undeclared identifier 'AT_BSDFLAGS'
    if (aux_info[AT_BSDFLAGS] != NULL &&                                       
                 ^                     
/usr/home/ray/freebsd/libexec/rtld-elf/rtld.c:451:12: error: use of undeclared identifier 'AT_BSDFLAGS'                                                        
        (aux_info[AT_BSDFLAGS]->a_un.a_val & ELF_BSDF_SIGFASTBLK) != 0)        
                  ^                                                            
/usr/home/ray/freebsd/libexec/rtld-elf/rtld.c:451:39: error: use of undeclared identifier 'ELF_BSDF_SIGFASTBLK'                                                
        (aux_info[AT_BSDFLAGS]->a_un.a_val & ELF_BSDF_SIGFASTBLK) != 0)        
                                             ^                                 
/usr/home/ray/freebsd/libexec/rtld-elf/rtld.c:1231:20: error: no member named 'l_refname' in 'struct link_map'; did you mean 'l_name'?                         
                if (obj->linkmap.l_refname == NULL) 
...
In D26352#585876, @i_maskray.me wrote:
In D26352#585872, @kib wrote:

This breaks FreeBSD. If you look at the deleted block, the deleted comment explains why (and this is only one instance of it, there are more).

I can indeed find many instances of Search the dynamic linker itself in the file. Is this unfixable now?

I might have an idea how to handle this, but first I need to evaluate the scope.
Some time ago this quirk was also needed for libc+libthr operations, but I suspect that I fixed all places already while making libthr dlopenable.

(The special case for the ld-elf.so.1 is strange: in glibc, the dynamic linker needs to be in a DT_NEEDED entry to be searchable, so there is no such special case.)

We do not link with ld-elf.so.1, and until recently we do not even have libdl.so.

After make buildworld, how can I rebuild just libexec/rtld-elf? make -C libexec/rtld-elf does not work

% make -C libexec/rtld-elf                                                                                                                       [41/30860]
cc  -O2 -pipe -fno-common   -Wall -DFREEBSD_ELF -DIN_RTLD -ffreestanding -I/usr/home/ray/freebsd/lib/csu/common -I/usr/home/ray/freebsd/libexec/rtld-elf/amd64 
-I/usr/home/ray/freebsd/libexec/rtld-elf -fpic -DPIC  -I/usr/home/ray/freebsd/libexec/rtld-elf/rtld-libc -mno-mmx -mno-sse -mno-avx -mno-avx2 -msoft-float -fvi
sibility=hidden -g -MD  -MF.depend.rtld.o -MTrtld.o -std=gnu99 -Wno-format-zero-length -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter 
-Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wchar-subscripts -Winl
ine -Wnested-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -Wformat=2 -Wno-format-extra-args -Werror -Wmissing-variable-declarations -Wthr
ead-safety -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable  -Qunused-arguments    -c /usr/home/ray/freebsd/libexec/rtld-elf/rtld.c -o rtld.o   
/usr/home/ray/freebsd/libexec/rtld-elf/rtld.c:450:18: error: use of undeclared identifier 'AT_BSDFLAGS'
    if (aux_info[AT_BSDFLAGS] != NULL &&                                       
                 ^                     
/usr/home/ray/freebsd/libexec/rtld-elf/rtld.c:451:12: error: use of undeclared identifier 'AT_BSDFLAGS'                                                        
        (aux_info[AT_BSDFLAGS]->a_un.a_val & ELF_BSDF_SIGFASTBLK) != 0)        
                  ^                                                            
/usr/home/ray/freebsd/libexec/rtld-elf/rtld.c:451:39: error: use of undeclared identifier 'ELF_BSDF_SIGFASTBLK'                                                
        (aux_info[AT_BSDFLAGS]->a_un.a_val & ELF_BSDF_SIGFASTBLK) != 0)        
                                             ^                                 
/usr/home/ray/freebsd/libexec/rtld-elf/rtld.c:1231:20: error: no member named 'l_refname' in 'struct link_map'; did you mean 'l_name'?                         
                if (obj->linkmap.l_refname == NULL) 
...

You should 'enter the build environment'. Basically you do make buildenv (but care to supply all the same knobs like MAKEOBJDIRPREFIX etc), then you can cd libexec/rtld-elf and do make there. But note the useful hint in the libexec/rtld-elf/Makefile at the beginning.

Fix all STB_WEAK occurrences

Unfortunately this can cause a segfault in a function in libc.so

libexec/rtld-elf/rtld.c
3690

I suggest you to leave this (and all similar 'search the dynamic linker itself') places intact. Basically, the biggest issue I am aware of where FreeBSD depends on the strong override of weak, is rtld export. We provide weak symbols like dlopen exported from libc weak, and rtld links to its own definition of the symbols.

If you leave this chunk as is, rtld export should still work. Then I suspect there is nothing else left that depends on this behavior in the base system.

libexec/rtld-elf/rtld.c
3690

And one more thing. Perhaps, old behavior should not be removed completely. I suggest to add e.g. the variable ${LD}_WEAK_OLD, which if set, restores the old mode of operation where strong overrides weak (feel free to suggest more adequate name).

fbsd-phab_maskray.me edited the summary of this revision. (Show Details)

You may use git commit --amend --author='Fangrui Song <i@maskray.me>'

Add LD_DYNAMIC_WEAK

Test:

cat > ./a.c <<eof
#include <dlfcn.h>
#include <stdio.h>

void foo();

int main(int argc, char *argv[]) {
  foo();

  void *h = dlopen(0, RTLD_LAZY);
  void (*fptr)() = dlsym(h, "foo");
  fptr();
}
eof
cat > ./b.c <<eof
#include <stdio.h>

__attribute__((weak))
void foo() {
  puts("b.c");
}
eof
cat > ./c.c <<eof
#include <stdio.h>

void foo() {
  puts("b.c");
}
eof
cat > ./Makefile <<eof
a: a.c b.so c.so
        cc a.c ./b.so ./c.so -o a -ldl

b.so: b.c
        cc -shared -fpic b.c -o $@

c.so: c.c
        cc -shared -fpic c.c -o $@
eof


% ~/freebsd/obj/default/usr/home/ray/freebsd/amd64.amd64/libexec/rtld-elf/ld-elf.so.1 ./a
b.c
b.c
ray @ freebsd >>= ~/tmp 
% LD_DYNAMIC_WEAK= ~/freebsd/obj/default/usr/home/ray/freebsd/amd64.amd64/libexec/rtld-elf/ld-elf.so.1 ./a
c.c
c.c
fbsd-phab_maskray.me retitled this revision from Don't allow strong symbols to override weak ones for lookups to rtld: Don't allow strong symbols to override weak ones for lookups.Aug 14 2021, 4:56 AM

Dynamic linker itself is just an example of the breakage. There are more problems with e.g. libc/libthr, and this is only the base system. I do not know how much and where other problems are, in real world.

More, what users would do with this env var? They see that an app does not work, so what is next? How could they know that the env var needs to be set and _this_ is the cause of the breakage. And note that the variable must be ignored for setuid/setgid images.

So it cannot be handled this way.

I think your patch might be useful for experimentation, if you flip the meaning of the env var. That is, instead of making default standard-compliant, keep it as is. If the var is set, switch to desired behavior. Then we can commit this and some users would help us with evaluating the scope of the problem. In fact, I think this is a good first step.

fbsd-phab_maskray.me retitled this revision from rtld: Don't allow strong symbols to override weak ones for lookups to rtld: Switch to the standard symbol lookup behavior if LD_DYNAMIC_WEAK=0.
fbsd-phab_maskray.me edited the summary of this revision. (Show Details)
fbsd-phab_maskray.me removed a reviewer: manpages.

Disable by default

In D26352#711126, @kib wrote:

Dynamic linker itself is just an example of the breakage. There are more problems with e.g. libc/libthr, and this is only the base system. I do not know how much and where other problems are, in real world.

More, what users would do with this env var? They see that an app does not work, so what is next? How could they know that the env var needs to be set and _this_ is the cause of the breakage. And note that the variable must be ignored for setuid/setgid images.

So it cannot be handled this way.

I think your patch might be useful for experimentation, if you flip the meaning of the env var. That is, instead of making default standard-compliant, keep it as is. If the var is set, switch to desired behavior. Then we can commit this and some users would help us with evaluating the scope of the problem. In fact, I think this is a good first step.

In the long term it will help, because the LD_DYNAMIC_WEAK=0 behavior matches Linux glibc/musl.

kib edited reviewers, added: fbsd-phab_maskray.me; removed: kib.

Take over the review.

rtld conventions is that just a presence of the env var triggers the behavior, follow it for LD_DYNAMIC_WEAK.
Ignore the var for setuid binaries.
Clarify man page and add note about rtld symbols.

This revision is now accepted and ready to land.Aug 15 2021, 5:11 AM