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

patchefd --set-rpath ... nodejs broken since ~0.17.2 (0.15.0 works) #531

Open
kvtb opened this issue Nov 5, 2023 · 3 comments · May be fixed by #544
Open

patchefd --set-rpath ... nodejs broken since ~0.17.2 (0.15.0 works) #531

kvtb opened this issue Nov 5, 2023 · 3 comments · May be fixed by #544
Labels

Comments

@kvtb
Copy link

kvtb commented Nov 5, 2023

Describe the bug

patchelf --set-rpath ${stdenv.cc.cc.lib}/lib node produces broken executable with patchelf-0.17.2 and patchelf-0.18.0, no problem with patchelf-0.15.0

the resulting executables crash with Segmentation Failed

ldd node produces no output at all

Steps To Reproduce

node is from this archive: https://nodejs.org/dist/v16.14.2/node-v16.14.2-linux-x64.tar.gz

Then run patchelf --set-rpath ${stdenv.cc.cc.lib}/lib node where ${stdenv.cc.cc.lib} is the path to gcc libs path on your system

Expected behavior

Expected working executable as it was with patchelf-0.15.0

patchelf --version output

0.17.2, 0.18.0

Additional context

Downloaded node may seems ridiculous example, but using downloaded node and protoc it is an usual part of Maven build process.
BTW, there was no problem with patching protoc with any patchelf version

@kvtb kvtb added the bug label Nov 5, 2023
@kvtb kvtb changed the title patchefd --set-rpath ... nodejs broken since ~0.17.2 (15.0 works) patchefd --set-rpath ... nodejs broken since ~0.17.2 (0.15.0 works) Nov 5, 2023
@Patryk27
Copy link
Member

Patryk27 commented Feb 5, 2024

Similar as #244, would (partially) fixed by #264 and (hopefully) will be fixed together with my recent pcloud-related things (since pcloud also relies on node under the hood).

@Patryk27
Copy link
Member

Patryk27 commented Feb 6, 2024

Alright, got it!

The issue is that libnode.so happens to have .rodata placed 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, when growing the program header table, moves somewhere else (which also changes its virtual memory address, aii aii!).

Relocating this section (at least without relocating all the other sections) is illegal, because the underlying assembly might use (and, in this case, does use) RIP-relative addressing, which breaks when the sections are shuffled around in the memory like that.

tl;dr patchelf changes memory address of an *.elf section that is supposed to be non-movable

I wonder if instead of extending the PHDR we could just allocate a new one, at the end of the file? This wouldn't cause this mayhem; I'll try playing with that.

Edit: I should perhaps also mention that allocating .rodata at the beginning of the file is not a crime on its own - I think it's just that most binaries allocate this section somewhere further in the file and that's why this bug remained hidden for so long (because usually the first sections in the *.elf file were something patchelf could freely shuffle around).

@Patryk27
Copy link
Member

Patryk27 commented Feb 6, 2024

Ah, we extend PHDR instead of allocating a new one due to a Linux bug:

Bug in Linux kernel, in fs/binfmt_elf.c:

Considering the commit is from 2005, hopefully that's not the case anymore!

@Patryk27 Patryk27 linked a pull request Feb 6, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants