[pacman-dev] [PATCH] libmakepkg: Support file 5.33's application/x-pie-executable
file 5.33 introduces a new MIME type "application/x-pie-executable", which is used for relocatable binaries. makepkg ignored these binaries and did not attempt to strip them. Handle the new MIME type like the old "application/x-sharedlib". Stripping the binaries with --strip-unneeded to keep relocation information should be the correct thing to do. file 5.33 also misidentifies actual libraries as PIE executables, so we didn't strip any shared libraries, either. We now work around this bug. Signed-off-by: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com> --- scripts/libmakepkg/tidy/strip.sh.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/libmakepkg/tidy/strip.sh.in b/scripts/libmakepkg/tidy/strip.sh.in index e20114c0..36d1b89e 100644 --- a/scripts/libmakepkg/tidy/strip.sh.in +++ b/scripts/libmakepkg/tidy/strip.sh.in @@ -125,6 +125,8 @@ tidy_strip() { esac;; *application/x-executable*) # Binaries strip_flags="$STRIP_BINARIES";; + *application/x-pie-executable*) # Relocatable binaries + strip_flags="$STRIP_SHARED";; *) continue ;; esac -- 2.17.0
2018-04-20 19:46 GMT+02:00 Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>:
file 5.33 introduces a new MIME type "application/x-pie-executable", which is used for relocatable binaries. makepkg ignored these binaries and did not attempt to strip them.
Handle the new MIME type like the old "application/x-sharedlib". Stripping the binaries with --strip-unneeded to keep relocation information should be the correct thing to do.
file 5.33 also misidentifies actual libraries as PIE executables, so we didn't strip any shared libraries, either. We now work around this bug.
Signed-off-by: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com> --- scripts/libmakepkg/tidy/strip.sh.in | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/scripts/libmakepkg/tidy/strip.sh.in b/scripts/libmakepkg/tidy/strip.sh.in index e20114c0..36d1b89e 100644 --- a/scripts/libmakepkg/tidy/strip.sh.in +++ b/scripts/libmakepkg/tidy/strip.sh.in @@ -125,6 +125,8 @@ tidy_strip() { esac;; *application/x-executable*) # Binaries strip_flags="$STRIP_BINARIES";; + *application/x-pie-executable*) # Relocatable binaries + strip_flags="$STRIP_SHARED";; *) continue ;; esac
I don't think that's enough .. since everything , .so , .a ( possible glibc2.7+gcc8 ) and any binary is now marked application/x-pie-executable in PIE builds. So probably something like this is what you wish : https://github.com/abucodonosor/pacman/commit/f38df66ad39383dbce92cef53d3da7... Regards, Gabriel C
On Sun, Apr 22, 2018, 18:22 Gabriel C <nix.or.die@gmail.com> wrote:
I don't think that's enough .. since everything , .so , .a ( possible glibc2.7+gcc8 ) and any binary is now marked application/x-pie-executable in PIE builds.
So probably something like this is what you wish :
https://github.com/abucodonosor/pacman/commit/f38df66ad39383dbce92cef53d3da7...
No, I think that's wrong. You're also removing the relocation information from the PIE binaries.
2018-04-22 21:53 GMT+02:00 Jan Alexander Steffens <jan.steffens@gmail.com>:
On Sun, Apr 22, 2018, 18:22 Gabriel C <nix.or.die@gmail.com> wrote:
I don't think that's enough .. since everything , .so , .a ( possible glibc2.7+gcc8 ) and any binary is now marked application/x-pie-executable in PIE builds.
So probably something like this is what you wish :
https://github.com/abucodonosor/pacman/commit/f38df66ad39383dbce92cef53d3da7...
No, I think that's wrong. You're also removing the relocation information from the PIE binaries.
So explain why that is wrong and how is different to what is done right now .. If you say that is wrong then you say is wrong in general. Also how this removes realocation informations ? That would be then broken for <file 5.33 ,right ? Did you run at least plain 'file' to see what file reports now no matter you have a shared lib , execuable or a static lib ? $ file ./zstd ./zstd: ELF 64-bit LSB pie executable x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 4.4.113, BuildID[sha1]=ad32b90631979b66db1e61d4cf94ae62f74c7656, not stripped $ file ./libzstd.so.1.3.4 ./libzstd.so.1.3.4: ELF 64-bit LSB pie executable x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=f21d565252953c3557f0a02e125bfc483198b5a9, not stripped And now that is *not a pie executable* but a shared lib..
On 04/22/2018 08:44 PM, Gabriel C wrote:
No, I think that's wrong. You're also removing the relocation information from the PIE binaries.
So explain why that is wrong and how is different to what is done right now .. If you say that is wrong then you say is wrong in general.
Well, uh, the whole point is that he's saying we should preserve the behavior with the new version of file?
Also how this removes realocation informations ? That would be then broken for <file 5.33 ,right ?
Did you run at least plain 'file' to see what file reports now no matter you have a shared lib , execuable or a static lib ?
Okay??? On older versions of file, these were both treated as boring old application/x-sharedlib, and now on new versions of file it will be treated as exciting new application/x-pie-executable, which... has the exact same behavior as application/x-sharedlib. Because heftig is trying to preserve the status quo, whereas you're suggesting we break with our current behavior and start treating pie executables like any other kind. All this is literally in the commit message for this patch. So let's not waste time further discussing it.... If you object to the conceptual idea of using the same strip options on pie executables *and* shared libs, then by all means tell us why you object, but leave the file program out of it. Also do tell us why we've been doing the wrong thing for quite some time now. ;) -- Eli Schwartz Bug Wrangler and Trusted User
On 23/04/18 01:44, Gabriel C wrote:
2018-04-22 21:53 GMT+02:00 Jan Alexander Steffens <jan.steffens@gmail.com>:
On Sun, Apr 22, 2018, 18:22 Gabriel C <nix.or.die@gmail.com> wrote:
I don't think that's enough .. since everything , .so , .a ( possible glibc2.7+gcc8 ) and any binary is now marked application/x-pie-executable in PIE builds.
So probably something like this is what you wish :
https://github.com/abucodonosor/pacman/commit/f38df66ad39383dbce92cef53d3da7...
No, I think that's wrong. You're also removing the relocation information from the PIE binaries.
So explain why that is wrong and how is different to what is done right now .. If you say that is wrong then you say is wrong in general.
There are two issues here: 1) file sucks. It is not distinguishing between PIE binaries and libraries. Has that been reported upstream or are we just working around a bug in makepkg? If this is fixed upstream, the fix I will accept into pacman will be quite different... 2) Once we can detect binaries/libraries correctly again, the default STRIP_BINARIES in Arch Linux is not suitable for the "PIE by default" setup. makepkg.conf needs changed rather than just hiding that... (FYI, this currently screwing over the Arch glibc PKGBUILD which uses $STRIP_BINARIES..). A
On 26/04/18 00:20, Allan McRae wrote:
There are two issues here:
1) file sucks. It is not distinguishing between PIE binaries and libraries.
Has that been reported upstream or are we just working around a bug in makepkg? If this is fixed upstream, the fix I will accept into pacman will be quite different...
And replying to my own comment... In the original patch commit: "file 5.33 also misidentifies actual libraries as PIE executables" Is there any actually difference between a library and a binary when they are built with PIE? Aren't the elf headers exactly the same? Note, we have been detecting PIE binaries as application/x-sharedlib with previous file versions. So either: 1) original patch is fine, as we need to update the description of STRIP_SHARED to include all PIE things, or 2) we need a STRIP_PIE variable. Can someone confirm what we get with static PIE with gcc8+? This will make my decision. Allan
On 21/04/18 03:46, Jan Alexander Steffens (heftig) wrote:
file 5.33 introduces a new MIME type "application/x-pie-executable", which is used for relocatable binaries. makepkg ignored these binaries and did not attempt to strip them.
Handle the new MIME type like the old "application/x-sharedlib". Stripping the binaries with --strip-unneeded to keep relocation information should be the correct thing to do.
file 5.33 also misidentifies actual libraries as PIE executables, so we didn't strip any shared libraries, either. We now work around this bug.
Signed-off-by: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com> ---
Applied with the following documentation additions: diff --git a/doc/makepkg.conf.5.txt b/doc/makepkg.conf.5.txt index 267dc9e9..5417aa0e 100644 --- a/doc/makepkg.conf.5.txt +++ b/doc/makepkg.conf.5.txt @@ -193,8 +193,8 @@ Options for details. **STRIP_SHARED=**"--strip-unneeded":: - Options to be used when stripping shared libraries. See linkman:strip[1] - for details. + Options to be used when stripping shared libraries or PIE executables. + See linkman:strip[1] for details. **STRIP_STATIC=**"--strip-debug":: Options to be used when stripping static libraries. See linkman:strip[1]
scripts/libmakepkg/tidy/strip.sh.in | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/scripts/libmakepkg/tidy/strip.sh.in b/scripts/libmakepkg/tidy/strip.sh.in index e20114c0..36d1b89e 100644 --- a/scripts/libmakepkg/tidy/strip.sh.in +++ b/scripts/libmakepkg/tidy/strip.sh.in @@ -125,6 +125,8 @@ tidy_strip() { esac;; *application/x-executable*) # Binaries strip_flags="$STRIP_BINARIES";; + *application/x-pie-executable*) # Relocatable binaries + strip_flags="$STRIP_SHARED";; *) continue ;; esac
participants (5)
-
Allan McRae
-
Eli Schwartz
-
Gabriel C
-
Jan Alexander Steffens
-
Jan Alexander Steffens (heftig)