Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

patchelf --set-rpath outputs a binary with broken .dynsym/.dynstr #244

Open
delroth opened this issue Sep 25, 2020 · 8 comments · May be fixed by #544
Open

patchelf --set-rpath outputs a binary with broken .dynsym/.dynstr #244

delroth opened this issue Sep 25, 2020 · 8 comments · May be fixed by #544
Labels

Comments

@delroth
Copy link
Contributor

delroth commented Sep 25, 2020

Describe the bug

This seems to be one of the root causes behind NixOS/nixpkgs#97407

When running a certain combination of patchelf + strip + patchelf on the upstream released stage2 ghc 8.6.5 binary, the last patchelf invocation (--set-rpath with a long rpath value) ends up generating a broken ELF file which segfaults the dynamic linker.

(gdb) target remote localhost:9000
Remote debugging using localhost:9000
warning: Loadable section ".dynsym" outside of ELF segments
warning: Loadable section ".dynstr" outside of ELF segments
warning: remote target does not support file transfer, attempting to access files from local filesystem.
Reading symbols from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1...
(No debugging symbols found in /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1)
0x00000055008020c0 in _start ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x0000005500808cf4 in decompose_rpath.isra ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1
(gdb) bt
#0  0x0000005500808cf4 in decompose_rpath.isra ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1
#1  0x000000550080901c in _dl_init_paths ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1
#2  0x0000005500804898 in dl_main ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1
#3  0x00000055008165b4 in _dl_sysdep_start ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1
#4  0x00000055008029d4 in _dl_start_final ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1
#5  0x0000005500802cf0 in _dl_start ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1
#6  0x00000055008020c8 in _start ()
   from /nix/store/yfa0b4pyywvnspwnlk2nw9id6h6f874x-glibc-2.31/lib/ld-linux-aarch64.so.1

Note that gdb seems to already point to a problem with the binary before execution:

warning: Loadable section ".dynsym" outside of ELF segments
warning: Loadable section ".dynstr" outside of ELF segments

And indeed, readelf seems to agree:

Before:

  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 4] .dynsym           DYNSYM           0000000000400298  00010298
       0000000000007560  0000000000000018   A       5     1     8
  [ 5] .dynstr           STRTAB           00000000004077f8  000177f8
       000000000000c5e1  0000000000000000   A       0     0     1

  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000010000 0x0000000000400000 0x0000000000400000
                 0x00000000001ab134 0x00000000001ab134  R E    0x10000

After:

  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 1] .dynstr           STRTAB           00000000003f0270  00000270
       000000000000c915  0000000000000000   A       0     0     8
  [ 2] .dynsym           DYNSYM           00000000003fcb88  0000cb88
       0000000000007560  0000000000000018   A       1     1     8

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000010000 0x0000000000400000 0x0000000000400000
                 0x00000000001ab134 0x00000000001ab134  R E    0x10000

Steps To Reproduce

I attached the input binary, gzipped to make github happy: repro.gz
. To reproduce, run:

src/patchelf --set-rpath '/nix/store/mbd1w6i6b98wbh5pf0ikghs1fz5p24j7-ncurses-6.2-abi5-compat/lib:/nix/store/qsxgr8vk6y8m95r7jf3qxrkz648g8p91-gmp-6.2.0/lib:$ORIGIN/../haskeline-0.7.4.3:$ORIGIN/../stm-2.5.0.0:$ORIGIN/../ghc-8.6.5:$ORIGIN/../terminfo-0.4.1.2:$ORIGIN/../process-1.6.5.0:$ORIGIN/../hpc-0.6.0.3:$ORIGIN/../ghci-8.6.5:$ORIGIN/../transformers-0.5.6.2:$ORIGIN/../template-haskell-2.14.0.0:$ORIGIN/../pretty-1.1.3.6:$ORIGIN/../ghc-heap-8.6.5:$ORIGIN/../ghc-boot-8.6.5:$ORIGIN/../ghc-boot-th-8.6.5:$ORIGIN/../directory-1.3.3.0:$ORIGIN/../unix-2.7.2.2:$ORIGIN/../time-1.8.0.2:$ORIGIN/../filepath-1.4.2.1:$ORIGIN/../binary-0.8.6.0:$ORIGIN/../containers-0.6.0.1:$ORIGIN/../bytestring-0.10.8.2:$ORIGIN/../deepseq-1.4.4.0:$ORIGIN/../array-0.5.3.0:$ORIGIN/../base-4.12.0.0:$ORIGIN/../integer-gmp-1.0.2.0:$ORIGIN/../ghc-prim-0.5.3:$ORIGIN/../rts' repro

Expected behavior

A clear and concise description of what you expected to happen.

patchelf --version output

HEAD

Additional context

For some reason the binaries in question don't crash on a 64k page size system. I suspect this is because there is another tiny LOAD segment (< 0x1000 bytes) which if rounded to 64k does end up mapping the two sections by accident (but not if rounded to 4k). That's pure luck though.

@delroth delroth added the bug label Sep 25, 2020
@delroth
Copy link
Contributor Author

delroth commented Sep 25, 2020

What seems to be happening is that patchelf is finding some unused space which isn't covered by any PT_LOAD segment. It's placing .dynstr/.dynsym in there, but it's not making sure that these are mapped even though it seems to be required.

A simple workaround should be to just ensure there's a segment covering these, and if not create a PT_LOAD segment. This could be optimized further to instead try and extend an existing segment... but I don't think there is much point to that except creating a "nicer" ELF file.

@domenkozar
Copy link
Member

cc @rpurdie

@delroth
Copy link
Contributor Author

delroth commented Oct 6, 2020

Looking at this a bit more, I was wondering what indicates that .dynsym/.dynstr should be resident in memory (e.g. are they "special" as per some spec, and are there other sections like those). The answer is that there is nothing special about those two sections: there is a generic way to indicate a section should be mapped in memory via the SHF_ALLOC section flag (A in readelf output).

Whatever fix patchelf ends up implementing should likely be using SHF_ALLOC instead of hardcoding the two problematic sections.

@ehmry
Copy link
Contributor

ehmry commented Feb 13, 2021

I've hit the same problem when expanding .dnystr using --replace-needed.

@dreamc-2016
Copy link

I've hit the same problem when expanding .dnystr using --replace-needed.

Have you solved this problem?

@ehmry
Copy link
Contributor

ehmry commented Aug 5, 2022

@dreamc-2016 I must have fixed it in #264. I have a few patches on my fork that do stuff but I haven't tested them on Linux targets so I haven't been keen on merging them.

@haampie
Copy link
Contributor

haampie commented Sep 22, 2022

@ehmry this issue still persists :( can the PR be revived?

To reproduce: (1) build patchelf itself with a short rpath without pie/pic, (2) copy the patchelf executable, (3) set the rpath to something longer, (4) run the modified executable in gdb.

On Ubuntu GCC is configured with --enable-default-pie, so you have to explicitly pass the pie/pic flags.

Example:

$ cd $(mktemp -d)
$ curl -Lfs https://github.com/NixOS/patchelf/releases/download/0.15.0/patchelf-0.15.0.tar.gz | tar -zx --strip-components=1 -f -
$ ./configure
$ make "LDFLAGS=-Wl,-rpath,/example -no-pie" "CXXFLAGS=-O2 -g -fno-pic" install prefix=$PWD/install -j
$ cd install/bin
$ cp patchelf patchelf.copy
$ ./patchelf --set-rpath /longer/path patchelf.copy
$ gdb -ex r --args ./patchelf.copy

Reading symbols from ./patchelf.copy...

warning: Loadable section ".dynstr" outside of ELF segments
Starting program: /tmp/tmp.3gvU6yERyd/install/bin/patchelf.copy 

This is Ubuntu 20.04, GCC 9.4.0, binutils 2.34.

Would a better solution be to add a new pt_load program header covering the moved/new section?


Edit, it's actually a different issue.

Patchelf grows the dynstr section, reshuffles the sections, ends up with two contiguous pt_load program headers, and the dynstr section goes out of bounds of the first pt_load.

@ehmry
Copy link
Contributor

ehmry commented Sep 23, 2022

I will not reopen my PR because I don't have the time to give patchelf patches the attention and care that they need :| .

JPEWdev pushed a commit to JPEWdev/patchelf that referenced this issue Jul 31, 2023
This is not a robust fix, the first PT_LOAD segment is expanded
forward to cover .dynstr or .dynsym.

This is verified to fix segmentation faults in the loader when loading
programs where the .dynstr or .dynstm section gets bumped out of the
LOAD segment

Fixes: NixOS#244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment