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

make-derivation: enable pie hardening with musl #49704

Merged
merged 1 commit into from
Nov 7, 2018

Conversation

matthewbauer
Copy link
Member

Fixes #49071

@GrahamcOfBorg GrahamcOfBorg added the 6.topic: stdenv Standard environment label Nov 3, 2018
@FRidh
Copy link
Member

FRidh commented Nov 3, 2018

Please describe in the commit what it fixes instead of just referring to an issue.

Fixes NixOS#49071

On ld.gold, we produce broken executables when linking with the Musl
libc. This appears to be a known bug when using ld.gold and Musl. This
thread describes the workaround as enabling PIE when using ld.gold and
Musl:

https://www.openwall.com/lists/musl/2015/05/01/5

By default we don’t enable PIE to avoid breaking things. But in the
Musl case we are breaking things by not enabling PIE. So this adds a
special case for defaultHardeningFlags which keeps the pie hardening
for everything. Any packages that break with PIE can add the pie flag
to disableHardeningFlags array (a no-op for now on anything but Musl).
@matthewbauer
Copy link
Member Author

Thanks- updated.

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Nov 3, 2018
@dtzWill
Copy link
Member

dtzWill commented Nov 3, 2018

Would it make sense to enable PIE by default? Or rather, what's the reason we don't do this always?

Is performance the concern or breakage (or ...)?

Performance seems like a bad reason, don't other distributions enable by default? Such a trade-off seems like it should be opt-in, but regardless first it'd be good to sort out what the reason is before disagreeing with it :P.

@matthewbauer matthewbauer requested a review from cstrahan November 4, 2018 18:45
@matthewbauer
Copy link
Member Author

I am thinking of just doing this for now as it avoid a mass rebuild. If someone wants to try enabling PIE globally, that would be great. It will just probably be on you to debug any of the new failures.

@dtzWill
Copy link
Member

dtzWill commented Nov 6, 2018 via email

@globin
Copy link
Member

globin commented Nov 7, 2018

IIRC we tried that at first but it caused too much breakage so we didn't go that far.

@globin globin merged commit 6d531f3 into NixOS:master Nov 7, 2018
@dtzWill
Copy link
Member

dtzWill commented Nov 7, 2018

I'm seeing this break musl immediately?

https://gist.github.com/dtzWill/d93dc2211fd2d134a81cd858311f42ce

nix build nixpkgs.pkgsMusl.bash, dies building bootstrap compiler

What sort of builds does this not break (and presumably improves behavior for)?

Checking behavior across branches presently...

@GrahamcOfBorg build pkgsMusl.bash

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: pkgsMusl.bash

Partial log (click to expand)

Cannot nix-instantiate `pkgsMusl.bash' because:
error: Musl libc only supports Linux systems.

@GrahamcOfBorg
Copy link

Unexpected error: command failed with exit code 1 on aarch64-linux (full log)

Attempted: pkgsMusl.bash

Partial log (click to expand)

cannot build derivation '/nix/store/n51xzgr07sny7hxal9fzf8rwfbw7kkb5-mpfr-4.0.1.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/h66rmfzfswfwgp0y4x0j83lxib1gqvd6-texinfo-6.5.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/5k4bbhgpl7ig43ywhylqi60qxsgigbms-libelf-0.8.13.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/c8jd9mwjpsv6vz01nkj6va5ylmk3xlq4-libmpc-1.1.0.drv': 4 dependencies couldn't be built
cannot build derivation '/nix/store/5rq674isjrhsp3hg0d0skr5ab2y4a9ar-gcc-7.3.0.drv': 15 dependencies couldn't be built
cannot build derivation '/nix/store/i7wfcp5ij0d2p05hkbbamd97b2h4hzg2-bootstrap-stage4-gcc-wrapper-7.3.0.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/k3hlyx9wgx5ayvp9nlmnvlhrnzrx6mrv-bootstrap-stage4-stdenv-linux.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/ida3gqjqgrjrd30453chsbvxxx6idc42-autoconf-2.69.drv': 4 dependencies couldn't be built
cannot build derivation '/nix/store/q8vnwykkhb3qczd6n9jqvy6cgwvb0bsd-bash-4.4-p23.drv': 5 dependencies couldn't be built
error: build of '/nix/store/q8vnwykkhb3qczd6n9jqvy6cgwvb0bsd-bash-4.4-p23.drv' failed

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: pkgsMusl.bash

Partial log (click to expand)

make[2]: Leaving directory '/build/build'
make[1]: *** [Makefile:21598: stage1-bubble] Error 2
make[1]: Leaving directory '/build/build'
make: *** [Makefile:22372: profiledbootstrap] Error 2
builder for '/nix/store/1757dkhp6j6j8mdp2z5qw9gda9nyzcxj-gcc-7.3.0.drv' failed with exit code 2
cannot build derivation '/nix/store/av8hqpi3za0svxcf2m7jaqc4nv0m0055-bootstrap-stage4-gcc-wrapper-7.3.0.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/6fwfwlygcz0nb8bfdk1z849db5wac1lb-bootstrap-stage4-stdenv-linux.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/2v57cyha2a6d94cis2np56r5ala5dvw6-autoconf-2.69.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/mbs1zl34blnjkfbzx79yhdh0kzc97s2i-bash-4.4-p23.drv': 3 dependencies couldn't be built
error: build of '/nix/store/mbs1zl34blnjkfbzx79yhdh0kzc97s2i-bash-4.4-p23.drv' failed

@dtzWill
Copy link
Member

dtzWill commented Nov 8, 2018

So this seems to only break things, is there a case for not reverting it? Otherwise please do so... (or I'll do so in a day or two)

@matthewbauer
Copy link
Member Author

That should be fixed in 2e2afa1. Hoping binutils/gcc are the only ones.

@dtzWill
Copy link
Member

dtzWill commented Nov 15, 2018

Additional failures:

  • glibc
  • ghc

I'm a fan of this but this looks like it was merged too eagerly and wasn't... tested at all? Including checking if the original issue was fixed since build fails long before that point AFAICT.
Unfortunately I think our testing bandwidth is thin currently, so I'm not sure how to proceed. Maybe whack-a-mole the next few failures and cross our fingers? I'd like to avoid a revert :).

FWIW here's a few packages that no one's obligated to support BUT I build regularly and so they work reasonably well as oracles, even if they're not intended to be such:

  • pkgsMusl.cachix
  • pkgsMusl.nix (pkgsMusl.nixUnstable)
  • pkgsMusl.llvmPackages_{5,6,7}.{llvm,clang}
    Just in case that helps -- maybe we can put together a musl jobset at some point :). (and contribute builders :))

@dtzWill
Copy link
Member

dtzWill commented Nov 15, 2018

glibc is just an addition to hardeningDisable, ghc is a bit trickier. Currently testing this: dtzWill@0caf0a3 (will report back in an hour or however long ghc takes :P).

@dtzWill
Copy link
Member

dtzWill commented Nov 15, 2018

Nope, looks like problem is with bootstrap ghc not being built -fPIC -- so either new bootstrap tarball (eep) or maybe just disable PIE for ghc? Shame b/c it might be nice to use gold linker with it...

@matthewbauer
Copy link
Member Author

Yeah i was probably too quick with this! Sorry about that. I didn’t think pie could break so much. Feel free to revert it (but leave in the hardeningDisable flags for future reference).

@matthewbauer
Copy link
Member Author

I would say just disable PIE for ghc. There's some weird stuff ghc bootstrapping does that makes this stuff complex. We have to do some weird patches to get this working on Android:

https://github.com/reflex-frp/reflex-platform/blob/develop/nixpkgs-overlays/mobile-ghc/8.2.y/android-patches/force-relocation-equal-pic.patch

We do have a cross-trunk jobset with some musl jobs:

https://hydra.nixos.org/jobset/nixpkgs/cross-trunk#tabs-jobs

Maybe we should add cachix to it?

@nh2
Copy link
Contributor

nh2 commented Jul 4, 2021

Related problem with writeup: #129247 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: stdenv Standard environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ld.gold crashes or produces invalid executables when Musl is used as a libc
6 participants