gcc: build with --enable-default-pie configure option#439314
gcc: build with --enable-default-pie configure option#439314emilazy merged 1 commit intoNixOS:stagingfrom
Conversation
2c0cbaa to
7640607
Compare
emilazy
left a comment
There was a problem hiding this comment.
I agree that this is the correct path forward for handling this migration, and am looking forward to Nixpkgs not being embarrassingly behind on this default.
If workable it might be nice to drop the hardening flag entirely, and handle treewide removal in a followup. Packages that cannot build with pie can explicitly pass
-no-piewhen needed. Most packages already do due to the prevalence of compilers built with--enable-default-pieacross other distros, however packages that have not been maintained in the past decade may need it passed viaNIX_CFLAGS_COMPILE. I have ran into zero packages that needed this treatment in a full rebuild of my desktop on this PR, but I'd be surprised if there are none.
We have a few instances of hardeningDisable = [ "pie" ]; in the tree. Have you tested those? If they’re no longer necessary, it would be good to remove them beforehand. If they are necessary, then we either need to migrate them to another scheme or make the hardeningDisable set the appropriate flag to disable it.
I personally support doing things outside of wrappers wherever possible, with a view towards reducing or eliminating their role entirely over time, so I would not mind the hardeningDisable flag going away. However, I’ll defer to @risicle on this; there is a reasonable argument for keeping the existing, consistent interface, I think, if it’s not too troublesome to adapt it to pass an explicit flag to disable PIE.
|
It's an interesting question we've not really encountered before. I guess the whole "hardening flags" mechanism was built around just that - hardening "flags" that had to be added to the invocation. It's not really a "flag" anymore if it's built-in. It would actually take a bit of refactoring of I don't think I have a strong opinion, my only concern being how this might affect more obscure architectures/platforms/stdenvs. |
The packages I could identify that set Skipped |
|
It might be easiest to land this change without changing anything about the existing "pie" flag, since our darwin and pkgsLLVM stdenvs already opportunistically turn on pie and this PR merely aligns GCC toolchains. Does that make sense? It'd leave followup work of deprecating the flag (probably accepting it and warning that it's a no-op for 1 release cycle) and treewide removal of its usage in hardeningDisable calls. |
@risicle do you have an idea of a good set of weird stdenvs to test? |
|
So is the thinking that the existing instances of |
|
FWIW, this seems to bootstrap and compile things fine on macOS. |
Sometimes the package already passes valgrind is an example of a package that will fail at runtime if pie is enabled. It has The worst case scenario for this approach is a package that passes flags that are compatible with PIE, doesn't pass |
|
No example of that last type because I can't find one, I've been running my desktop and build boxes on this for a week now and have yet to run into runtime issues. |
|
Seems like the GHC issue is relating to conflicts with it passing Not sure if you want to keep this drafted for now, but I would be comfortable moving forward with this. I’m okay splitting out the deprecation of the hardening flag, but I think we should do it soon after we merge this, since a non‐functioning hardening flag is pretty confusing, even if it’s harmless in this case. I’ve asked @alyssais to check if there are any Musl‐relevant considerations here, since there are a few packages that set |
Does it make sense to keep the hardening flag as a way to turn a package that would build but silently disable PIE into a build failure? That might be a remaining use for it but I don't know if it's a good use. 😅 |
|
Probably not. That seems more like the job of a |
There was a problem hiding this comment.
Flagging this on for static targets doesn't get automatic static-pie. The stdenv still works. Unsure if that means we should default on without hasSharedLibraries.
static-pie would still need a hardening flag and wrapper support. The "pie" hardening flag doesn't try to support static-pie currently.
There was a problem hiding this comment.
We apparently already default the hardening flag on for Musl (except on ARM, for some reason…?), so I guess our static builds are already using PIE, and that’s also probably why some things disable it conditional on Musl.
I think Alpine has a patch for default static PIE. It might just be a matter of adjusting spec files.
There was a problem hiding this comment.
I think that applies for pkgsMusl but not pkgsStatic. Testing with checksec on nixos-unstable:
$ nix run nixpkgs#checksec file (nix build github:nixos/nixpkgs/nixos-unstable#pkgsMusl.hello --print-out-paths -
-no-link)/bin/hello
RELRO Stack Canary NX PIE RPATH RUNPATH Symbols FORTIFY Fortified Fortifiable Name
Full RELRO Canary Found NX enabled PIE Enabled No RPATH RUNPATH 213 symbols No 0 0 /nix/store/zdfx8s4vyv0hghfi5bb3ib2rpa74xpr3-hello-2.12.2/bin/hello
$ nix run nixpkgs#checksec file (nix build github:nixos/nixpkgs/nixos-unstable#pkgsStatic.hello --print-out-paths
--no-link)/bin/hello
RELRO Stack Canary NX PIE RPATH RUNPATH Symbols FORTIFY Fortified Fortifiable Name
Partial RELRO Canary Found NX enabled PIE Disabled No RPATH No RUNPATH 462 symbols N/A 0 0 /nix/store/lwnanc42zfbpi2airnmg8h03a1v18whj-hello-static-x86_64-unknown-linux-musl-2.12.2/bin/helloThere was a problem hiding this comment.
So then should we set this to unconditionally default on?
|
stop-gap suggestion from chat: --- a/pkgs/stdenv/linux/make-bootstrap-tools.nix
+++ b/pkgs/stdenv/linux/make-bootstrap-tools.nix
@@ -50,6 +50,7 @@
bootGCC = pkgs.gcc.cc.override {
enableLTO = false;
isl = null;
+ enableDefaultPie = false;
};
bootBinutils = pkgs.binutils.bintools.override { |
partially fixes NixOS#439314 (comment) Co-authored-by: dramforever <[email protected]>
|
Bisect says this change broke |
|
Test is assuming non-ASLRed instruction pointers, https://github.com/gentoo/gentoo/blob/master/dev-debug/ltrace/files/ltrace-0.7.3-print-test-pie.patch fixes it. I can put up a PR later. |
|
This breaks cross-compilation with some exotic platforms, such as mmixware. Bisect log |
|
Probably we need to swap Do you have any examples of exotic targets that need adjusted other than mmixware? |
|
Continuing discussion in issue |
Fixes NixOS#439314 Co-authored-by: Martin Joerg <[email protected]>
Fixes NixOS#439314 Co-authored-by: Martin Joerg <[email protected]>
In discussion on NixOS#439314 conditionalizing this seemed unnecessary as pkgsStatic's gcc kept working. However, some less typical !hasSharedLibraries stdenvs for embedded platforms broke. It seems simplest to make this conditional on hasSharedLibraries for now. We may need to revisit this if we want to enable static-pie by default with gcc spec file changes.
|
Broke |
|
This broke cross-compiled gcc for x86_64-cygwin, which unfortunately is not reproducible on master. host=linux/target=cygwin works, but host=cygwin/target=cygwin is broken. PIC is disabled when building libiberty, but enabled when building c++tools. This makes the latter fail to link: This may be an upstream bug, but I thought I'd make a note of it. |
Let's configure GCC with
--enable-default-pie.We already build clang with the equivalent
CLANG_DEFAULT_PIE_ON_LINUXset toON(upstream default).Configuring GCC directly with
--enable-default-pieis simple and matches what most mainstream distributions have been doing for years. It results in a compiler that opportunistically enables pie whenever it does not conflict with other flags.Currently our clang stdenvs (pkgsLLVM, darwin default stdenvs) opportunistically apply pie, and our gcc stdenvs (linux default stdenv) do not. This is a bit weird since in some stdenvs adding the "pie" hardening flag is required to get ASLR supporting executables, and in others it's a mix of a no-op and a footgun that can cause builds to fail.
It's easier for
gccandclangto do the right thing by default based on whatever combination of flags have been passed than for our wrapper, so this PR should avoid issues with differing static/shared flags. gcc is extremely sensitive to the ordering of pie flags and static build flags. Prior attempts like #252310 got stuck, mostly due to this complexity.The
enable-default-pieapproach doesn't break some unusual stdenvs that we had trouble with in previous attempts.pkgsExtraHardening.pkgsStatic.hellocan be built on bothaarch64-linuxandx86_64-linux.Comparison of checksec PIE Disabled processes running on my desktop before and after rebuilding on this PR branch:
This impacts the behavior of the existing "pie" hardening flag, so we need to decide what to do about that. The current state on this branch enables pie by default at the compiler level while leaving the hardening flag as a no-op, which is fine for some testing but not ready to merge.
If workable it might be nice to drop the hardening flag entirely, and handle treewide removal in a followup.
Packages that cannot build with pie can explicitly pass
-no-piewhen needed. Most packages already do due to the prevalence of compilers built with--enable-default-pieacross other distros, however packages that have not been maintained in the past decade may need it passed viaNIX_CFLAGS_COMPILE.I have ran into zero packages that needed this treatment in a full rebuild of my desktop on this PR, but I'd be surprised if there are none.
An alternative to dropping the "pie" flag is to teach the wrappers to pass
-no-piewhen the flag is disabled for stdenvs using compilers that have pie by default, along with adding "pie" to the default hardening flag list for those stdenvs.Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Annoyances encountered: tailscale build failing due to kernel version #438765, dbus-broker failure due to kernel version #439331
Add a 👍 reaction to pull requests you find important.