Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-112301: Add fortify source level 3 to default compiler options#121520
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
@corona10 can we test with buildbots? |
bedevere-bot commentedJul 9, 2024
Misc/NEWS.d/next/Security/2024-07-08-23-39-04.gh-issue-112301.TD8G01.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
ghost commentedJul 9, 2024 • edited by ghost
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by ghost
Uh oh!
There was an error while loading.Please reload this page.
@nohlson I will take a look at this PR by this weekend, sorry, I am too busy these days. |
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
1d61e81 to7a595e7Compare
corona10 left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nohlson
I have 2 questions.
- Is this flag the subset of the overall policy that we want to measure?
#112301 (comment) - Do you have a plan to provide some kind of
./configure --disable-openssf-guide?
I would prefer the options we introduce for this effort to be always on, and put a lot of consideration into benchmark impacting options we do include, and possibly not enabling them if we deem the impact too much rather than enabling everything and making a new configure argument. |
I am pretty sure that there will be users who want to disable options that we added for the OpenSSF guide with a single configuration command :) |
vstinner commentedJul 18, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This change broke RHEL8 buildbot, test_cext and test_cppext fail with: |
@nohlson Would you like to check? |
vstinner commentedJul 19, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I suggest checking for the FORTIFY flag using |
We already do. |
For other flags yes, but apparently, not for |
…er options (pythongh-121520)"Adding the flag broke buildbots.This reverts commitbdab67e.
gh-112301: Added
-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3to default compiler options for all buildsThis option adds runtime protections for glibc to abort execution when unsafe behavior is encountered. Here are the GNU docs on the option
This is a very brief writeup that I found useful from Red Hat explaining some benefits
Also the OpenSSF compiler hardening guidance gives a very good description of the option
This is an option that theoretically affects the runtime so pyperformance benchmarks were run.The benchmark for this branch shows little overall impact but does impact some benchmark types:
A benchmark was run a few weeks ago with this option that was slightly different:
The benchmarks show that overall there is little memory impact and overall very small performance impact but within benchmark types there is some movement . Many compilers use
-D_FORTIFY_SOURCE=2by default. Level 3 adds additional bounds checking. My recommendation and the recommendation of the OpenSSF guidance is to raise to level 3 for this protection but would like to discuss further.Attn:@mdboom