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

Add option to make the rpath relative under a specified root directory #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grandwolf
Copy link

@grandwolf grandwolf commented Mar 14, 2017

The buildroot project wants to make the SDK (toolchain) tree relocatable allowing to install it at
any location. Furthermore, the target root file system trees needs ELF file sanitation to remove
references to the build system. This patch implements the necessary actions to sanitize the
ELF files under the specified tree. See also [1].

Running "patchelf" with the option "--make-rpath-relative ROOTDIR" will modify or delete
the RPATHDIRs of the RPATH according the following rules:

  • RPATHDIR starts with "$ORIGIN":
    The original build-system already took care of setting a relative RPATH, resolve it and
    test if it's valid (does exist),

  • RPATHDIR starts with ROOTDIR:
    The original build-system added some absolute RPATH (absolute on the build machine).
    Test if it's valid (does exist).

  • ROOTDIR/RPATHDIR exists:
    The original build-system already took care of setting an absolute RPATH (absolute in the
    final rootfs), resolve it and test if it's valid (does exist).

  • RPATHDIR points somewhere else:
    (can be anywhere: build trees, staging tree, host location, non-existing location, etc.).
    Just discard such a path.

In addition, the option "--no-standard-libs" will discard RPATHDIRs "ROOTDIR/lib" and
"ROOTDIR/usr/lib". Like "--shrink-rpath", RPATHDIRs are also discarded if the directories do
not contain a library referenced by DT_NEEDED fields.
If the option "--relative-to-file" is given, the rpath will start with "$ORIGIN" making it relative
to the ELF file, otherwise an absolute path relative to ROOTDIR will be used.

Please comment. If necessary/useful we could also make it more modular, e.g. by extending
"--shrink-rpath".

[1] http://lists.busybox.net/pipermail/buildroot/2016-April/159422.html

Running "patchelf" with the option "--make-rpath-relative ROOTDIR" will
modify or delete the RPATHDIRs according the following rules:

RPATHDIR starts with "$ORIGIN":
    The original build-system already took care of setting a relative
    RPATH, resolve it and test if it's valid according to the rules
    below.

RPATHDIR starts with ROOTDIR:
    The original build-system added some absolute RPATH (absolute on
    the build machine). While this is not OK , it can still be fixed; so
    test if it is worthwhile to keep it.

ROOTDIR/RPATHDIR exists:
    The original build-system already took care of setting an absolute
    RPATH (absolute in the final rootfs), resolve it and test if it's
    worthwhile to keep it.

RPATHDIR points somewhere else:
    (can be anywhere: build trees, staging tree, host location,
    non-existing location, etc.). Just discard such a path.

In addition, the option "--no-standard-libs" will discard RPATHDIRs
ROOTDIR/lib and ROOTDIR/usr/lib. Like "--shrink-rpath", RPATHDIRs
are discarded if the directories do not contain a library
referenced by DT_NEEDED fields.
If the option "--relative-to-file" is given, the rpath will start
with "$ORIGIN" making it relative to the ELF file, otherwise an
absolute path relative to ROOTDIR will be used.

This option is useful to sanitize the ELF files of a target root
filesystem and to help making a SDK/toolchain relocatable, e.g.
of the buildroot project [1].

[1] http://lists.busybox.net/pipermail/buildroot/2016-April/159422.html

fixes
@ebkalderon
Copy link

@grandwolf This looks like a pretty interesting feature. Is this PR still being worked on?

@probonopd
Copy link

probonopd commented Feb 5, 2019

I'd also be interested in this feature, e.g., for use in linuxdeployqt.

@grandwolf
Copy link
Author

grandwolf commented Feb 5, 2019 via email

given, "rootdir/lib" and "rootdir/usr/lib" directories are discarded
as well. If the option "--relative-to-file" is given, the RPATH will
start with "$ORIGIN" making it relative to the ELF file, otherwise
an absolute path relative to ROOTDIR will be used. Furthermore, all
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems bogus. What happens if the application wants to load a shared library with dlopen()?
The shared object will not have a DT_NEEDED entry and the RPATH will get discarded ... then the application will fail at runtime.

woodsts pushed a commit to woodsts/buildroot that referenced this pull request Aug 25, 2020
Our patch
0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch adds
an option --make-rpath-relative, which we use to tweak RPATH of target
binaries.

However, one of the effect of this option is that it drops RPATH
entries if the corresponding directory does not contain a library that
is referenced by a DT_NEEDED entry of the binary.

This unfortunately isn't correct, as RPATH entries are not only used
by the dynamic linker to resolve the location of libraries listed
through DT_NEEDED entries: RPATH entries are also used by dlopen()
when resolving the location of libraries that are loaded at runtime.

Therefore, the removal of RPATH entries that don't correspond to
directories containing libraries referenced by DT_NEEDED entries break
legitimate uses of RPATH for dlopen()ed libraries.

This issue was even pointed out during the review of the upstream pull
request:

  NixOS/patchelf#118 (comment)

This fixes tst-origin uClibc-ng unit test:

https://github.com/wbx-github/uclibc-ng-test/blob/master/test/dlopen/Makefile.in#L25
https://github.com/wbx-github/uclibc-ng-test/blob/master/test/dlopen/tst-origin.c#L15

Without this patch:

$ gcc -o toto toto.c -Wl,-rpath,/tmp/test/bar
$ readelf -d toto | grep PATH
 0x000000000000000f (RPATH)              Library rpath: [/tmp/test/bar]
$ ./output/host/bin/patchelf --debug --make-rpath-relative /tmp/
toto
patching ELF file `toto'
Kernel page size is 4096 bytes
removing directory '/tmp/test/bar' from RPATH because it does not contain needed libs
new rpath is `'
$ readelf -d toto | grep PATH
 0x000000000000001d (RUNPATH)            Library runpath: []

With the patch applied:

$ gcc -o toto toto.c -Wl,-rpath,/tmp/test/bar
$ readelf -d toto | grep PATH
 0x000000000000000f (RPATH)              Library rpath: [/tmp/test/bar]
$ ./output/host/bin/patchelf --debug --make-rpath-relative /tmp/ toto
patching ELF file `toto'
Kernel page size is 4096 bytes
keeping relative path of /tmp/test/bar
new rpath is `test/bar'
$ readelf -d toto | grep PATH
 0x000000000000001d (RUNPATH)            Library runpath: [test/bar]

Signed-off-by: Yann Sionneau <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this pull request Aug 28, 2020
Our patch
0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch adds
an option --make-rpath-relative, which we use to tweak RPATH of target
binaries.

However, one of the effect of this option is that it drops RPATH
entries if the corresponding directory does not contain a library that
is referenced by a DT_NEEDED entry of the binary.

This unfortunately isn't correct, as RPATH entries are not only used
by the dynamic linker to resolve the location of libraries listed
through DT_NEEDED entries: RPATH entries are also used by dlopen()
when resolving the location of libraries that are loaded at runtime.

Therefore, the removal of RPATH entries that don't correspond to
directories containing libraries referenced by DT_NEEDED entries break
legitimate uses of RPATH for dlopen()ed libraries.

This issue was even pointed out during the review of the upstream pull
request:

  NixOS/patchelf#118 (comment)

This fixes tst-origin uClibc-ng unit test:

https://github.com/wbx-github/uclibc-ng-test/blob/master/test/dlopen/Makefile.in#L25
https://github.com/wbx-github/uclibc-ng-test/blob/master/test/dlopen/tst-origin.c#L15

Without this patch:

$ gcc -o toto toto.c -Wl,-rpath,/tmp/test/bar
$ readelf -d toto | grep PATH
 0x000000000000000f (RPATH)              Library rpath: [/tmp/test/bar]
$ ./output/host/bin/patchelf --debug --make-rpath-relative /tmp/
toto
patching ELF file `toto'
Kernel page size is 4096 bytes
removing directory '/tmp/test/bar' from RPATH because it does not contain needed libs
new rpath is `'
$ readelf -d toto | grep PATH
 0x000000000000001d (RUNPATH)            Library runpath: []

With the patch applied:

$ gcc -o toto toto.c -Wl,-rpath,/tmp/test/bar
$ readelf -d toto | grep PATH
 0x000000000000000f (RPATH)              Library rpath: [/tmp/test/bar]
$ ./output/host/bin/patchelf --debug --make-rpath-relative /tmp/ toto
patching ELF file `toto'
Kernel page size is 4096 bytes
keeping relative path of /tmp/test/bar
new rpath is `test/bar'
$ readelf -d toto | grep PATH
 0x000000000000001d (RUNPATH)            Library runpath: [test/bar]

Signed-off-by: Yann Sionneau <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
(cherry picked from commit bcdb745)
Signed-off-by: Peter Korsgaard <[email protected]>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this pull request Aug 28, 2020
Our patch
0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch adds
an option --make-rpath-relative, which we use to tweak RPATH of target
binaries.

However, one of the effect of this option is that it drops RPATH
entries if the corresponding directory does not contain a library that
is referenced by a DT_NEEDED entry of the binary.

This unfortunately isn't correct, as RPATH entries are not only used
by the dynamic linker to resolve the location of libraries listed
through DT_NEEDED entries: RPATH entries are also used by dlopen()
when resolving the location of libraries that are loaded at runtime.

Therefore, the removal of RPATH entries that don't correspond to
directories containing libraries referenced by DT_NEEDED entries break
legitimate uses of RPATH for dlopen()ed libraries.

This issue was even pointed out during the review of the upstream pull
request:

  NixOS/patchelf#118 (comment)

This fixes tst-origin uClibc-ng unit test:

https://github.com/wbx-github/uclibc-ng-test/blob/master/test/dlopen/Makefile.in#L25
https://github.com/wbx-github/uclibc-ng-test/blob/master/test/dlopen/tst-origin.c#L15

Without this patch:

$ gcc -o toto toto.c -Wl,-rpath,/tmp/test/bar
$ readelf -d toto | grep PATH
 0x000000000000000f (RPATH)              Library rpath: [/tmp/test/bar]
$ ./output/host/bin/patchelf --debug --make-rpath-relative /tmp/
toto
patching ELF file `toto'
Kernel page size is 4096 bytes
removing directory '/tmp/test/bar' from RPATH because it does not contain needed libs
new rpath is `'
$ readelf -d toto | grep PATH
 0x000000000000001d (RUNPATH)            Library runpath: []

With the patch applied:

$ gcc -o toto toto.c -Wl,-rpath,/tmp/test/bar
$ readelf -d toto | grep PATH
 0x000000000000000f (RPATH)              Library rpath: [/tmp/test/bar]
$ ./output/host/bin/patchelf --debug --make-rpath-relative /tmp/ toto
patching ELF file `toto'
Kernel page size is 4096 bytes
keeping relative path of /tmp/test/bar
new rpath is `test/bar'
$ readelf -d toto | grep PATH
 0x000000000000001d (RUNPATH)            Library runpath: [test/bar]

Signed-off-by: Yann Sionneau <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
(cherry picked from commit bcdb745)
Signed-off-by: Peter Korsgaard <[email protected]>
@jraygauthier
Copy link
Member

Use patchelf --set-rpath "\$ORIGIN/my-sub-dir-if-any" my-exe. It worked fine for me.

@ffontaine
Copy link

Does this PR could be merged in a near future?

@piegamesde
Copy link
Member

I'm not involved in the project, but the formatting looks inconsistent (is there an auto formatter configured?). Especially, tabs and spaces are mixed.

@fallen
Copy link

fallen commented Sep 19, 2021

Does this PR could be merged in a near future?

Hello, I think the comment I posted has not been addressed by the PR.
I think it is still a problem to remove directories from RPATH just because they don't contain any library pointed by a DT_NEEDED entry.

@follower
Copy link

@fallen FWIW I think your concerns might be addressed (for buildroot, at least, presumably) with: buildroot/buildroot@5be9c96

Those changes appear listed on the issue page of #118 but might not be obvious, e.g. if you get notifications via email.

See, e.g.: #118 (reference)

However GH is still reporting file conflicts.

@ffontaine
Copy link

I can open a new PR to include buildroot/buildroot@5be9c96 and fix those file conflicts based on the patch that I sent to buildroot a few days ago: https://patchwork.ozlabs.org/project/buildroot/patch/[email protected]/

fallen added a commit to kalray/buildroot that referenced this pull request Dec 13, 2021
Our patch
0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch adds
an option --make-rpath-relative, which we use to tweak RPATH of target
binaries.

However, one of the effect of this option is that it drops RPATH
entries if the corresponding directory does not contain a library that
is referenced by a DT_NEEDED entry of the binary.

This unfortunately isn't correct, as RPATH entries are not only used
by the dynamic linker to resolve the location of libraries listed
through DT_NEEDED entries: RPATH entries are also used by dlopen()
when resolving the location of libraries that are loaded at runtime.

Therefore, the removal of RPATH entries that don't correspond to
directories containing libraries referenced by DT_NEEDED entries break
legitimate uses of RPATH for dlopen()ed libraries.

This issue was even pointed out during the review of the upstream pull
request:

  NixOS/patchelf#118 (comment)

This fixes tst-origin uClibc-ng unit test:

https://github.com/wbx-github/uclibc-ng-test/blob/master/test/dlopen/Makefile.in#L25
https://github.com/wbx-github/uclibc-ng-test/blob/master/test/dlopen/tst-origin.c#L15

Without this patch:

$ gcc -o toto toto.c -Wl,-rpath,/tmp/test/bar
$ readelf -d toto | grep PATH
 0x000000000000000f (RPATH)              Library rpath: [/tmp/test/bar]
$ ./output/host/bin/patchelf --debug --make-rpath-relative /tmp/
toto
patching ELF file `toto'
Kernel page size is 4096 bytes
removing directory '/tmp/test/bar' from RPATH because it does not contain needed libs
new rpath is `'
$ readelf -d toto | grep PATH
 0x000000000000001d (RUNPATH)            Library runpath: []

With the patch applied:

$ gcc -o toto toto.c -Wl,-rpath,/tmp/test/bar
$ readelf -d toto | grep PATH
 0x000000000000000f (RPATH)              Library rpath: [/tmp/test/bar]
$ ./output/host/bin/patchelf --debug --make-rpath-relative /tmp/ toto
patching ELF file `toto'
Kernel page size is 4096 bytes
keeping relative path of /tmp/test/bar
new rpath is `test/bar'
$ readelf -d toto | grep PATH
 0x000000000000001d (RUNPATH)            Library runpath: [test/bar]

Signed-off-by: Yann Sionneau <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
@QGB
Copy link

QGB commented May 8, 2023

/data/data/com.termux/files/usr/bin/bash: termux-change-repo: command not found

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 this pull request may close these issues.

9 participants