Disable PIE hardening in more places #50295
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Success on aarch64-linux (full log) Attempted: libxml2 Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: libxml2 Partial log (click to expand)
|
|
Success on x86_64-darwin (full log) Attempted: libxml2 Partial log (click to expand)
|
Some packages don’t work correctly with pie. Here I disable it for: - busybox - linux kernel - kexectools I also get rid of the Musl conditional for disabling pie in GCC and Binutils. Some day we might want to enable PIE without Musl and it will be useful to have the *just* work with our compiler and linkers.
matthewbauer
requested review from
edolstra,
Ericson2314,
FRidh,
LnL7,
Mic92,
nbp and
zimbatm
as code owners
GrahamcOfBorg
added
6.topic: erlang
6.topic: fetch
6.topic: golang
6.topic: nixos
6.topic: python
6.topic: ruby
6.topic: rust
6.topic: stdenv
|
what is the motivation behind that change @matthewbauer ? Isn't PIE a good thing to have? |
|
It doesn't work in many places. You will frequently get something along the lines of More info: https://wiki.ubuntu.com/SecurityTeam/PIE |
|
I had the same question, it's enabled for a reason so this seems kind of undesirable to me. |
|
On Tue, 13 Nov 2018 09:53:57 -0800, Daiderd Jordan ***@***.***> wrote:
I had the same question, it's enabled for a reason so this seems kind of undesirable to me.
There appears to be confusion, perhaps I can clear it up.
We (forcably) disable PIE currently, as I understand it.
I'm not entirely sure why this is the case,
but has been the behavior for a while now and certainly before
recent changes such as those in this PR.
The explanation I'm aware of is it broke "some" things and as a result
was disabled everywhere -- I believe during last big rework in hardening
flags and quite possibly before that as well. This PR seems to be
taking steps towards making it possible to enable PIE by default.
Recently lack of PIE was noticed to cause breakage when with musl
when **not** disabled and gold linker was used, so it was decided
to enabled PIE w/musl by default since it both seems reasonable from
a security perspective and would resolve the reported issue.
The musl-only guards for PIE were to avoid rebuilds,
but AFAIK everyone who's commented on these PR's is in agreement
that PIE seems to make sense as a general default.
This PR seems to mostly remove the misleading musl-only conditional
for disabling PIE on packages that break with it force-enabled--
I don't know but it's possible they manage PIE flags themselves.
As of last week-- I'm still catching up from travel-- it is the case
that "disabling" pie on anything other than musl has no actual effect
on the build, as only with musl (thanks to recent efforts :<3:) is the
flag even considered: with glibc we force-remove the flag regardless.
…
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#50295 (comment) part: text/html
|
|
Thanks for the update @dtzWill, makes sense! Without the context it looked like protections where casually removed but it's not the case. |
|
Yes, just to clear this up, we have never had PIE being enabled by default. It was always opt-in. This was mostly done because this caused much more breakage than the other hardening flags, and even without this it was hard enough to get it in. :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
/cc @dtzWill