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 --replace-needed forgets to modify .gnu.version_r section #84

Closed
njsmith opened this issue Apr 1, 2016 · 5 comments · Fixed by #85
Closed

patchelf --replace-needed forgets to modify .gnu.version_r section #84

njsmith opened this issue Apr 1, 2016 · 5 comments · Fixed by #85

Comments

@njsmith
Copy link
Contributor

njsmith commented Apr 1, 2016

If I have an ELF binary that is linked to a DSO that uses symbol versioning, then the DSO's SONAME appears in two different places: once as a DT_NEEDED entry, and once in the .gnu.version_r version requirements section. If I use patchelf --replace-needed, then it updates the DT_NEEDED entry, but doesn't update the .gnu.version_r table. The resulting binary is completely broken -- trying to load it triggers an assertion failure inside the dynamic loader, as it tries to check the version of a library that was never loaded:

Inconsistency detected by ld.so: dl-version.c: 224: _dl_check_map_versions: Assertion `needed != ((void *)0)' failed!

For example, here's readelf output for a library that's had patchelf --replace-needed libgfortran.so.3 libgfortran-ed201abd.so.3.0.0 run on it:

$ readelf -a lib/python2.7/site-packages/numpy/.libs/libopenblasp-r0-4830d766.2.17.so
[...]
Dynamic section at offset 0x2318100 contains 28 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libgfortran-ed201abd.so.3.0.0]
[...]
Version needs section '.gnu.version_r' contains 4 entries:
 Addr: 0x00000000000aff18  Offset: 0x0aff18  Link: 28 (.dynstr)
  000000: Version: 1  File: libgfortran.so.3  Cnt: 1
  0x0010:   Name: GFORTRAN_1.0  Flags: none  Version: 7

Notice that the DT_NEEDED entry is correct, but the .gnu.version_r entry still refers to the original libgfortran.so.3.

The fix is simple, at least conceptually :-). patchelf --replace-needed should check for any references to the replaced SONAME inside the .gnu.version_r section and update it if necessary.

This bug is a show-stopper for a major effort to enable binary distribution of Python packages as a standard feature on Linux -- we need patchelf --replace-needed to work around this glibc bug or misfeature in the presence of vendored libraries. So FYI, if there's anything we can do to help fix this then we're extremely motivated to do that -- if you don't have time to work on it but can provide hints on where to look then that'd be very much appreciated, or we could probably find some $$ to fund work if that would make a difference...

(Original issue/diagnosis is in pypa/auditwheel#25)

@njsmith
Copy link
Contributor Author

njsmith commented Apr 1, 2016

(sorry, fat-fingered that and submitted the new issue before writing it. Have now edited the original issue to actually describe what's happening)

njsmith added a commit to njsmith/patchelf that referenced this issue Apr 1, 2016
@njsmith
Copy link
Contributor Author

njsmith commented Apr 1, 2016

CC: @rmcgibbo @matthew-brett @ogrisel

It looks like the .gnu.version_r section is very simple: http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/symversion.html#SYMVERRQMTS

there's a start towards fixing this here: master...njsmith:master

just missing the core logic, and also it's probably totally broken because it was written in an hour starting from knowing nothing about ELF or patchelf internals and has never even been compiled :-) but that's how far I managed to get today so figured I'd make it public

njsmith added a commit to njsmith/patchelf that referenced this issue Apr 1, 2016
If the ELF binary that we're patching is linked to a DSO that uses
symbol versioning, then the DSO's SONAME appears in two different
places: once as a DT_NEEDED entry, and once in the .gnu.version_r
version requirements section. Previously, patchelf --replace-needed
would update DT_NEEDED entry, but fail to update the .gnu.version_r
table. This resulted in completely broken binaries -- trying to load
them would trigger an assertion failure inside the dynamic loader, as it
tries to check the version of a library that was never loaded:

  Inconsistency detected by ld.so: dl-version.c: 224: _dl_check_map_versions: Assertion `needed != ((void *)0)' failed!

This commit teaches --replace-needed to update the .gnu.version_r
table.

Fixes: NixOSgh-84
@njsmith
Copy link
Contributor Author

njsmith commented Apr 1, 2016

The branch above now seems to work... probably still need to add a test case, though

njsmith added a commit to njsmith/rpath-weirdness that referenced this issue Apr 1, 2016
This more realistic test triggers
  NixOS/patchelf#84
so that the workaround fails on patchelf 0.9.
@njsmith
Copy link
Contributor Author

njsmith commented Apr 1, 2016

PR: #85

@langeseb
Copy link

The same issue is also true for --remove-needed option.
I wonder, if the same fix could also be used in that case.
Further: the fix replaces the lib within the gnu.version_r section, but leaves the meta info/comment/description line untouched, e.g.:
0x0000 Version 1 File: libReplaced.so Cnt: 1
0x0010 Name: [OLDLIBNAME] Flags: [OLDFLAGS] Version: [OLDVERSION]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants