On 24/10/16 13:56, Allan McRae wrote:

That means we will add all of these to our default CFLAGS/LDFLAGS etc. The changes are:

1) building gcc to enable PIE by default 2) add -z,now to LDFLAGS 3) and -fno-plt and -fstack-check to our CFLAGS

For those wondering what happened with this: binutils build with gcc with default PIE: # grep Error binutils-git-89080.bfbf34d-1-x86_64-check.log make[5]: *** [Makefile:7161: incremental_test_2] Error 1 make[5]: *** [Makefile:7185: incremental_test_5] Error 1 make[5]: *** [Makefile:7200: incremental_copy_test] Error 1 make[5]: *** [Makefile:7206: incremental_common_test_1] Error 1 make[5]: *** [Makefile:7132: ehdr_start_test_4] Error 1 readelf: Error: the PHDR segment is not covered by a LOAD segment make[4]: *** [Makefile:5609: check-am] Error 2 make[3]: *** [Makefile:5613: check] Error 2 make[2]: *** [Makefile:941: check-recursive] Error 1 make[1]: *** [Makefile:6134: check-gold] Error 2 make[5]: *** [Makefile:3678: check-DEJAGNU] Error 1 make[4]: *** [Makefile:1953: check-am] Error 2 make[3]: *** [Makefile:1793: check-recursive] Error 1 make[2]: *** [Makefile:1955: check] Error 2 make[1]: *** [Makefile:7547: check-ld] Error 2 make: *** [Makefile:2206: do-check] Error 2 binutils build with current gcc: # grep Error binutils-git-89080.bfbf34d-1-x86_64-check.log readelf: Error: the PHDR segment is not covered by a LOAD segment I need time to fix this. It is probably just test suite assumptions rather than errors. A

On Mon, Oct 24, 2016 at 5:56 AM Allan McRae wrote:

Hi all,

The results from the test-sec-flags [1] suite are in. Many thanks to those that wrote this and those that submitted results. I'm not going to list the summaries here, but the results show that at worst enabling a bunch of security flags in our packages will have a <1% impact on performance (and more likely a fraction of a percent).

That means we will add all of these to our default CFLAGS/LDFLAGS etc. The changes are:

1) building gcc to enable PIE by default 2) add -z,now to LDFLAGS 3) and -fno-plt and -fstack-check to our CFLAGS

Adding PIE means that programs get loaded at a random address, preventing an attacker from manipulating global data or hijacking control by reusing code. Without this, ASLR is ineffective. Enabling this by default in our compiler (there is a configure flag) means we will need to rebuild all packages with static libraries (which should be a fairly limited set).

Adding -z,now to LDFLAGS disables lazy loading. This means all function symbols are loaded at startup (with minor performance hit), but that means our RELRO support will make everything ro instead of some of the things. Doing this enables us to use -fno-plt in our CFLAGS, which is a run-time optimisation allowing faster use of libraries.

Adding -fstack-check to our CFLAGS guarantees stack overflows aren't exploitable.

Note that any of these flags can be disabled in a PKGBUILD if really needed... But if that is the case, bug reports should be filed.

Given an assumed lack of objection, I will enable the build flags in our pacman.conf and rebuild gcc to enable pie and put them in [staging] at the end of this week (what better way to celebrate Halloween). We will need a new devtools release then too. Then the packages with static libraries will need rebuilt.

After that, I would like to see [core] completely rebuilt, and audited to ensure our CFLAGS/LDFLAGS are actually being used in the build.

Cheers, Allan

Will this affect i686 as well? According to this commit ( https://github.com/zen-kernel/zen-kernel/commit/cc701dc61a7187e9dfc300ad6ec3...) at least Ubuntu seems to have skipped that for now.

On 13/12/16 19:23, Jan Alexander Steffens via arch-dev-public wrote:

On Mon, Oct 24, 2016 at 5:56 AM Allan McRae wrote:

...

Hi all,

The results from the test-sec-flags [1] suite are in. Many thanks to those that wrote this and those that submitted results. I'm not going to list the summaries here, but the results show that at worst enabling a bunch of security flags in our packages will have a <1% impact on performance (and more likely a fraction of a percent).

That means we will add all of these to our default CFLAGS/LDFLAGS etc. The changes are:

1) building gcc to enable PIE by default 2) add -z,now to LDFLAGS 3) and -fno-plt and -fstack-check to our CFLAGS

Adding PIE means that programs get loaded at a random address, preventing an attacker from manipulating global data or hijacking control by reusing code. Without this, ASLR is ineffective. Enabling this by default in our compiler (there is a configure flag) means we will need to rebuild all packages with static libraries (which should be a fairly limited set).

Adding -z,now to LDFLAGS disables lazy loading. This means all function symbols are loaded at startup (with minor performance hit), but that means our RELRO support will make everything ro instead of some of the things. Doing this enables us to use -fno-plt in our CFLAGS, which is a run-time optimisation allowing faster use of libraries.

Adding -fstack-check to our CFLAGS guarantees stack overflows aren't exploitable.

Note that any of these flags can be disabled in a PKGBUILD if really needed... But if that is the case, bug reports should be filed.

Given an assumed lack of objection, I will enable the build flags in our pacman.conf and rebuild gcc to enable pie and put them in [staging] at the end of this week (what better way to celebrate Halloween). We will need a new devtools release then too. Then the packages with static libraries will need rebuilt.

After that, I would like to see [core] completely rebuilt, and audited to ensure our CFLAGS/LDFLAGS are actually being used in the build.

Cheers, Allan

Will this affect i686 as well? According to this commit ( https://github.com/zen-kernel/zen-kernel/commit/cc701dc61a7187e9dfc300ad6ec3...) at least Ubuntu seems to have skipped that for now.

That commit shows they disabled it for one package. It will affect both. A

On 13/12/16 19:07, Allan McRae wrote:

On 24/10/16 13:56, Allan McRae wrote:

...

That means we will add all of these to our default CFLAGS/LDFLAGS etc. The changes are:

1) building gcc to enable PIE by default 2) add -z,now to LDFLAGS 3) and -fno-plt and -fstack-check to our CFLAGS

For those wondering what happened with this:

binutils build with gcc with default PIE:

# grep Error binutils-git-89080.bfbf34d-1-x86_64-check.log make[5]: *** [Makefile:7161: incremental_test_2] Error 1 make[5]: *** [Makefile:7185: incremental_test_5] Error 1 make[5]: *** [Makefile:7200: incremental_copy_test] Error 1 make[5]: *** [Makefile:7206: incremental_common_test_1] Error 1 make[5]: *** [Makefile:7132: ehdr_start_test_4] Error 1 readelf: Error: the PHDR segment is not covered by a LOAD segment make[4]: *** [Makefile:5609: check-am] Error 2 make[3]: *** [Makefile:5613: check] Error 2 make[2]: *** [Makefile:941: check-recursive] Error 1 make[1]: *** [Makefile:6134: check-gold] Error 2 make[5]: *** [Makefile:3678: check-DEJAGNU] Error 1 make[4]: *** [Makefile:1953: check-am] Error 2 make[3]: *** [Makefile:1793: check-recursive] Error 1 make[2]: *** [Makefile:1955: check] Error 2 make[1]: *** [Makefile:7547: check-ld] Error 2 make: *** [Makefile:2206: do-check] Error 2

binutils build with current gcc:

# grep Error binutils-git-89080.bfbf34d-1-x86_64-check.log readelf: Error: the PHDR segment is not covered by a LOAD segment

I need time to fix this. It is probably just test suite assumptions rather than errors.

I'm putting these changes on a permanent hold due to a lack of time on my behalf. I'm not putting a binutils build with this many testsuite failures into the repos until I fully understand why this is happening. If default PIE is wanted, someone other than me will need to figure this out. Just build gcc with --enable-default-pie and then build binutils to see the issues in the testsuite. Allan

On 02/02/17 05:06, Bartłomiej Piotrowski wrote:

On 2016-12-24 08:39, Allan McRae wrote:

...

If default PIE is wanted, someone other than me will need to figure this out. Just build gcc with --enable-default-pie and then build binutils to see the issues in the testsuite.

Allan

Default PIE is not everything you planned to implement. What about the rest?

Can't be fucked doing this twice. A

On Fri, 2017-06-30 at 11:07 +0200, Bartłomiej Piotrowski wrote:

On 2016-10-24 05:56, Allan McRae wrote:

...

1) building gcc to enable PIE by default

I am in the middle of rebuilding gcc with --enable-default-pie. When it finishes, I will start a todo for rebuilding packages with static libraries.

I also enabled --enable-default-ssp, which means that -fstack-protector-strong will be dropped from our CFLAGS (as it will be enforced by gcc) on the next opportunity.

Bartłomiej

It's probably a good idea to leave it in CFLAGS for Clang.

On 06/30/17 at 09:49pm, Evangelos Foutras via arch-dev-public wrote:

On 30 June 2017 at 18:56, Daniel Micay via arch-dev-public wrote:

...

