Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable OSSF compiler hardening flags by default #9441

Merged
merged 10 commits into from
Mar 4, 2025

Conversation

garazdawi
Copy link
Contributor

This PR enables some extra gcc/clang flags by default that disallow certain types of bugs/attack vectors. From what I can tell the flags do not impact performance, but depending on the usecase they might.

The flags are taken from https://github.com/ossf/wg-best-practices-os-developers/blob/main/docs/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.md#-fno-strict-overflow

@garazdawi garazdawi added team:VM Assigned to OTP team VM enhancement labels Feb 14, 2025
@garazdawi garazdawi self-assigned this Feb 14, 2025
Copy link
Contributor

github-actions bot commented Feb 14, 2025

CT Test Results

   11 files    253 suites   3h 23m 19s ⏱️
3 240 tests 3 103 ✅ 137 💤 0 ❌
4 709 runs  4 312 ✅ 397 💤 0 ❌

Results for commit 9c916ab.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@garazdawi garazdawi added this to the OTP-28.0 milestone Feb 17, 2025
@garazdawi garazdawi added the testing currently being tested, tag is used by OTP internal CI label Feb 17, 2025
sverker
sverker previously approved these changes Feb 17, 2025
@sverker sverker removed the testing currently being tested, tag is used by OTP internal CI label Feb 20, 2025
@garazdawi garazdawi force-pushed the lukas/otp/ossf-compiler-flags branch from f29ae91 to 32841a7 Compare February 21, 2025 07:30
@garazdawi garazdawi added the testing currently being tested, tag is used by OTP internal CI label Feb 21, 2025
@garazdawi garazdawi force-pushed the lukas/otp/ossf-compiler-flags branch from facce89 to 727cf7c Compare February 21, 2025 13:30
@garazdawi garazdawi requested a review from sverker February 21, 2025 13:30
sverker
sverker previously approved these changes Feb 24, 2025
@garazdawi
Copy link
Contributor Author

garazdawi commented Feb 24, 2025

I've come to the realization that if we want to be able to enable these flags by default, we need to force usage of $(CC) as the linker, and not use $(CC) in some places and ld in other. This because the flags passed to the two are very different and we cannot use the builtin AC_LINK_IFELSE at all when LDFLAGS are flags to the LD process.

It is only DED_LD on some platforms (solaris) that use ld as the linker right now.

I don't think doing this should break anything, but it is hard to know for sure.

@garazdawi garazdawi added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Feb 24, 2025
@garazdawi garazdawi force-pushed the lukas/otp/ossf-compiler-flags branch 2 times, most recently from 259d8b3 to df6ccf4 Compare February 25, 2025 09:54
We do this as AC_LINK_IFELSE needs LDFLAGS to be in $(CC) format
to work and all OSs support using $(CC) as the linker.
This is done so that we in docker environments can install
valgrind after configure has been run.
@garazdawi garazdawi force-pushed the lukas/otp/ossf-compiler-flags branch from df6ccf4 to 9c916ab Compare March 3, 2025 20:01
@garazdawi garazdawi requested a review from sverker March 3, 2025 20:01
@garazdawi garazdawi merged commit 057cc09 into erlang:master Mar 4, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants