-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
cc-wrapper hardeningFlags tests: fix expected behaviour in corner cases, add tests for stackclashprotection
#253186
Conversation
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/2705 |
also use fortify1-based tests in some places that it may allow us to better test the behaviour of toolchains that only support that
2423b61
to
ec8d29a
Compare
stackclashprotection
Have pushed some further fixes and added some tests for the recently added Would love to get this merged and have a little more green around https://hydra.nixos.org/eval/1807613?filter=hardeningflags&compare=1807605&full= |
these were not updated to understand hardeningUnsupportedFlagsByTargetPlatform when it was added causing more tests to fail for clang than otherwise would
70ba12b
to
2e0d7e2
Compare
@ofborg build tests.hardeningFlags-gcc tests.hardeningFlags-clang |
I successfully ran
I just wanted to check if this is expected or not, as it’s not in your list of known failing tests. |
I assume this is Is it that it's not possible to disable pie on clang/linux? Are we just not supplying the right flags at the right point? Don't know - it's something we need to investigate at some point. |
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.
I’m pretty sure you can turn off PIE with Clang? But yeah, I have no idea.
tests.hardeningFlags-clang
builds on x86_64-darwin
, tests.hardeningFlags-gcc
doesn’t but that’s probably unrelated. Looks good to me!
Description of changes
A few fixups for the recently merged #217390, straightening out the expected behaviour of one test to match current reality and changing a few tests to use a
_FORTIFY_SOURCE=1
-protectable example program so that it can be more useful for testing clang's behaviour too.Note there are two tests here that will (still) be failing on
tests.hardeningFlags-clang
until I can get the fix for that merged (#253194, but is headed for staging):fortify3EnabledEnvEnablesFortify1
andfortify3EnabledEnvEnablesFortify1ExecTest
.Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)