patchelf icon indicating copy to clipboard operation
patchelf copied to clipboard

Allocate PHT & SHT at the end of the *.elf file

Open Patryk27 opened this issue 1 year ago • 2 comments

Closes https://github.com/NixOS/patchelf/issues/531. Closes https://github.com/NixOS/patchelf/issues/482. Closes https://github.com/NixOS/patchelf/issues/244.

Upstream-wise, affects https://github.com/NixOS/nixpkgs/issues/226339. (didn't want to write close so that merging this merge request doesn't close that issue at once)

Abstract

Patching an *.elf file might require extending its program header table, which can then cause it to run out of its originally allocated space (both in terms of file offset and virtual memory).

Currently patchelf solves this problem by finding which sections would overlap PHT and then by moving them to the end of *.elf file:

https://github.com/NixOS/patchelf/blob/7c2f768bf9601268a4e71c2ebe91e2011918a70f/src/patchelf.cc#L832

As compared to similar logic for binaries:

https://github.com/NixOS/patchelf/blob/7c2f768bf9601268a4e71c2ebe91e2011918a70f/src/patchelf.cc#L964

... the logic for libraries is missing a crucial check: it doesn't take into account whether that particular section can be shuffled around - in particular, sections with SHT_PROGBITS can't!

As luck would have it, libnode.so (e.g. shipped together with pcloud) does have .rodata (a section with SHT_PROGBITS active) right at the beginning of the file:

; readelf -S libnode.so

There are 29 section headers, starting at offset 0x152bf70:

Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 0]                   NULL             0000000000000000  00000000
       0000000000000000  0000000000000000           0     0     0
  [ 1] .rodata           PROGBITS         0000000000000240  00000240
       00000000003796a8  0000000000000000 AMS       0     0     32
/* ... */

... which patchelf happily moves around, breaking RIP-relative addressing in the assembly (which, after patching, tries to access the ZZZZ-ed memory).

This commit fixes the issue by changing the logic from:

if there's no space for PHT, shuffle sections around

... to, perhaps a bit more wasteful in terms of storage:

just always allocate PHT at the end of the file

As far as I've checked, the reason why PHT was so strictly kept at the beginning was an old Linux bug:

https://github.com/NixOS/patchelf/blob/7c2f768bf9601268a4e71c2ebe91e2011918a70f/src/patchelf.cc#L857 https://github.com/NixOS/patchelf/blob/7c2f768bf9601268a4e71c2ebe91e2011918a70f/BUGS#L1

... which is not present anymore (not sure when precisely was it fixed, though - the original entry in the BUGS file is dated 2005).

Seizing the day, I'm also including another fix (for binaries), so that merging this pull request will solve all pcloud-related problems.

Patryk27 avatar Feb 06 '24 18:02 Patryk27

Btw, I've got another merge request in the works, one that fixes https://github.com/NixOS/patchelf/issues/75 and hopefully will be able to handle https://github.com/NixOS/patchelf/issues/520 as well, but I've ultimately decided to move it out of this pull request, since it's technically a separate changeset (and less important, at least from my point of view) -- I'll prepare it in the upcoming weeks.

Patryk27 avatar Feb 08 '24 17:02 Patryk27

Another issue is that strip doesn't like binaries with with the PHT and SHT bits at the end, and will either fail or generate invalid binaries as output. One workaround is to just strip first, but I guess this is not possible in all workflows.

See #117 and #10.

Definitely seems like a strip/libbfd bug since, as far as can tell, nothing in the ELF spec requires these tables to be at the top. I spent a couple days stepping through bfd/elf.c back when I was working on this (which also places the updated headers at the end), but I was ultimately unable to decipher what was going on.

jvolkman avatar Mar 07 '24 00:03 jvolkman