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

Cross bootstrap tools have broken stackprotector support #113977

Closed
r-burns opened this issue Feb 22, 2021 · 11 comments · May be fixed by #141448
Closed

Cross bootstrap tools have broken stackprotector support #113977

r-burns opened this issue Feb 22, 2021 · 11 comments · May be fixed by #141448
Labels
0.kind: bug 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 6.topic: stdenv Standard environment

Comments

@r-burns
Copy link
Contributor

r-burns commented Feb 22, 2021

Describe the bug
The cross-compiled bootstrap tools produced by make-bootstrap-tools-cross.nix have faulty support for stack protector hardening (libssp) during the resulting stdenv bootstrap chain. The issue manifests as linker errors when building the stage4 gcc from the static stage3 libraries during stdenv bootstrap. This means that when initially bootstrapping a system from cross-compiled bootstrap tools, an ugly workaround is needed:

--- a/pkgs/stdenv/linux/default.nix
+++ b/pkgs/stdenv/linux/default.nix
@@ -259,7 +259,14 @@ in
   (prevStage: stageFun prevStage {
     name = "bootstrap-stage3";
 
-    overrides = self: super: rec {
+    overrides = self: super: let
+      wrapStage3 = pkg: (pkg.override {
+        stdenv = self.makeStaticLibraries self.stdenv;
+      }).overrideAttrs (oa: {
+        # Work around faulty stackprotector support in cross bootstrap tools
+        hardeningDisable = (oa.hardeningDisable or []) ++ [ "stackprotector" ];
+      });
+    in rec {
       inherit (prevStage)
         ccWrapperStdenv
         binutils coreutils gnugrep
@@ -268,10 +275,10 @@ in
       # Link GCC statically against GMP etc.  This makes sense because
       # these builds of the libraries are only used by GCC, so it
       # reduces the size of the stdenv closure.
-      gmp = super.gmp.override { stdenv = self.makeStaticLibraries self.stdenv; };
-      mpfr = super.mpfr.override { stdenv = self.makeStaticLibraries self.stdenv; };
-      libmpc = super.libmpc.override { stdenv = self.makeStaticLibraries self.stdenv; };
-      isl_0_20 = super.isl_0_20.override { stdenv = self.makeStaticLibraries self.stdenv; };
+      gmp = wrapStage3 super.gmp;
+      mpfr = wrapStage3 super.mpfr;
+      libmpc = wrapStage3 super.libmpc;
+      isl_0_20 = wrapStage3 super.isl_0_20;
       gcc-unwrapped = super.gcc-unwrapped.override {
         isl = isl_0_20;
       };

To Reproduce
(The following steps are slightly modified from my original use case, to ease repro on commodity x86_64 hardware. You don't need any --option system fudging if you have a foreign arch system you can cross-compile to.)
Steps to reproduce the behavior:

  1. Checkout any recent commit. I'm on f288a8c.
  2. Patch make-bootstrap-tools-cross.nix for compilation to x86_64.
--- a/pkgs/stdenv/linux/make-bootstrap-tools-cross.nix
+++ b/pkgs/stdenv/linux/make-bootstrap-tools-cross.nix
@@ -13,6 +13,7 @@ in lib.mapAttrs (n: make) (with lib.systems.examples; {
   armv6l     = raspberryPi;
   armv7l     = armv7l-hf-multiplatform;
   aarch64    = aarch64-multiplatform;
+  x86_64 = gnu64;
   x86_64-musl  = musl64;
   armv6l-musl  = muslpi;
   aarch64-musl = aarch64-multiplatform-musl;
  1. Build cross-compiled bootstrap tools. (We'll force the build system to be an i686 here, to take the cross-compilation codepaths on ordinary x86 hardware.)
    nix build -f pkgs/stdenv/linux/make-bootstrap-tools-cross.nix --option system i686-linux x86_64.build
  2. Copy result/on-server/bootstrap-tools.tar.xz to pkgs/stdenv/linux/bootstrap-files/ and set it as the x86_64 bootstrap.
--- a/pkgs/stdenv/linux/bootstrap-files/x86_64.nix
+++ b/pkgs/stdenv/linux/bootstrap-files/x86_64.nix
@@ -2,8 +2,11 @@
 (import ./i686.nix) //
 
 {
+  /*
   bootstrapTools = import <nix/fetchurl.nix> {
     url = "http://tarballs.nixos.org/stdenv-linux/x86_64/c5aabb0d603e2c1ea05f5a93b3be82437f5ebf31/bootstrap-tools.tar.xz";
     sha256 = "a5ce9c155ed09397614646c9717fc7cd94b1023d7b76b618d409e4fefd6e9d39";
   };
+  */
+  bootstrapTools = ./bootstrap-tools.tar.xz;
 }
  1. Build stdenv: nix build -f . stdenv
  2. Observe undefined reference to ``__stack_chk_guard' linking errors near the end of the stage 4 gcc build. The stage3 static library dependencies of gcc (gmp, mpfr, mpc, and isl) must be overridden to disable stackprotector hardening for gcc to build.

Log snippet:

/build/build/./prev-gcc/xg++ -B/build/build/./prev-gcc/ -B/nix/store/0v9kxf2w2rqvd5410vnc5lrqciiryaxd-gcc-10.2.0/x86_64-unknown-linux-gnu/bin/ -nostdinc+>
/build/build/./prev-gcc/xg++ -B/build/build/./prev-gcc/ -B/nix/store/0v9kxf2w2rqvd5410vnc5lrqciiryaxd-gcc-10.2.0/x86_64-unknown-linux-gnu/bin/ -nostdinc+>
/build/build/./prev-gcc/xg++ -B/build/build/./prev-gcc/ -B/nix/store/0v9kxf2w2rqvd5410vnc5lrqciiryaxd-gcc-10.2.0/x86_64-unknown-linux-gnu/bin/ -nostdinc+>
  cc1-checksum.o libbackend.a main.o libcommon-target.a libcommon.a ../libcpp/libcpp.a ../libdecnumber/libdecnumber.a libcommon.a ../libcpp/libcpp.a   ..>
/build/build/./prev-gcc/xg++ -B/build/build/./prev-gcc/ -B/nix/store/0v9kxf2w2rqvd5410vnc5lrqciiryaxd-gcc-10.2.0/x86_64-unknown-linux-gnu/bin/ -nostdinc+>
      cp/cp-lang.o c-family/stub-objc.o cp/call.o cp/class.o cp/constexpr.o cp/constraint.o cp/coroutines.o cp/cp-gimplify.o cp/cp-objcp-common.o cp/cp-u>
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: /nix/store/4sssnshkvw4i722j0qrkdhyjcmx71l62-isl-0.20/lib/libisl.a(isl_map.o): in func>
(.text+0x82b): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0x8b0): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0x927): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0xa29): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0xaa3): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: /nix/store/4sssnshkvw4i722j0qrkdhyjcmx71l62-isl-0.20/lib/libisl.a(isl_map.o):(.text+0>
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: /nix/store/4sssnshkvw4i722j0qrkdhyjcmx71l62-isl-0.20/lib/libisl.a(isl_map.o): in func>
(.text+0x82b): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0x8b0): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0x927): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0xa29): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0xaa3): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: /nix/store/4sssnshkvw4i722j0qrkdhyjcmx71l62-isl-0.20/lib/libisl.a(isl_map.o):(.text+0>
collect2: error: ld returned 1 exit status
make[3]: *** [../../gcc-10.2.0/gcc/lto/Make-lang.in:88: lto1] Error 1
make[3]: *** Waiting for unfinished jobs....
collect2: error: ld returned 1 exit status
make[3]: *** [../../gcc-10.2.0/gcc/lto/Make-lang.in:92: lto-dump] Error 1
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: /nix/store/4sssnshkvw4i722j0qrkdhyjcmx71l62-isl-0.20/lib/libisl.a(isl_map.o): in func>
(.text+0x82b): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0x8b0): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0x927): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0xa29): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0xaa3): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: /nix/store/4sssnshkvw4i722j0qrkdhyjcmx71l62-isl-0.20/lib/libisl.a(isl_map.o):(.text+0>
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: /nix/store/4sssnshkvw4i722j0qrkdhyjcmx71l62-isl-0.20/lib/libisl.a(isl_map.o): in func>
(.text+0x82b): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0x8b0): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0x927): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0xa29): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: (.text+0xaa3): undefined reference to `__stack_chk_guard'
/nix/store/467843qgpic7qqhsq1q1ciikr775h9lz-binutils-2.35.1/bin/ld: /nix/store/4sssnshkvw4i722j0qrkdhyjcmx71l62-isl-0.20/lib/libisl.a(isl_map.o):(.text+0>
collect2: error: ld returned 1 exit status
make[3]: *** [../../gcc-10.2.0/gcc/c/Make-lang.in:85: cc1] Error 1
collect2: error: ld returned 1 exit status
make[3]: *** [../../gcc-10.2.0/gcc/cp/Make-lang.in:120: cc1plus] Error 1
rm gcc.pod
make[3]: Leaving directory '/build/build/gcc'
make[2]: *** [Makefile:4906: all-stageprofile-gcc] Error 2

Expected behavior
Successful build of stdenv using cross-compiled bootstrap files

Additional context
I first encountered this months ago when using cross-built bootstrap tools for powerpc64le, and assumed it was an arch-specific issue. However, when later using native-built bootstrap tools I realized the stackprotector hardening could be reenabled in the stage3 libraries. It wasn't until I tested i686 -> x86_64 bootstrap that I realized this was a more general issue.

Notify maintainers
Cross infra CODEOWNERS: @Ericson2314 @matthewbauer

@r-burns r-burns added 0.kind: bug 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 6.topic: stdenv Standard environment labels Feb 22, 2021
@DieracDelta
Copy link
Member

DieracDelta commented Apr 26, 2021

Maybe this isn't the morally correct fix, but I'm also hitting this problem... Thoughts on merging the provided fix into Nixpkgs?

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/getting-started-with-nix-on-ppc64le/12712/2

@r-burns
Copy link
Contributor Author

r-burns commented Apr 26, 2021

Hmm, I think I would want input from someone more versed in stdenv. AFAICT the stage3 static libs are only used for the stage4 gcc, but that gcc is the one used in stdenv. So these libraries would never be exposed directly in nixpkgs, but would be used transitively by gcc when every package is built. So I'd need to know what issue the stackprotector hardening addresses, and if that's potentially an issue for us to disable it for dependencies of stdenv's gcc. It looks like in most cases, when stackprotector causes issues, we just turn it off. But that only seems to be the case for leaf packages; I would imagine stdenv requires much more scrutiny.

@DieracDelta
Copy link
Member

DieracDelta commented Apr 26, 2021

For some reason, this is actually making it into nixpkgs for me. E.g. the cross compiler I grab here gives a gcc cross compiler from x8664 to riscv64. When I try to compile a kernel with that cross compiler toolchain (riscv64-unknown-linux-gnu-*), I get stack protector errors at link time (exactly as you've described). I'm not sure which stage gcc this is; perhaps this is completely different. I just pattern matched on the error you've described.

@piperswe
Copy link
Contributor

piperswe commented Oct 13, 2021

So I'd need to know what issue the stackprotector hardening addresses, and if that's potentially an issue for us to disable it for dependencies of stdenv's gcc.

The stack protector guards against buffer overflow in the built artifact. Hardening of a compiler's stack protector should have no impact on that compiler's output, and since this compiler is only used to compile stage4 I'd say this change is fine.

@r-burns
Copy link
Contributor Author

r-burns commented Oct 13, 2021

since this compiler is only used to compile stage4 I'd say this change is fine.

This compiler /is/ stage4, and is exposed as stdenv.cc.

Realistically, I think we have bigger fish to fry w.r.t. the security of nixpkgs. But I'm not a security expert so I'd want some more reassurance here.

@piperswe
Copy link
Contributor

This compiler /is/ stage4, and is exposed as stdenv.cc.

Oh, it might be worth it to rebuild the compiler in stage4 then.

@r-burns
Copy link
Contributor Author

r-burns commented Oct 22, 2021

Hmm, I just found #40797 in the issue backlog and am wondering if it's related.

@stale
Copy link

stale bot commented Apr 25, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 25, 2022
@trofi
Copy link
Contributor

trofi commented Jul 18, 2022

Proposed the possible fix as #181943

@r-burns
Copy link
Contributor Author

r-burns commented Oct 16, 2022

Confirmed this has been resolved by trofi's sysroot fix. Thanks!

@r-burns r-burns closed this as completed Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 6.topic: stdenv Standard environment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants