From cb929ba3d3e458872abe8fc126e0f811ab49f424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sun, 23 Sep 2018 14:45:32 +0100 Subject: [PATCH 01/10] autoPatchelfHook: do not patch statically linked files Also speed up quite significantly due less forking. (cherry picked from commit 58a97dfb491be6ae92499c3f819440f281d826a1) --- .../setup-hooks/auto-patchelf.sh | 25 ++++++------------- pkgs/top-level/all-packages.nix | 3 +-- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 32fdb1000e27..94ea3e4e98e3 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -7,21 +7,7 @@ gatherLibraries() { addEnvHooks "$targetOffset" gatherLibraries isExecutable() { - [ "$(file -b -N --mime-type "$1")" = application/x-executable ] -} - -findElfs() { - find "$1" -type f -exec "$SHELL" -c ' - while [ -n "$1" ]; do - mimeType="$(file -b -N --mime-type "$1")" - if [ "$mimeType" = application/x-executable \ - -o "$mimeType" = application/x-pie-executable \ - -o "$mimeType" = application/x-sharedlib ]; then - echo "$1" - fi - shift - done - ' -- {} + + readelf -h "$1" | grep -q '^ *Type: *EXEC\>' } # We cache dependencies so that we don't need to search through all of them on @@ -167,9 +153,12 @@ autoPatchelf() { # findDependency outside of this, the dependency cache needs to be rebuilt # from scratch, so keep this in mind if you want to run findDependency # outside of this function. - findElfs "$prefix" | while read -r elffile; do - autoPatchelfFile "$elffile" - done + while IFS= read -r -d $'\0' file; do + isELF "$file" || continue + # dynamically linked? + readelf -l "$file" | grep -q "^ *INTERP\\>" || continue + autoPatchelfFile "$file" + done < <(find "$prefix" -type f -print0) } # XXX: This should ultimately use fixupOutputHooks but we currently don't have diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix index 6b0a0a6e500d..b4d66385424d 100644 --- a/pkgs/top-level/all-packages.nix +++ b/pkgs/top-level/all-packages.nix @@ -81,8 +81,7 @@ with pkgs; { deps = [ autoconf264 automake111x gettext libtool ]; } ../build-support/setup-hooks/autoreconf.sh; - autoPatchelfHook = makeSetupHook - { name = "auto-patchelf-hook"; deps = [ file ]; } + autoPatchelfHook = makeSetupHook { name = "auto-patchelf-hook"; } ../build-support/setup-hooks/auto-patchelf.sh; ensureNewerSourcesHook = { year }: makeSetupHook {} From 7b0e45e482ad51a42aeafa49cb13031201f19076 Mon Sep 17 00:00:00 2001 From: aszlig Date: Tue, 25 Sep 2018 04:11:33 +0200 Subject: [PATCH 02/10] autoPatchelfHook: Only check PT_INTERP on execs If the ELF file is not an executable, we do not get a PT_INTERP section, because after all, it's a *shared* library. So instead of checking for PT_INTERP (to avoid statically linked executables) for all ELF files, we add another check to see if it's an executable and *only* skip it when it is and there's no PT_INTERP. Signed-off-by: aszlig (cherry picked from commit 9920215d005db148564e826478474faa236a4e75) --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 94ea3e4e98e3..f808cd9f78d7 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -155,8 +155,10 @@ autoPatchelf() { # outside of this function. while IFS= read -r -d $'\0' file; do isELF "$file" || continue - # dynamically linked? - readelf -l "$file" | grep -q "^ *INTERP\\>" || continue + if isExecutable "$file"; then + # Skip if the executable is statically linked. + readelf -l "$file" | grep -q "^ *INTERP\\>" || continue + fi autoPatchelfFile "$file" done < <(find "$prefix" -type f -print0) } From f4136c19b519ed17bcc3d8aa6d9c86a0d11f2145 Mon Sep 17 00:00:00 2001 From: aszlig Date: Tue, 25 Sep 2018 04:48:12 +0200 Subject: [PATCH 03/10] autoPatchelfHook: Silence errors in isExecutable The "maxx" package recursively runs isExecutable on a bunch of files and since the change to use "readelf" instead of "file" a lot of errors like this one are printed during build: readelf: Error: Not an ELF file - it has the wrong magic bytes at the start While the isExecutable was never meant to be used outside of the autoPatchelfHook, it's still a good idea to silence the errors because whenever readelf fails, it clearly indicates that the file in question is not a valid ELF file. Signed-off-by: aszlig (cherry picked from commit b4526040a21303a1822fcad348e9b9e39a3876b9) --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index f808cd9f78d7..7c165627f72e 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -7,7 +7,7 @@ gatherLibraries() { addEnvHooks "$targetOffset" gatherLibraries isExecutable() { - readelf -h "$1" | grep -q '^ *Type: *EXEC\>' + readelf -h "$1" 2> /dev/null | grep -q '^ *Type: *EXEC\>' } # We cache dependencies so that we don't need to search through all of them on From d3c7d378069240eee19c8fcd4e3964a601ff7427 Mon Sep 17 00:00:00 2001 From: aszlig Date: Sat, 3 Nov 2018 04:50:26 +0100 Subject: [PATCH 04/10] autoPatchelfHook: Correctly detect PIE binaries I originally thought it would just be enough to just check for an INTERP section in isExecutable, however this would mean that we don't detect statically linked ELF files, which would break our recent improvement to gracefully handle those. In theory, we are only interested in ELF files that have an INTERP section, so checking for INTERP would be enough. Unfortunately the isExecutable function is already used outside of autoPatchelfHook, so we can't easily get rid of it now, so let's actually strive for more correctness and make isExecutable actually match ELF files that are executable. So what we're doing instead now is to check whether either the ELF type is EXEC *or* we have an INTERP section and if one of them is true we should have an ELF executable, even if it's statically linked. Along the way I also set LANG=C for the invocations of readelf, just to be sure we don't get locale-dependent output. Tested this with the following command (which contains almost[1] all the packages using autoPatchelfHook), checking whether we run into any library-related errors: nix-build -E 'with import ./. { config.allowUnfree = true; }; runCommand "test-executables" { drvs = [ anydesk cups-kyodialog3 elasticsearch franz gurobi masterpdfeditor oracle-instantclient powershell reaper sourcetrail teamviewer unixODBCDrivers.msodbcsql17 virtlyst vk-messenger wavebox zoom-us ]; } ("for i in $drvs; do for b in $i/bin/*; do " + "[ -x \"$b\" ] && timeout 10 \"$b\" || :; done; done") ' Apart from testing against library-related errors I also compared the resulting store paths against the ones prior to this commit. Only anydesk and virtlyst had the same as they didn't have self-references, everything else differed only because of self-references, except elasticsearch, which had the following PIE binaries: * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/autoconfig * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/autodetect * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/categorize * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/controller * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/normalize These binaries were now patched, which is what this commit is all about. [1]: I didn't include the "maxx" package (MaXX Interactive Desktop) because the upstream URLs are no longer existing and I couldn't find them elsewhere on the web. Signed-off-by: aszlig Fixes: https://github.com/NixOS/nixpkgs/issues/48330 Cc: @gnidorah (for MaXX Interactive Desktop) (cherry picked from commit c64624b8435af07d95effbb928d955963a5fa859) --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 7c165627f72e..d1ae317ff9a4 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -7,7 +7,16 @@ gatherLibraries() { addEnvHooks "$targetOffset" gatherLibraries isExecutable() { - readelf -h "$1" 2> /dev/null | grep -q '^ *Type: *EXEC\>' + # For dynamically linked ELF files it would be enough to check just for the + # INTERP section. However, we won't catch statically linked executables as + # they only have an ELF type of EXEC but no INTERP. + # + # So what we do here is just check whether *either* the ELF type is EXEC + # *or* there is an INTERP section. This also catches position-independent + # executables, as they typically have an INTERP section but their ELF type + # is DYN. + LANG=C readelf -h -l "$1" 2> /dev/null \ + | grep -q '^ *Type: *EXEC\>\|^ *INTERP\>' } # We cache dependencies so that we don't need to search through all of them on @@ -157,7 +166,7 @@ autoPatchelf() { isELF "$file" || continue if isExecutable "$file"; then # Skip if the executable is statically linked. - readelf -l "$file" | grep -q "^ *INTERP\\>" || continue + LANG=C readelf -l "$file" | grep -q "^ *INTERP\\>" || continue fi autoPatchelfFile "$file" done < <(find "$prefix" -type f -print0) From c6fbed235ac5bcb2bc3a0274e64290ff4babc129 Mon Sep 17 00:00:00 2001 From: aszlig Date: Mon, 19 Nov 2018 17:18:27 +0100 Subject: [PATCH 05/10] autoPatchelfHook: Make easier to run autoPatchelf The autoPatchelf main function which is run against all of the outputs was pretty much tailored towards this specific setup-hook and was relying on $prefix to be set globally. So if you wanted to run autoPatchelf manually - let's say during buildPhase - you would have needed to run it like this: prefix=/some/directory autoPatchelf This is now more intuitive and all you need to do is run the following: autoPatchelf /some/directory Signed-off-by: aszlig (cherry picked from commit d03e4ffdbf9849f7e928c7a95be76aac69780c55) --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index d1ae317ff9a4..59e9a933211f 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -153,7 +153,7 @@ autoPatchelf() { # Add all shared objects of the current output path to the start of # cachedDependencies so that it's choosen first in findDependency. cachedDependencies+=( - $(find "$prefix" \! -type d \( -name '*.so' -o -name '*.so.*' \)) + $(find "$@" \! -type d \( -name '*.so' -o -name '*.so.*' \)) ) local elffile @@ -169,7 +169,7 @@ autoPatchelf() { LANG=C readelf -l "$file" | grep -q "^ *INTERP\\>" || continue fi autoPatchelfFile "$file" - done < <(find "$prefix" -type f -print0) + done < <(find "$@" -type f -print0) } # XXX: This should ultimately use fixupOutputHooks but we currently don't have @@ -181,5 +181,5 @@ autoPatchelf() { # behaviour as fixupOutputHooks because the setup hook for patchelf is run in # fixupOutput and the postFixup hook runs later. postFixupHooks+=( - 'for output in $outputs; do prefix="${!output}" autoPatchelf; done' + 'autoPatchelf $(for output in $outputs; do echo "${!output}"; done)' ) From f4eb9c59935a3986cec624f2d61bf8101e48a986 Mon Sep 17 00:00:00 2001 From: aszlig Date: Mon, 19 Nov 2018 17:36:22 +0100 Subject: [PATCH 06/10] autoPatchelfHook: Allow to prevent automatic run If you want to only run autoPatchelf on a specific path and leave everything else alone, we now have a $dontAutoPatchelf environment variable, which causes the postFixup hook to not run at all. The name "dontAutoPatchelf" probably is a bit weird in conjunction with putting "autoPatchelfHook" in nativeBuildInputs, but unless someone comes up with a better name I keep it that way because it's consistent with all the other dontStrip, dontPatchShebangs, dontPatchELF and whatnot. A specific example where this is needed is when building the Android SDK emulator, which contains a few ARM binaries in subdirectories that should not be patched. If we were to run autoPatchelf on all outputs unconditionally we'd run into errors because some ARM libraries couldn't be found. Signed-off-by: aszlig (cherry picked from commit e4fbb244ee313a3003144e8e148341c9e5c67295) --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 59e9a933211f..62348d71ed05 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -180,6 +180,11 @@ autoPatchelf() { # So what we do here is basically run in postFixup and emulate the same # behaviour as fixupOutputHooks because the setup hook for patchelf is run in # fixupOutput and the postFixup hook runs later. -postFixupHooks+=( - 'autoPatchelf $(for output in $outputs; do echo "${!output}"; done)' -) +postFixupHooks+=(' + if [ -z "$dontAutoPatchelf" ]; then + autoPatchelf $(for output in $outputs; do + [ -e "${!output}" ] || continue + echo "${!output}" + done) + fi +') From 83b7d5813a515b60dc7fb87495bd2e4f33b2ecc8 Mon Sep 17 00:00:00 2001 From: aszlig Date: Mon, 19 Nov 2018 23:23:38 +0100 Subject: [PATCH 07/10] autoPatchelfHook: Add --no-recurse flag This is to be used with the autoPatchelf command and allows to only patch a specific file or directory without recursing into subdirectories. Apart from being able to run the command in a standalone way, as detailled in the previous commit this is also needed for the Android SDK emulator, because according to @svanderburg there are subdirectories we don't want to patch. The reason why I didn't use GNU getopt is that it might not be available on all operating systems and the getopts bash builtin doesn't support long arguments. Apart from that, the implementation for recognizing the flag is pretty trivial and it's also using bash builtins only, so if we want to do something really fancy someday, we can still change it. Signed-off-by: aszlig (cherry picked from commit 3ca35ce0b2dd1adb11044e03a816c77a72f7135d) --- .../setup-hooks/auto-patchelf.sh | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 62348d71ed05..43b1679670d4 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -148,12 +148,32 @@ autoPatchelfFile() { } autoPatchelf() { + local -a findOpts=() + + while [ $# -gt 0 ]; do + case "$1" in + --) shift; break;; + --no-recurse) shift; findOpts+=("-maxdepth" 1);; + --*) + echo "autoPatchelf: ERROR: Invalid command line" \ + "argument: $1" >&2 + return 1;; + *) break;; + esac + done + + if [ $# -eq 0 ]; then + echo "autoPatchelf: No paths to patch specified." >&2 + return 1 + fi + echo "automatically fixing dependencies for ELF files" >&2 # Add all shared objects of the current output path to the start of # cachedDependencies so that it's choosen first in findDependency. cachedDependencies+=( - $(find "$@" \! -type d \( -name '*.so' -o -name '*.so.*' \)) + $(find "$@" "${findOpts[@]}" \! -type d \ + \( -name '*.so' -o -name '*.so.*' \)) ) local elffile @@ -169,7 +189,7 @@ autoPatchelf() { LANG=C readelf -l "$file" | grep -q "^ *INTERP\\>" || continue fi autoPatchelfFile "$file" - done < <(find "$@" -type f -print0) + done < <(find "$@" "${findOpts[@]}" -type f -print0) } # XXX: This should ultimately use fixupOutputHooks but we currently don't have @@ -182,7 +202,7 @@ autoPatchelf() { # fixupOutput and the postFixup hook runs later. postFixupHooks+=(' if [ -z "$dontAutoPatchelf" ]; then - autoPatchelf $(for output in $outputs; do + autoPatchelf -- $(for output in $outputs; do [ -e "${!output}" ] || continue echo "${!output}" done) From 2586da9d0ea4fb4944a1556f7a540e0023fcec87 Mon Sep 17 00:00:00 2001 From: aszlig Date: Sun, 25 Nov 2018 16:17:14 +0100 Subject: [PATCH 08/10] autoPatchelfHook: Add addAutoPatchelfSearchPath This function is useful if autoPatchelf is invoked during some of the phases of a build and allows to add arbitrary shared objects to the search path. So far the same functionality was in autoPatchelf itself, but not available as a separate function, so when adding shared objects to the dependency cache one would have to do so manually. The function also has the --no-recurse flag, which prevents recursing into subdirectories. Signed-off-by: aszlig (cherry picked from commit 2faf905f98eeb9f674ef845ae9bf9fd7f1279d1e) --- .../setup-hooks/auto-patchelf.sh | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 43b1679670d4..61bdafa88c37 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -147,13 +147,38 @@ autoPatchelfFile() { fi } -autoPatchelf() { +# Can be used to manually add additional directories with shared object files +# to be included for the next autoPatchelf invocation. +addAutoPatchelfSearchPath() { local -a findOpts=() + # XXX: Somewhat similar to the one in the autoPatchelf function, maybe make + # it DRY someday... while [ $# -gt 0 ]; do case "$1" in --) shift; break;; --no-recurse) shift; findOpts+=("-maxdepth" 1);; + --*) + echo "addAutoPatchelfSearchPath: ERROR: Invalid command line" \ + "argument: $1" >&2 + return 1;; + *) break;; + esac + done + + cachedDependencies+=( + $(find "$@" "${findOpts[@]}" \! -type d \ + \( -name '*.so' -o -name '*.so.*' \)) + ) +} + +autoPatchelf() { + local -a norecurse= + + while [ $# -gt 0 ]; do + case "$1" in + --) shift; break;; + --no-recurse) shift; norecurse=1;; --*) echo "autoPatchelf: ERROR: Invalid command line" \ "argument: $1" >&2 @@ -171,11 +196,7 @@ autoPatchelf() { # Add all shared objects of the current output path to the start of # cachedDependencies so that it's choosen first in findDependency. - cachedDependencies+=( - $(find "$@" "${findOpts[@]}" \! -type d \ - \( -name '*.so' -o -name '*.so.*' \)) - ) - local elffile + addAutoPatchelfSearchPath ${norecurse:+--no-recurse} -- "$@" # Here we actually have a subshell, which also means that # $cachedDependencies is final at this point, so whenever we want to run @@ -189,7 +210,7 @@ autoPatchelf() { LANG=C readelf -l "$file" | grep -q "^ *INTERP\\>" || continue fi autoPatchelfFile "$file" - done < <(find "$@" "${findOpts[@]}" -type f -print0) + done < <(find "$@" ${norecurse:+-maxdepth 1} -type f -print0) } # XXX: This should ultimately use fixupOutputHooks but we currently don't have From 66de7c793938ff63b46b55b2ce0519454eb1b64d Mon Sep 17 00:00:00 2001 From: aszlig Date: Mon, 26 Nov 2018 01:13:59 +0100 Subject: [PATCH 09/10] autoPatchelfHook: Fix type of norecurse variable While declaring it as an array doesn't do any harm in our usage, it might be a bit confusing when reading the code. Signed-off-by: aszlig (cherry picked from commit 9f23a63f79e1a9d4a7f51444c12605d73ec770e1) --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 61bdafa88c37..9a124f415b8c 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -173,7 +173,7 @@ addAutoPatchelfSearchPath() { } autoPatchelf() { - local -a norecurse= + local norecurse= while [ $# -gt 0 ]; do case "$1" in From 10ffd6f112b3da8ce9700d693ec79487f2bcb127 Mon Sep 17 00:00:00 2001 From: aszlig Date: Mon, 26 Nov 2018 01:58:36 +0100 Subject: [PATCH 10/10] autoPatchelfHook: Skip on missing segment headers If the file in question is not a shared object file but an ELF, we really want to skip the file, because we won't have anything to patch there. For example if the file is created via "gcc -c -o foo.o foo.c", we don't get a segment header and so far autoPatchelf was trying to patch such a file. By checking for missing segment headers, we're now no longer going to attempt patching such a file. Signed-off-by: aszlig Reported-by: Sander van der Burg (cherry picked from commit 4a6e3e4185dcbe53c3d5eceaf2570d51516ad2b4) --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 9a124f415b8c..5bedd1a9f9ca 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -205,9 +205,12 @@ autoPatchelf() { # outside of this function. while IFS= read -r -d $'\0' file; do isELF "$file" || continue + segmentHeaders="$(LANG=C readelf -l "$file")" + # Skip if the ELF file doesn't have segment headers (eg. object files). + echo "$segmentHeaders" | grep -q '^Program Headers:' || continue if isExecutable "$file"; then # Skip if the executable is statically linked. - LANG=C readelf -l "$file" | grep -q "^ *INTERP\\>" || continue + echo "$segmentHeaders" | grep -q "^ *INTERP\\>" || continue fi autoPatchelfFile "$file" done < <(find "$@" ${norecurse:+-maxdepth 1} -type f -print0)