Page MenuHomeFreeBSD

Adopt higher level of stack protection.
AbandonedPublic

Authored by koobs on Aug 23 2015, 3:21 AM.
Referenced Files
F106017829: D3463.diff
Mon, Dec 23, 11:53 PM
Unknown Object (File)
Mon, Dec 9, 2:39 PM
Unknown Object (File)
Oct 29 2024, 11:20 PM
Unknown Object (File)
Sep 12 2024, 8:13 PM
Unknown Object (File)
Aug 16 2024, 9:32 PM
Unknown Object (File)
Aug 6 2024, 5:32 AM
Unknown Object (File)
Aug 5 2024, 9:55 PM
Unknown Object (File)
Jun 27 2024, 7:41 PM

Details

Reviewers
jlh
pfg
imp
op
Group Reviewers
secteam
Summary

It is well known that the normal stack protection supported from
the early gcc days is very weak. Google engineers have developed
a new ssp-protector-strong option that provides much better
protection without hurting performance.

This has been adopted by most linux distributions[2]
and OpenBSD. Apparently clang in FreeBSD 11 supports it and
we recently brought support for that option in our base gcc
so it seems like a good moment to adopt such option.

This is complementary but unrelated to the GSoC 2015
to bring the FORTIFY_SOURCE functionality to libc.

[1] https://lwn.net/Articles/584225/
[2] https://securityblog.redhat.com/tag/stack-protector/

Test Plan

Tested for performance with dbench (pho).
Passed port exp-run with stricter --stack-protector-all.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 194
Build 194: arc lint + arc unit

Event Timeline

pfg retitled this revision from to Adopt higher level of stack protection..
pfg updated this object.
pfg edited the test plan for this revision. (Show Details)
pfg added a reviewer: op.
imp requested changes to this revision.Aug 23 2015, 3:25 AM
imp added a reviewer: imp.

What's the performance impact?

This revision now requires changes to proceed.Aug 23 2015, 3:25 AM
This comment was removed by op.

We should investigate that the -fstack-protector-strong triggers the same as -fstack-protector-all:

https://github.com/freebsd/freebsd/blob/master/lib/libc/Makefile#L187

I could add this patch to HardenedBSD 11-CURRENT-unstable, and see what's happen.

In D3463#70669, @imp wrote:

What's the performance impact?

According to redhat/fedora the performance impact is negligible[1] and all linux distributions have been passing the word without any hard numbers.

As I see it, the FreeBSD project trusted this technology but it has a completely inadequate coverage (about 2%), this bumps the coverage to about 20% without adding significant overhead. The alternative should just be disabling completely the stack protection for production builds, since 2% is just useless and AFAICT has provided no real protection.

I also see this a temporary step while we figure out the safestack bits and as an alternative for builds that still depend on GCC.

[1] https://fedorahosted.org/fesco/ticket/1128

This comment was removed by pfg.
This comment was removed by pfg.

I'll patch my laptop and have started a clean build so I'll see if there would be visible performance/other impact after running with it for a few days.

Will it be possible that you can provide some basic test data, for instance, a few world stone runs on memory disk, and see how exactly the impact would be? It would be easier to convince people when you have first hand data, and although we know others have already do some testing, it's important to know if the same applies on FreeBSD because it's likely that the experiments were not on FreeBSD, and the result may be different.

I'd recommend requesting an exp-run in the meantime, by the way. In general I'm for this be enabled by default, as long as the performance change is within in an reasonable range and thanks for working on it!

...

Will it be possible that you can provide some basic test data, for instance, a few world stone runs on memory disk, and see how exactly the impact would be? It would be easier to convince people when you have first hand data, and although we know others have already do some testing, it's important to know if the same applies on FreeBSD because it's likely that the experiments were not on FreeBSD, and the result may be different.

OK, yes I see. Also the clang implementation may present important differences with the GCC implementation. I will running some other tests.

I'd recommend requesting an exp-run in the meantime, by the way. In general I'm for this be enabled by default, as long as the performance change is within in an reasonable range and thanks for working on it!

An exp-run is unlikely to show anything but I think I will add in a patch to activate relro (which is also supposed to have no performance impact) so we can test both changes simultaneously over the ports tree.

Peter Holm has kindly provided some testing:

"... There seems to be no difference between a pristine build and one with your patch.
Your patch only seems to add 128 bytes to the kernel size, so if that
is true that could explain the findings. If I had more time I would
probably investigate the consequence of the flag compared the object
file size of a few files. ..."

This mostly matches the results from linux but I still have to look at the details.

For the exp-run I opened PR 203394 (it includes another change as well which shouldn't have any consequence)..

Results from exp-run (PR 2013394) are out:


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 to Peter Holm and Antoine Brodin for the extensive testing.

Given that we now have hard evidence from dbench that there is no performance hit from the patch and that the change doesn't cause any breakage in regular operation, I intend to commit the change between today and tomorrow (I'll give some time for people to express approval for the change).

Committed as r288669.

This is already in the tree (apparently no one approved it though).

koobs added a reviewer: pfg.

Apologies, I intended to Commandeer -> Close.