Created attachment 161460 [details] Enable stack-protector-strong and RELRO As discussed in D3463, we would like to consider the impact of making the stack-protector stronger as is done in most linux distributions. While here, include a patch to make our binutils enable partial RELRO by default. These changes should cause no impact on the ports.
Created attachment 161577 [details] Enable stack-protector-ALL and RELRO I have been doing some experimental builds and while we would still only enable stack-protector-strong by default .... we can make the experiment stricter: stack-protector-all puts some more pressure into the system but we should catch more errors.
So this is going to likely build OK but the "errors" to be caught would happen at runtime, yes? And then how do we get these packages out to the masses without bumping PORTREVISION?
(In reply to Mark Felder from comment #2) First of all this is a stricter patch than what would eventually be committed. The idea is to detect bugs in the toolchain and in programs that do their automatic testing and/or bootstrapping before bringing the lighter change. If buffer overflows are detected during the exp-run, or after committing the lighter patch, they must be fixed individually in the each of the ports so I would expect PORTREVISION would be bumped for the individual port(s). This said, if the individual ports are up to date it's unlikely stack-protector-strong will reveal any new bug since this has been done already for a while by most linux distributions. The change will not be dramatic ... we are just catching up.
(In reply to Pedro F. Giffuni from comment #3) I understand. I'm just saying that getting updated packages built with stack-protector-strong onto endusers' computers will be a long process because they won't instantly be downloading the newly built packages.
The proposed patch is for base/head, it doesn't affect packages built on 9.3, 10.1 or 10.2 security branches.
(In reply to Antoine Brodin from comment #5) Also .. it won't be MFCd as the older clang in 10.x doesn't support stack-protector-strong.
Exp-run results on head i386: http://package23.nyi.freebsd.org/jail.html?mastername=headi386PR203394-default Exp-run results on head amd64: http://package22.nyi.freebsd.org/jail.html?mastername=headamd64PR203394-default 0 new failure
Huge thanks for the testing! I have updated the code review to reflect the results.
Aren't we missing setting this in /usr/ports/Mk/bsd.ssp.mk? Would the next logical step here be an exp-run for ports with bsd.ssp.mk using -fstack-protector-strong (or -all)?
Created attachment 161684 [details] Mk/bsd.ssp.mk: Enable -fstack-protector-all for testing > These changes should cause no impact on the ports. I'm not sure if I misinterpreted this as meaning the change would not change stack protector for ports or that the change would change the stack protector for ports and not impact an the build. Nonetheless, here's a patch for discussion that's the logical next step. This is for a more aggressive test with -all for CURRENT after Clang 3.5 was imported.
Created attachment 161685 [details] Mk/bsd.ssp.mk: Enable -fstack-protector-all for testing Syntax fixup
(In reply to Jason Unovitch from comment #11) gcc from base at osversion 1100052 didn't support -fstack-protector-strong
(In reply to Jason Unovitch from comment #9) Hmm... I was only interested in testing the change for the base, which as you said was the first logical step. So basically only the toolchain was tested (there was some more testing done with dbench). The partial RELRO did affect most ports, but it was there basically to take advantage of the exp-run. The patch does look OK, but I am not a ports committer so it's up to ports guys to decide. It doesn't bother me if you want to hijack this PR, but perhaps it should be discussed in a separate PR ;).
(In reply to Antoine Brodin from comment #12) Good call. It looks like that was introduced in https://svnweb.freebsd.org/changeset/base/286074 on July 30. OSVERSION of 1100078 was cut on 12 Aug and could be an option. (In reply to Pedro F. Giffuni from comment #13) OK, that makes sense. I concur further discussion should be done in another PR so I obsoleted this for now.
A commit references this bug: Author: pfg Date: Sun Oct 4 18:54:03 UTC 2015 New revision: 288669 URL: https://svnweb.freebsd.org/changeset/base/288669 Log: Bump the stack protector to level "strong". The general stack protector is known to be weak and has pretty small coverage. While setting stack-protector-all would give better protection it would come with a performance cost: for this reason Google's Chrome OS team developed a new stack-protector-strong variant. In addition to the protections offered by -fstack-protector, the new option will guard any function that declares any type or length of local array, even those in structs or unions. It will also protect functions that use a local variable's address in a function argument or on the right-hand side of an assignment. The option was introduced in GCC-4.9, but support for it has been back-ported to our base GCC (r286074) and is also available in clang. The change was tested with dbench and doesn't introduce performance regressions. An exp-run over the ports tree revealed no failures when using the stricter stack-protector-all. Thanks to all testers involved. Reference: https://outflux.net/blog/archives/2014/01/27/fstack-protector-strong/ Tested by: pho, portmgr (antoine) Discussed with: secteam (delphij) Differential Revision: https://reviews.freebsd.org/D3463 PR: 203394 (exp-run) Relnotes: yes MFC: no (not supported in older clang) Changes: head/share/mk/bsd.sys.mk
This should be closed.