It's probably a good idea to leave it in CFLAGS for Clang.

I am planning to patch Clang to follow GCC's behavior similarly to what Alpine does. [1]

We can discuss dropping the related compilation flags from makepkg.conf at a later stage.

[1] https://git.alpinelinux.org/cgit/aports/tree/main/clang

Seeing that Arch is defined, will you upstream these changes? If so (+1) -- Jelle van der Waa

On 30/06/17 22:22, Jelle van der Waa wrote:

On 06/30/17 at 09:49pm, Evangelos Foutras via arch-dev-public wrote:

...

On 30 June 2017 at 18:56, Daniel Micay via arch-dev-public wrote:

...

It's probably a good idea to leave it in CFLAGS for Clang.

I am planning to patch Clang to follow GCC's behavior similarly to what Alpine does. [1]

We can discuss dropping the related compilation flags from makepkg.conf at a later stage.

[1] https://git.alpinelinux.org/cgit/aports/tree/main/clang

Seeing that Arch is defined, will you upstream these changes? If so (+1)

I don't think special-casing distros is the right way to go for PIE and SSP; I would much rather see upstream add a CMake option for those. [1] I'll probably patch isPIEDefault() and GetDefaultStackProtectorLevel() to return true and 2 respectively; assuming it actually works and I am able to address any test failures. [1] https://bugs.llvm.org/show_bug.cgi?id=13410

On 02/07/17 06:51, Bartłomiej Piotrowski wrote:

On 2017-06-30 23:44, Allan McRae wrote:

...

On 30/06/17 19:07, Bartłomiej Piotrowski wrote:

...

On 2016-10-24 05:56, Allan McRae wrote:

...

1) building gcc to enable PIE by default

I am in the middle of rebuilding gcc with --enable-default-pie. When it finishes, I will start a todo for rebuilding packages with static libraries.

I also enabled --enable-default-ssp, which means that -fstack-protector-strong will be dropped from our CFLAGS (as it will be enforced by gcc) on the next opportunity.

Are you adding full RELRO + no-plt at the same time?

A

Yes, and -fstack-check=specific too, although I might drop no-plt if it will cause too many builders.

I thought the conclusion from the Stack Clash bugs was that the current -fstack-check was fundamentally flawed and was being completely rewritten for the next gcc. Is the "=specific" version OK?

On Sun, 2017-07-02 at 08:32 +1000, Allan McRae wrote:

On 02/07/17 06:51, Bartłomiej Piotrowski wrote:

...

On 2017-06-30 23:44, Allan McRae wrote:

...

On 30/06/17 19:07, Bartłomiej Piotrowski wrote:

...

On 2016-10-24 05:56, Allan McRae wrote:

...

1) building gcc to enable PIE by default

I am in the middle of rebuilding gcc with --enable-default-pie. When it finishes, I will start a todo for rebuilding packages with static libraries.

I also enabled --enable-default-ssp, which means that -fstack-protector-strong will be dropped from our CFLAGS (as it will be enforced by gcc) on the next opportunity.

Are you adding full RELRO + no-plt at the same time?

A

Yes, and -fstack-check=specific too, although I might drop no-plt if it will cause too many builders.

I thought the conclusion from the Stack Clash bugs was that the current -fstack-check was fundamentally flawed and was being completely rewritten for the next gcc. Is the "=specific" version OK?

It's safe and they fixed the performance issue already, but it's not perfect in terms of exploit mitigation. They didn't mention that in their report but there are issues involving sizes larger than PTRDIFF_MAX / SSIZE_MAX so for an unbounded VLA / alloca it doesn't necessarily mitigate exploitation. The generic -fstack-check implementation is broken / probably still unsafe, but that's not what's used on x86. The -fstack-check=specific switch just makes sure that the generic impl isn't being used which is already the case on x86. The other issue brought up by Red Hat is that -fstack-check assumes all code is compiled with -fstack-check and optimizes out checks that aren't strictly required if other code is compiled with it. I don't see that as the big issue that they do. Compiling all code with it isn't very far fetched and by delaying it for that reason, they're making it into a bigger issue than starting ASAP. It also sometimes leaves unchecked gaps of a few pages. The larger gap in new kernels deals with that for the main thread, for not secondary threads... glibc will need to adopt a larger, and they seem to be waiting for the kernel to expose the gap size via a sysctl rather than switching to the new way as the default before then. Technically those can be caught for VLAs (but not alloca?) via -fsanitize=vla-bound, but GCC is missing -fsanitize-trap=vla-bound, it only has -fsanitize-undefined-trap-on-error to convert all of the UBSan sanitizers into production trapping mode which might not be desired.

On 2 July 2017 at 19:19, Daniel Micay via arch-dev-public wrote:

Using -fno-plt would be a nice tiny little performance boost at runtime but then it's important to make sure everything is compiled with -Wl,- z,now and there might be programs ignoring LDFLAGS but respecting CFLAGS. Ideally -z now would be the default in the linker first. If we aren't going to patch the default, then I think a configure flag for that needs to land upstream.

It's also worth noting that clang does not support the -fno-plt option and I couldn't find any discussions about adding support for it. If it's only a tiny performance improvement, I strongly believe we should skip it for now.

On Wed, 2017-07-05 at 12:36 +0300, Evangelos Foutras wrote:

On 2 July 2017 at 19:19, Daniel Micay via arch-dev-public wrote:

...

Using -fno-plt would be a nice tiny little performance boost at runtime but then it's important to make sure everything is compiled with -Wl,- z,now and there might be programs ignoring LDFLAGS but respecting CFLAGS. Ideally -z now would be the default in the linker first. If we aren't going to patch the default, then I think a configure flag for that needs to land upstream.

It's also worth noting that clang does not support the -fno-plt option and I couldn't find any discussions about adding support for it.

If it's only a tiny performance improvement, I strongly believe we should skip it for now.

There's no loss of compatibility from only some code using it. The only issue with it is that immediate binding *must* be used to support it, so if CFLAGS is respected then LDFLAGS *must* be respected, or immediate binding needs to be set as the default in the linker(s).

On Wed, 2017-07-05 at 21:06 +0300, Evangelos Foutras wrote:

On 5 July 2017 at 19:51, Daniel Micay wrote:

...

On Wed, 2017-07-05 at 12:36 +0300, Evangelos Foutras wrote:

...

On 2 July 2017 at 19:19, Daniel Micay via arch-dev-public wrote:

...

Using -fno-plt would be a nice tiny little performance boost at runtime but then it's important to make sure everything is compiled with -Wl,- z,now and there might be programs ignoring LDFLAGS but respecting CFLAGS. Ideally -z now would be the default in the linker first. If we aren't going to patch the default, then I think a configure flag for that needs to land upstream.

It's also worth noting that clang does not support the -fno-plt option and I couldn't find any discussions about adding support for it.

If it's only a tiny performance improvement, I strongly believe we should skip it for now.

There's no loss of compatibility from only some code using it. The only issue with it is that immediate binding *must* be used to support it, so if CFLAGS is respected then LDFLAGS *must* be respected, or immediate binding needs to be set as the default in the linker(s).

By lacking support I meant that clang aborts the compilation and throws an error with -fno-plt:

clang-4.0: error: unknown argument: '-fno-plt'

I briefly discussed this on IRC with Bartłomiej and he finds it acceptable to add workarounds (removing -fno-plt from CFLAGS) to the few dozen packages that use clang for building (not counting AUR packages).

I disagree and do not consider insignificant performance increases to be sufficient justification to add GCC 6+ specific options to the default CFLAGS and necessitate adding workarounds to packages that do not use the default compiler.

And then there is the (minor) issue you highlight, and possibly other issues we don't know about (since -fno-plt hasn't been adopted by any other distro).

On a related note, it is unclear to me whether you mean that packages might fail to build (and/or run?) if they only respect CFLAGS but not LDFLAGS, or that a package might fail to build (and/or run?) because one of its dependencies didn't respect LDFLAGS.

Using -fno-plt requires linking objects built with the code compiled with that option using -Wl,-z,now but I don't think a build failure will happen if it's missing. It will be a fault (not memory corruption) at runtime when it tries to use a function via -fno-plt calling convention that hasn't been lazily linked via PLT calling convention. It's likely never going to be a subtle failure that isn't blatantly obvious though. So for example, lets say a static library respects CFLAGS and gets compiled with -fno-plt. Lets say it's linked into an executable that does not respect LDFLAGS. Alternatively, the executable build system may respect LDFLAGS but overrides CFLAGS like 'CFLAGS := -O2' or something similar, which would break in the same way. So I think it would be a good idea to flip the default to -z,now in the linker if we're going to use -fno-plt. I think they'd take a patch for that upstream. Clang issue could be avoided with a 1 line patch adding another no-op flag and they'd take that upstream. It's harmless to use the slower lazy linking calling convention when -fno-plt is passed. -fno-plt code is fully compatible with non -fno-plt code, the only requirement is that -fno-plt code is linked with -Wl,-z,now which works fine for non -fno-plt code too and is desirable for security either way.