Linux: build with and test for control flow instrumentation on x86_64 #23839
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
This was referenced Dec 22, 2021
fanquake
added a commit
to fanquake/bitcoin
that referenced
this pull request
Dec 23, 2021
Our minimum required GCC is GCC 8, and this change in required for changes like bitcoin#23839 which take advantage of flags introduced in that version of GCC. This should have been included as part of 182de7b.
5a0da56 to
2f30d95
Compare
|
Rebased on top of #23845 which fixes the previous releases CI (by using GCC 8 instead of 7). Also fixed the linting issue and simplified the |
maflcko
pushed a commit
that referenced
this pull request
Dec 23, 2021
2da97b2 ci: use GCC 8 when building packages in native_qt5 CI (fanquake) Pull request description: Our minimum required GCC is GCC 8, and this change in required for PRs like #23839 which take advantage of flags introduced in that version of GCC. This should have been included as part of 182de7b. ACKs for top commit: MarcoFalke: cr ACK 2da97b2 hebasto: ACK 2da97b2 Tree-SHA512: 2b64c21119fb95b959ac0f7d8d2d38f6ed98309695bb35425fad45b3b628247c2c78d0647c4d1f511669e8d333c6febe1cc44fac8027ed0bd7593eb630add548
2f30d95 to
5559e65
Compare
This is required so that we can test for control flow in our ELF security checks. Otherwise test_bitcoin will fail, as it has a main that is provided by Boost, and wont have been built with -fcf-protection=full. While here simplify to using -fcf-protection for all x86_64 builds.
5559e65 to
5a8f907
Compare
|
Un-drafted, now that the base PR's have been merged. Added Guix hashes to PR description. |
Guix builds: |
luke-jr
reviewed
Jan 11, 2022
| main = binary.get_function_address('main') | ||
| content = binary.get_content_from_virtual_address(main, 4, lief.Binary.VA_TYPES.AUTO) | ||
|
|
||
| if content == [243, 15, 30, 250]: # endbr64 |
There was a problem hiding this comment.
This feels kind of ugly. Is there no better way to check it? Will this reliably check it with future compilers?
There was a problem hiding this comment.
Is there no better way to check it?
Do you have any suggestions? As far as I'm aware there's no standard way to check if a binary has control instrumentation or not. I think this is about the most direct way we can test for what we're looking for.
Will this reliably check it with future compilers?
I'm not going to try and guess what future compilers may or may not do, but I would be surprised if this did stop working. In any case, it would be immediately obvious, and would only occur when making changes to our Guix environment, so at that point we could figure out what to do.
There was a problem hiding this comment.
I think this is the only way to do it. I also think it is future-proof, there's only one way to do control flow experimentation. And yeah, if not, we can check what to do. Compiler upgrades are an explicit choice anyhow.
mzumsande
pushed a commit
to mzumsande/bitcoin
that referenced
this pull request
Jan 17, 2022
Our minimum required GCC is GCC 8, and this change in required for changes like bitcoin#23839 which take advantage of flags introduced in that version of GCC. This should have been included as part of 182de7b.
|
Code review ACK 5a8f907 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jan 28, 2022
…rumentation on x86_64 5a8f907 scripts: add CONTROL_FLOW to ELF security checks (fanquake) e13f8f7 build: build x86_64 Linux Boost with -fcf-protection=full (fanquake) 6ca5efa script rename control flow check to MACHO specific (fanquake) Pull request description: Closes bitcoin#21888. TODO: * Duplication in security-check-tests Guix build: ```bash bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum 8025e2e6859247eaf865a4a5009d0a39570ec5e8ab51739fa4da0d8ee4ab2117 guix-build-5a8f907c93f1/output/aarch64-linux-gnu/SHA256SUMS.part 75576482994493262dbf7d3567b0450c0804bdc75c186d6b4c6a856bd0d8f0a3 guix-build-5a8f907c93f1/output/aarch64-linux-gnu/bitcoin-5a8f907c93f1-aarch64-linux-gnu-debug.tar.gz 7cb89c9327cc540859334b597c041a2250156e3e83ce2aa7d16177376056302f guix-build-5a8f907c93f1/output/aarch64-linux-gnu/bitcoin-5a8f907c93f1-aarch64-linux-gnu.tar.gz e264053e4c7a5d65340dd7bbd2c664bcd596292ab80f00a0bf0026bfe0c480bc guix-build-5a8f907c93f1/output/arm-linux-gnueabihf/SHA256SUMS.part 6c3ee9b3c0c4583620301b183226678f1618605dd07dbed0bbdf7d06d3591314 guix-build-5a8f907c93f1/output/arm-linux-gnueabihf/bitcoin-5a8f907c93f1-arm-linux-gnueabihf-debug.tar.gz f217cb8d8e233a8dbdbfde7dabf12c5d867d7de53e8c652b8ed1d4a55da82fd9 guix-build-5a8f907c93f1/output/arm-linux-gnueabihf/bitcoin-5a8f907c93f1-arm-linux-gnueabihf.tar.gz 48b4b2a1b52b3098f4e92c11cb60f1e5e9696a2a960560cd6adea72277eaa4a4 guix-build-5a8f907c93f1/output/dist-archive/bitcoin-5a8f907c93f1.tar.gz 40832db2446e129879caa9fbc9d682c53069876dbb2e0d4d76592e5dcb40bb12 guix-build-5a8f907c93f1/output/powerpc64-linux-gnu/SHA256SUMS.part 26fdfa9b7b77d5db415ef34054cf6f6d020a5dab73239db6dd05539f654bc5d5 guix-build-5a8f907c93f1/output/powerpc64-linux-gnu/bitcoin-5a8f907c93f1-powerpc64-linux-gnu-debug.tar.gz 3adbdb9d3eb1cb5f9adc38b29450054f286bd6d74cef8619adaee89494853605 guix-build-5a8f907c93f1/output/powerpc64-linux-gnu/bitcoin-5a8f907c93f1-powerpc64-linux-gnu.tar.gz 8ec2baf82483a698350bfdabf530cd9b5241690c916f597c746210e95ac451de guix-build-5a8f907c93f1/output/powerpc64le-linux-gnu/SHA256SUMS.part 1797e75e1f66ec9068fa4e57e0108960475e863f8f054fbe854358b1f995c4df guix-build-5a8f907c93f1/output/powerpc64le-linux-gnu/bitcoin-5a8f907c93f1-powerpc64le-linux-gnu-debug.tar.gz 448bd289ef26c777a1fc4498e7ba7fb17d0f6a932dcac91b2f89cbba63704bb8 guix-build-5a8f907c93f1/output/powerpc64le-linux-gnu/bitcoin-5a8f907c93f1-powerpc64le-linux-gnu.tar.gz a23ee91eeae515c2a6a31eb25d659fab833839aaafa4676ccee364bdad2a468c guix-build-5a8f907c93f1/output/riscv64-linux-gnu/SHA256SUMS.part 82df1b6d5020d0af8268ecc8e823f752f20dec308277763b2dd675804dfa4bbd guix-build-5a8f907c93f1/output/riscv64-linux-gnu/bitcoin-5a8f907c93f1-riscv64-linux-gnu-debug.tar.gz 09a6098ce83896a6ee6d5c8aff12eaca51595bd724c8e0b2a6f90b6410dc168c guix-build-5a8f907c93f1/output/riscv64-linux-gnu/bitcoin-5a8f907c93f1-riscv64-linux-gnu.tar.gz 4fac2951f80eaa2bd1747a263be6be1b76282cac5062f7d86db631a2fb80f8db guix-build-5a8f907c93f1/output/x86_64-apple-darwin/SHA256SUMS.part 3392f417b09efca5916c384f0b2d0c177a72ec4921399c62e84484f0054cc8c4 guix-build-5a8f907c93f1/output/x86_64-apple-darwin/bitcoin-5a8f907c93f1-osx-unsigned.dmg bfb4f8ade6107996ec4bc9efdb53959151b8fb19b6790c34472fe218dd02383d guix-build-5a8f907c93f1/output/x86_64-apple-darwin/bitcoin-5a8f907c93f1-osx-unsigned.tar.gz 4de92e149bad46fc863efb3b650753d194aad96be991d020e0b859452cf27457 guix-build-5a8f907c93f1/output/x86_64-apple-darwin/bitcoin-5a8f907c93f1-osx64.tar.gz 9329549a2f275a59e329afc8744ff8cbc297f3042e0ad03b47626805c7aae2e8 guix-build-5a8f907c93f1/output/x86_64-linux-gnu/SHA256SUMS.part d87deb1eca8a1d3780f02edde78350d27f700e13c7ff444be0bfb34e7369904a guix-build-5a8f907c93f1/output/x86_64-linux-gnu/bitcoin-5a8f907c93f1-x86_64-linux-gnu-debug.tar.gz 76f3bc2fad010d9373e854d941e8205b68fa6c9a8ecaff34c4978ae3ae76c806 guix-build-5a8f907c93f1/output/x86_64-linux-gnu/bitcoin-5a8f907c93f1-x86_64-linux-gnu.tar.gz 2948631081c4bd475529da2b7bc2b32b5aa2e44de46dfdaa6cf9432b3c9fe869 guix-build-5a8f907c93f1/output/x86_64-w64-mingw32/SHA256SUMS.part c5d56fac163b73c00e9745aa7b0f9a0bd9fcac7517e39b677869b6e76faf7218 guix-build-5a8f907c93f1/output/x86_64-w64-mingw32/bitcoin-5a8f907c93f1-win-unsigned.tar.gz f12c7cd72511544c67f4934981c090cee0d9c17e931c059edbbbef6e843dd651 guix-build-5a8f907c93f1/output/x86_64-w64-mingw32/bitcoin-5a8f907c93f1-win64-debug.zip eb952cc4554f92ba6787353f4305d7cbcb1c6dafc4a3867b3088106252a573b8 guix-build-5a8f907c93f1/output/x86_64-w64-mingw32/bitcoin-5a8f907c93f1-win64-setup-unsigned.exe 7bd2b071f1cdf9410535e6a10dd1da519f942bd2c1e47ed52c5b8c4f977e8f27 guix-build-5a8f907c93f1/output/x86_64-w64-mingw32/bitcoin-5a8f907c93f1-win64.zip ``` ACKs for top commit: laanwj: Code review ACK 5a8f907 hebasto: ACK 5a8f907, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 75702ac01175ccf08e73b5c3bce266cee9dd15ccf8fb38d46108cfada156de9a8c18e27d996f9343ae33f7a75a6904d335cbe25256d90af1f4ccbe72ce1788bb
rebroad
pushed a commit
to rebroad/bitcoin
that referenced
this pull request
Feb 3, 2022
Our minimum required GCC is GCC 8, and this change in required for changes like bitcoin#23839 which take advantage of flags introduced in that version of GCC. This should have been included as part of 182de7b.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #21888.
TODO:
Guix build: