diff options
| author | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-04-22 15:15:12 +0200 |
|---|---|---|
| committer | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-04-22 15:18:11 +0200 |
| commit | 5dcb0615898216c503e965a01d855a5999a586b5 (patch) | |
| tree | 1f3f40770e332b58f30605e1c83a7ad1480bc0f5 | |
| parent | ce4e1f0282c6a622e8252f06028f1da416a79e23 (diff) | |
| parent | 315a4d36f716341a38bc4e4de8630b3246d27dbc (diff) | |
Merge #18702: build: fix ASLR for bitcoin-cli on Windows
315a4d36f716341a38bc4e4de8630b3246d27dbc build: fix ASLR for bitcoin-cli on Windows (fanquake)
Pull request description:
ASLR is not currently working for the `bitcoin-cli.exe` binary. This is
due to it not having a .reloc section, which is stripped by default by
the mingw-w64 ld we use for gitian builds. A good summary of issues with
ld and mingw-w64 is available in this thread:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011.
All other Windows binaries that we distribute (bitcoind, bitcoin-qt,
bitcoin-wallet, bitcoin-tx and test_bitcoin) do not suffer this issue,
and currently having working ASLR. This is due to them exporting
(inadvertent or not) libsecp256k1 symbols, and, as a result, the .reloc
section is not stripped by ld.
This change is a temporary workaround, also the same one described here:
https://www.kb.cert.org/vuls/id/307144/, that causes main() to be
exported. Exporting a symbol will mean that the .reloc section is not
stripped, and ASLR will function correctly.
Ultimately, this will be fixed by using a newer version of binutils (that has this [change](https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=dc9bd8c92af67947db44b3cb428c050259b15cd0)). Whether that's through bumping our gitian distro, or Guix.
Related to #18629, which has a bunch of additional information in the PR description. If you would like to verify whether or not ASLR is indeed working, with or without this change. One easy way to check is using a tool like [VMMap](https://docs.microsoft.com/en-us/sysinternals/downloads/vmmap).
Here are the memory mappings for the 0.20.0rc1 `bitcoind.exe` and `bitcoin-cli.exe` binaries. You'll notice that over machine restarts, even though the image is marked `(ASLR)` (which I assume may be due to the header bit being set), no ASLR is actually occuring for `bitcoin-cli.exe`:
#### bitcoind.exe
#### bitcoin-cli.exe
ACKs for top commit:
dongcarl:
ACK 315a4d36f716341a38bc4e4de8630b3246d27dbc
laanwj:
ACK 315a4d36f716341a38bc4e4de8630b3246d27dbc
Tree-SHA512: 95f4dc15420ed9bcdeacb763e11c3c7e563eec594a172746fa0346c13f97db3a8769357dffc89fea1e57ae67133f337b1013a73b584662f5b6c4d251ca20a2b1
| -rw-r--r-- | src/bitcoin-cli.cpp | 12 |
1 files changed, 10 insertions, 2 deletions
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 02efefe671..cdaabd6fab 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -551,11 +551,19 @@ static int CommandLineRPC(int argc, char *argv[]) return nRet; } -int main(int argc, char* argv[]) -{ #ifdef WIN32 +// Export main() and ensure working ASLR on Windows. +// Exporting a symbol will prevent the linker from stripping +// the .reloc section from the binary, which is a requirement +// for ASLR. This is a temporary workaround until a fixed +// version of binutils is used for releases. +__declspec(dllexport) int main(int argc, char* argv[]) +{ util::WinCmdLineArgs winArgs; std::tie(argc, argv) = winArgs.get(); +#else +int main(int argc, char* argv[]) +{ #endif SetupEnvironment(); if (!SetupNetworking()) { |