Details

Reviewers

bdragon
kan
emaste
fbsd-phab_maskray.me

Commits

rG7da378f9de1a: rtld: Switch to the standard symbol lookup behavior if LD_DYNAMIC_WEAK is set

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

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 33438
Build 30725: arc lint + arc unit

Event Timeline

fbsd-phab_maskray.me created this revision.Sep 7 2020, 5:30 PM

Herald added a subscriber: bdragon. · View Herald TranscriptSep 7 2020, 5:30 PM

fbsd-phab_maskray.me requested review of this revision.Sep 7 2020, 5:30 PM

Harbormaster completed remote builds in B33432: Diff 76739.Sep 7 2020, 5:30 PM

fbsd-phab_maskray.me edited the summary of this revision. (Show Details)Sep 7 2020, 5:35 PM

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

fbsd-phab_maskray.me added reviewers: kib, bdragon, kan.Sep 7 2020, 5:37 PM

fbsd-phab_maskray.me edited the summary of this revision. (Show Details)Sep 7 2020, 5:39 PM

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

lwhsu added a reviewer: emaste.Sep 7 2020, 5:41 PM

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

Harbormaster completed remote builds in B33438: Diff 76753.Sep 8 2020, 3:38 AM

fbsd-phab_maskray.me edited the summary of this revision. (Show Details)Sep 8 2020, 3:41 AM

kib added inline comments.Sep 8 2020, 7:35 PM

libexec/rtld-elf/rtld.c
3654	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.

kib added inline comments.Sep 8 2020, 7:43 PM

libexec/rtld-elf/rtld.c
3654	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).

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

Herald added a subscriber: imp. · View Herald TranscriptAug 14 2021, 12:04 AM

Harbormaster completed remote builds in B41037: Diff 93668.Aug 14 2021, 12:04 AM

fbsd-phab_maskray.me edited the summary of this revision. (Show Details)Aug 14 2021, 12:07 AM

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

add doc

Herald added a reviewer: manpages. · View Herald TranscriptAug 14 2021, 5:30 AM

Harbormaster completed remote builds in B41039: Diff 93671.Aug 14 2021, 5:30 AM

fbsd-phab_maskray.me marked 2 inline comments as done.Aug 14 2021, 6:22 PM

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.

Disable by default

Harbormaster completed remote builds in B41046: Diff 93700.Aug 15 2021, 3:34 AM

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.

actually default to true

Harbormaster completed remote builds in B41047: Diff 93701.Aug 15 2021, 3:47 AM

fix the condition

Harbormaster completed remote builds in B41048: Diff 93702.Aug 15 2021, 3:51 AM

kib commandeered this revision.Aug 15 2021, 4:17 AM

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.

fbsd-phab_maskray.me accepted this revision.Aug 15 2021, 5:11 AM

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

Closed by commit rG7da378f9de1a: rtld: Switch to the standard symbol lookup behavior if LD_DYNAMIC_WEAK is set (authored by fbsd-phab_maskray.me, committed by kib). · Explain WhyAug 16 2021, 10:56 AM

This revision was automatically updated to reflect the committed changes.

kib added a commit: rG7da378f9de1a: rtld: Switch to the standard symbol lookup behavior if LD_DYNAMIC_WEAK is set.

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

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 76753

libexec/rtld-elf/rtld.c

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 76753

libexec/rtld-elf/rtld.c

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

Revision Contents
Changeset List