Library corruption when setting RPATH

[pablogsal]

Describe the bug

Some shared libraries are corrupted when patchelf sets the RPATH to some new value.

Steps To Reproduce

Consider this shared library:

https://www.dropbox.com/s/n47wla3tgw4ny56/libfreebl3.so?dl=0

If you create a very simple executable that links against it, everything works:

echo "int main(){return 0;}" > lel.c
gcc lel.c -L . -lfreebl3
./a.out

but now, changing the RPATH with patchelf (master) makes the program segfault:

./src/patchelf --set-rpath $ORIGIN --force-rpath libfreebl3.so
❯ ./a.out
[1]    19792 segmentation fault (core dumped)  ./a.out

Apparently, the segfault happens when the linker is resolving relocations:

LD_DEBUG=all ./a.out

...
     19907:     file=libdl.so.2 [0];  generating link map
     19907:       dynamic: 0x00007fe6ad83ecf0  base: 0x00007fe6ad83a000   size: 0x0000000000005090
     19907:         entry: 0x00007fe6ad83b210  phdr: 0x00007fe6ad83a040  phnum:                 11
     19907:
     19907:     checking for version `GLIBC_2.2.5' in file /usr/lib/libc.so.6 [0] required by file ./a.out [0]
     19907:     checking for version `GLIBC_2.2.5' in file /usr/lib/libdl.so.2 [0] required by file libfreebl3.so [0]
     19907:     checking for version `GLIBC_2.3' in file /usr/lib/libc.so.6 [0] required by file libfreebl3.so [0]
     19907:     checking for version `GLIBC_2.2.5' in file /usr/lib/libc.so.6 [0] required by file libfreebl3.so [0]
     19907:     checking for version `GLIBC_2.2.5' in file /lib64/ld-linux-x86-64.so.2 [0] required by file /usr/lib/libc.so.6 [0]
     19907:     checking for version `GLIBC_2.3' in file /lib64/ld-linux-x86-64.so.2 [0] required by file /usr/lib/libc.so.6 [0]
     19907:     checking for version `GLIBC_PRIVATE' in file /lib64/ld-linux-x86-64.so.2 [0] required by file /usr/lib/libc.so.6 [0]
     19907:     checking for version `GLIBC_PRIVATE' in file /lib64/ld-linux-x86-64.so.2 [0] required by file /usr/lib/libdl.so.2 [0]
     19907:     checking for version `GLIBC_PRIVATE' in file /usr/lib/libc.so.6 [0] required by file /usr/lib/libdl.so.2 [0]
     19907:     checking for version `GLIBC_2.4' in file /usr/lib/libc.so.6 [0] required by file /usr/lib/libdl.so.2 [0]
     19907:     checking for version `GLIBC_2.2.5' in file /usr/lib/libc.so.6 [0] required by file /usr/lib/libdl.so.2 [0]
     19907:
     19907:     Initial object scopes
     19907:     object=./a.out [0]
     19907:      scope 0: ./a.out libfreebl3.so /usr/lib/libc.so.6 /usr/lib/libdl.so.2 /lib64/ld-linux-x86-64.so.2
     19907:
     19907:     object=linux-vdso.so.1 [0]
     19907:      scope 0: ./a.out libfreebl3.so /usr/lib/libc.so.6 /usr/lib/libdl.so.2 /lib64/ld-linux-x86-64.so.2
     19907:      scope 1: linux-vdso.so.1
     19907:
     19907:     object=libfreebl3.so [0]
     19907:      scope 0: libfreebl3.so
     19907:      scope 1: ./a.out libfreebl3.so /usr/lib/libc.so.6 /usr/lib/libdl.so.2 /lib64/ld-linux-x86-64.so.2
     19907:
     19907:     object=/usr/lib/libc.so.6 [0]
     19907:      scope 0: ./a.out libfreebl3.so /usr/lib/libc.so.6 /usr/lib/libdl.so.2 /lib64/ld-linux-x86-64.so.2
     19907:
     19907:     object=/usr/lib/libdl.so.2 [0]
     19907:      scope 0: ./a.out libfreebl3.so /usr/lib/libc.so.6 /usr/lib/libdl.so.2 /lib64/ld-linux-x86-64.so.2
     19907:
     19907:     object=/lib64/ld-linux-x86-64.so.2 [0]
     19907:      no scope
     19907:
     19907:
     19907:     relocation processing: /usr/lib/libc.so.6
     19907:     symbol=_res;  lookup in file=./a.out [0]
     19907:     symbol=_res;  lookup in file=libfreebl3.so [0]
[1]    19907 segmentation fault (core dumped)  LD_DEBUG=all ./a.out

the backtrace according to gdb corroborates this:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7fdb504 in do_lookup_x () from /lib64/ld-linux-x86-64.so.2
(gdb) bt
#0  0x00007ffff7fdb504 in do_lookup_x () from /lib64/ld-linux-x86-64.so.2
#1  0x00007ffff7fdc121 in _dl_lookup_symbol_x () from /lib64/ld-linux-x86-64.so.2
#2  0x00007ffff7fddcaf in _dl_relocate_object () from /lib64/ld-linux-x86-64.so.2
#3  0x00007ffff7fd55d3 in dl_main () from /lib64/ld-linux-x86-64.so.2
#4  0x00007ffff7feb992 in _dl_sysdep_start () from /lib64/ld-linux-x86-64.so.2
#5  0x00007ffff7fd2ff1 in _dl_start () from /lib64/ld-linux-x86-64.so.2
#6  0x00007ffff7fd2098 in _start () from /lib64/ld-linux-x86-64.so.2
#7  0x0000000000000001 in ?? ()
#8  0x00007fffffffdcea in ?? ()
#9  0x0000000000000000 in ?? ()

Expected behavior

The library is not corrupted and running the previous example works correctly without segfaulting.

patchelf --version output

patchelf 0.12

[pablogsal]

Seems that this happens when patchelf writes a new ELF file because this happens as well with other commands like --set-soname:

./src/patchelf --set-soname libfreebl2.so libfreebl3.so
LD_DEBUG=all ./a.out
...
     35708:
     35708:
     35708:     relocation processing: /usr/lib/libc.so.6
     35708:     symbol=_res;  lookup in file=./a.out [0]
     35708:     symbol=_res;  lookup in file=libfreebl3.so [0]
[1]    35708 segmentation fault (core dumped)  LD_DEBUG=all ./a.out

[eytanaim]

While patching my local copy of the code to overcome these corruptions, I have managed to understand the root cause of all 3 different corruptions.
They all happened due to the fact that the vaddr values of sections and headers are not identical to the offset values.
It seems like 'new' versions of shared objects (rhel 7 and above) use vaddr equal to offset value, while old versions (rhel5 and 6) use a large value that represents a real address and is different than the offset value.

It seems that the current build of patchelf doesn't support system libraries (libpthread, libm, etc...) On rhel 5 and 6.

[pablogsal]

They all happened due to the fact that the vaddr values of sections and headers are not identical to the offset values.
It seems like 'new' versions of shared objects (rhel 7 and above) use vaddr equal to offset value, while old versions (rhel5 and 6) use a large value that represents a real address and is different than the offset value.

@eytanaim Thanks for the investigation! What offset are you referring to? The offset in the PT_LOAD sections?

While patching my local copy of the code to overcome these corruptions, I have managed to understand the root cause of all 3 different corruptions.

Do you have a patch or some diff for seeing what you are referring to?

[eytanaim]

I actually thought that I have replied to another thread, but looking on your attached so seems to suit my issue as well.
Please check if the following patch fixes your problem:
patch.txt

[pablogsal]

Please check if the following patch fixes your problem:

It does! (Or at least it does not crash anymore) Could you create a PR for this?

[pablogsal]

Actually, to fix the crash is enough with this change:

@@ -778,7 +782,7 @@ void ElfFile<ElfFileParamNames>::rewriteSectionsLibrary()
     /* Some sections may already be replaced so account for that */
     unsigned int i = 1;
     Elf_Addr pht_size = sizeof(Elf_Ehdr) + (phdrs.size() + num_notes + 1)*sizeof(Elf_Phdr);
-    while( shdrs[i].sh_addr <= pht_size && i < rdi(hdr->e_shnum) ) {
+    while( shdrs[i].sh_offset <= pht_size && i < rdi(hdr->e_shnum) ) {
         if (not haveReplacedSection(getSectionName(shdrs[i])))
             replaceSection(getSectionName(shdrs[i]), shdrs[i].sh_size);
         i++;

[pablogsal]

I think that we could start with a patch that only changes the sh_addr to sh_offset as further changes may be related to different issues.

[domenkozar]

It would be great if someone can create a PR with a test attached like so https://github.com/NixOS/patchelf/pull/237/files#diff-128d6520cbd423e01b660cc84f2f0ce0

[domenkozar]

Refs #164

[pablogsal]

It would be great if someone can create a PR with a test attached like so https://github.com/NixOS/patchelf/pull/237/files#diff-128d6520cbd423e01b660cc84f2f0ce0

I can do that, are you ok vendoring the problematic lib in the test suite?

[pablogsal]

I can do that, are you ok vendoring the problematic lib in the test suite?

@domenkozar Unfortunately, producing libraries with this setup requires some linker scripts as it needs to modify the program header address. and that will complicate considerably the test suite in order to make it portable. Given that this code path is always hitted, we still have certainty that works as expected.