guix: patch NSIS to remove .reloc sections from installer stubs #25788
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
With the release of binutils/ld 2.36, ld swapped to much improved default settings when producing windows binaries with mingw-w64. One of these changes was to stop stripping the .reloc section from binaries, which is required for working ASLR. .reloc section stripping is something we've accounted for previously, see bitcoin#18702. The related upstream discussion is in this thread: https://sourceware.org/bugzilla/show_bug.cgi?id=19011. When we switched to using a newer Guix time-machine in bitcoin#23778, we begun using binutils 2.37 to produce releases. Since then, our windows installer (produced with makensis) has not functioned correctly when run on a Windows system with the "Force randomization for images (Mandatory ASLR)" option enabled. Note that all of our other release binaries, which all contain .reloc sections, function fine under the same option, so it cannot be just the presence of a .reloc section that is the issue. For now, restore makensis to it's pre-binutils-2.36 behaviour, which fixes the produced installer. The underlying issue can be further investigated in future.
hebasto
reviewed
Aug 5, 2022
Member
There was a problem hiding this comment.
Concept ACK 7a0b129.
Tested on Windows 11 Pro 21H2 with the "Force randomization for images (Mandatory ASLR)" option being set to "On by default".
I can run bitcoin-7a0b129c41d9-win64-setup-unsigned.exe successfully.
Guix builds on x86_64:
7e0723388913ac1ec9f650b943c6b23351ba0cd921c0ec830abf16b16724d503 guix-build-7a0b129c41d9/output/dist-archive/bitcoin-7a0b129c41d9.tar.gz
c3bb9c68895ffafa2900b0d18c1268e299d012a7dc70593f20f9900cf116eb05 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/SHA256SUMS.part
b57aa99c242b0aae64653c64ada38f6d3f0cbd902bbc096d3dc529fdcf87d681 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-debug.zip
341d99afc9961299883be6cd9666e8bc0f3f6296cff758719a32d27419acad36 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-setup-unsigned.exe
1d9ef48d3c9ed93a925962356b41cdaeb9d09fd758de193cd4d5f4d1ec6791eb guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-unsigned.tar.gz
28c81d99a9a4bd6648449393f91db213369e958add579ba9e9a1721540d2c4f7 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64.zip
Member
To be precise, UPDATE: And the |
Member
|
ACK 7a0b129 Tested on Win 10 with Forced ALSR enabled. Works as expected. It would be nice if our installer had functioning ASLR, but I agree that the existing issues and guidance seems to indicate that nsis installers should not have .reloc sections. guix build x86_64: |
jarolrod
reviewed
Aug 5, 2022
Contributor
jarolrod
left a comment
There was a problem hiding this comment.
ACK 7a0b129
guix hashes:
x86:
$ env HOSTS='x86_64-w64-mingw32' ./contrib/guix/guix-build
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
7e0723388913ac1ec9f650b943c6b23351ba0cd921c0ec830abf16b16724d503 guix-build-7a0b129c41d9/output/dist-archive/bitcoin-7a0b129c41d9.tar.gz
c3bb9c68895ffafa2900b0d18c1268e299d012a7dc70593f20f9900cf116eb05 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/SHA256SUMS.part
b57aa99c242b0aae64653c64ada38f6d3f0cbd902bbc096d3dc529fdcf87d681 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-debug.zip
341d99afc9961299883be6cd9666e8bc0f3f6296cff758719a32d27419acad36 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-setup-unsigned.exe
1d9ef48d3c9ed93a925962356b41cdaeb9d09fd758de193cd4d5f4d1ec6791eb guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-unsigned.tar.gz
28c81d99a9a4bd6648449393f91db213369e958add579ba9e9a1721540d2c4f7 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64.zip
arm64:
$ env HOSTS='x86_64-w64-mingw32' ./contrib/guix/guix-build
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
7e0723388913ac1ec9f650b943c6b23351ba0cd921c0ec830abf16b16724d503 guix-build-7a0b129c41d9/output/dist-archive/bitcoin-7a0b129c41d9.tar.gz
c3bb9c68895ffafa2900b0d18c1268e299d012a7dc70593f20f9900cf116eb05 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/SHA256SUMS.part
b57aa99c242b0aae64653c64ada38f6d3f0cbd902bbc096d3dc529fdcf87d681 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-debug.zip
341d99afc9961299883be6cd9666e8bc0f3f6296cff758719a32d27419acad36 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-setup-unsigned.exe
1d9ef48d3c9ed93a925962356b41cdaeb9d09fd758de193cd4d5f4d1ec6791eb guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-unsigned.tar.gz
28c81d99a9a4bd6648449393f91db213369e958add579ba9e9a1721540d2c4f7 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64.zip
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Aug 6, 2022
…installer stubs 7a0b129 guix: patch NSIS to remove .reloc sections from install stubs (fanquake) Pull request description: With the release of binutils/ld 2.36, ld swapped to much improved default settings when producing windows binaries with mingw-w64. One of these changes was to stop stripping the .reloc section from binaries, which is required for working ASLR. When we switched to using a newer Guix time-machine in bitcoin#23778, we begun using binutils 2.37 to produce releases. Since then, our windows installer (produced with makensis) has not functioned correctly when run on a Windows system with the "Force randomization for images (Mandatory ASLR)" option enabled. Note that all of our other release binaries, which all contain .reloc sections, function fine under the same option, so it cannot be just the presence of a .reloc section that is the issue. The root cause of the problem is that when we compile NSIS (makensis), a number of exe installer stubs are produced at the same time, for use later when makensis is actually run. Given the new linker defaults, the stubs will contain .reloc sections, when previously they would not. It seems that, in combination with how makensis mutates the stub when it actually builds the installer, causes the problem. According to upstream, https://sourceforge.net/p/nsis/bugs/1131/#abb6: > Looks like the problem is the very existance of the .reloc section. > It's not supposed to be there, and makensis doesn't handle it. The most recent .reloc related upstream activity is in https://sourceforge.net/p/nsis/bugs/1283/, where the conclusion again seemed to be that .relo sections are not wanted, but there hasn't been any further follow up. For now, restore pre-binutils-2.36 behaviour, by passing `-Wl,--disable-reloc-section` to the linker when building the installer stubs, which fixes the produced installer. The underlying issue can be further investigated in future. .reloc section stripping is something we've accounted for previously, see bitcoin#18702, and related upstream discussion is in this thread: https://sourceware.org/bugzilla/show_bug.cgi?id=19011. Fixes bitcoin#25726. Guix Build (x86_64): ```bash 7e0723388913ac1ec9f650b943c6b23351ba0cd921c0ec830abf16b16724d503 guix-build-7a0b129c41d9/output/dist-archive/bitcoin-7a0b129c41d9.tar.gz c3bb9c68895ffafa2900b0d18c1268e299d012a7dc70593f20f9900cf116eb05 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/SHA256SUMS.part b57aa99c242b0aae64653c64ada38f6d3f0cbd902bbc096d3dc529fdcf87d681 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-debug.zip 341d99afc9961299883be6cd9666e8bc0f3f6296cff758719a32d27419acad36 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-setup-unsigned.exe 1d9ef48d3c9ed93a925962356b41cdaeb9d09fd758de193cd4d5f4d1ec6791eb guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-unsigned.tar.gz 28c81d99a9a4bd6648449393f91db213369e958add579ba9e9a1721540d2c4f7 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64.zip ``` Guix Build (arm64): ```bash 7e0723388913ac1ec9f650b943c6b23351ba0cd921c0ec830abf16b16724d503 guix-build-7a0b129c41d9/output/dist-archive/bitcoin-7a0b129c41d9.tar.gz c3bb9c68895ffafa2900b0d18c1268e299d012a7dc70593f20f9900cf116eb05 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/SHA256SUMS.part b57aa99c242b0aae64653c64ada38f6d3f0cbd902bbc096d3dc529fdcf87d681 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-debug.zip 341d99afc9961299883be6cd9666e8bc0f3f6296cff758719a32d27419acad36 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-setup-unsigned.exe 1d9ef48d3c9ed93a925962356b41cdaeb9d09fd758de193cd4d5f4d1ec6791eb guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-unsigned.tar.gz 28c81d99a9a4bd6648449393f91db213369e958add579ba9e9a1721540d2c4f7 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64.zip ``` ACKs for top commit: achow101: ACK 7a0b129 hebasto: ACK 7a0b129 jarolrod: ACK 7a0b129 Tree-SHA512: 9e14e98207d20236b833603319fc4bb335c878a7c179ab495b33d143e2a900c6926125536bbb7499ee4f0f676cd5ea45c8c86cd7e544ed9a76bb298f98db6197
fanquake
added a commit
to fanquake/bitcoin
that referenced
this pull request
Aug 7, 2022
With the release of binutils/ld 2.36, ld swapped to much improved default settings when producing windows binaries with mingw-w64. One of these changes was to stop stripping the .reloc section from binaries, which is required for working ASLR. .reloc section stripping is something we've accounted for previously, see bitcoin#18702. The related upstream discussion is in this thread: https://sourceware.org/bugzilla/show_bug.cgi?id=19011. When we switched to using a newer Guix time-machine in bitcoin#23778, we begun using binutils 2.37 to produce releases. Since then, our windows installer (produced with makensis) has not functioned correctly when run on a Windows system with the "Force randomization for images (Mandatory ASLR)" option enabled. Note that all of our other release binaries, which all contain .reloc sections, function fine under the same option, so it cannot be just the presence of a .reloc section that is the issue. For now, restore makensis to it's pre-binutils-2.36 behaviour, which fixes the produced installer. The underlying issue can be further investigated in future. Github-Pull: bitcoin#25788 Rebased-From: 7a0b129
Member
Author
|
Backported in #25799. |
achow101
added a commit
that referenced
this pull request
Aug 9, 2022
…install stubs fc77b2a guix: patch NSIS to remove .reloc sections from install stubs (fanquake) Pull request description: Backport of #25788 to the 23.x branch. Guix Build (x86_64): ```bash 5533c15a0084dfc174b68620a638f5499677be20eafdb1261457f7759298abdc guix-build-fc77b2a41dd9/output/dist-archive/bitcoin-fc77b2a41dd9.tar.gz b67742b17aa813350051635f1e0a9b27921deb22c40d89c8d108fd809619826c guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/SHA256SUMS.part 34e06ab6fbcce2508095d6899daf9c38b962df1a042d0bedee49169f394d47a5 guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/bitcoin-fc77b2a41dd9-win64-debug.zip 63e1e7c1aa62577a21606c789bdaf983352c5285eec5722608cf7a3240a25d6a guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/bitcoin-fc77b2a41dd9-win64-setup-unsigned.exe 0ae26c5bc2f2aa86c451d3cfa3fbdbd73edab6136fd8d6510cd1c47a833973b7 guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/bitcoin-fc77b2a41dd9-win64-unsigned.tar.gz ddb1feb2541d1a4922a62997224e965a3e79c7233c75106eb8c7ad76eff6cf89 guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/bitcoin-fc77b2a41dd9-win64.zip ``` Guix Build (arm64): ```bash 5533c15a0084dfc174b68620a638f5499677be20eafdb1261457f7759298abdc guix-build-fc77b2a41dd9/output/dist-archive/bitcoin-fc77b2a41dd9.tar.gz 3c5cb927e397d14f39bed24480b6800d946854c376d35cfb7a9dd635d45a5d6b guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/SHA256SUMS.part b520c2e9f4c29fe711225c737b4c16e2ae933d5954cfb15cec554e1e0b57b061 guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/bitcoin-fc77b2a41dd9-win64-debug.zip 63e1e7c1aa62577a21606c789bdaf983352c5285eec5722608cf7a3240a25d6a guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/bitcoin-fc77b2a41dd9-win64-setup-unsigned.exe 0ae26c5bc2f2aa86c451d3cfa3fbdbd73edab6136fd8d6510cd1c47a833973b7 guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/bitcoin-fc77b2a41dd9-win64-unsigned.tar.gz 1447a28a582c0e5f2dd03ce22ecf078c6398142b3a2be2de2347a5f095f14d5b guix-build-fc77b2a41dd9/output/x86_64-w64-mingw32/bitcoin-fc77b2a41dd9-win64.zip ``` ACKs for top commit: achow101: ACK fc77b2a hebasto: ACK fc77b2a, `bitcoin-fc77b2a41dd9-win64-setup-unsigned.exe` tested on on Windows 11 Pro 21H2 with the "_Force randomization for images (Mandatory ASLR)_" option being set to "_On by default_". jarolrod: ACK fc77b2a Tree-SHA512: 11000a2236532753a025bfa4ed4dadbbf3432d39b35edcbdd6d09a1b69621331cc43c8fef1bf0ad80909335588535eb741e43ce1f516101c415175e378815579
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
With the release of binutils/ld 2.36, ld swapped to much improved
default settings when producing windows binaries with mingw-w64. One of
these changes was to stop stripping the .reloc section from binaries,
which is required for working ASLR.
When we switched to using a newer Guix time-machine in #23778, we begun
using binutils 2.37 to produce releases. Since then, our windows
installer (produced with makensis) has not functioned correctly when run on
a Windows system with the "Force randomization for images (Mandatory ASLR)"
option enabled. Note that all of our other release binaries, which all
contain .reloc sections, function fine under the same option, so it
cannot be just the presence of a .reloc section that is the issue.
The root cause of the problem is that when we compile NSIS (makensis), a number
of exe installer stubs are produced at the same time, for use later when makensis
is actually run. Given the new linker defaults, the stubs will contain .reloc sections,
when previously they would not. It seems that, in combination with how makensis
mutates the stub when it actually builds the installer, causes the problem.
According to upstream, https://sourceforge.net/p/nsis/bugs/1131/#abb6:
The most recent .reloc related upstream activity is in
https://sourceforge.net/p/nsis/bugs/1283/, where the conclusion again seemed to
be that .relo sections are not wanted, but there hasn't been any further follow up.
For now, restore pre-binutils-2.36 behaviour, by passing
-Wl,--disable-reloc-sectionto the linker when building the installer stubs, which fixes the produced installer.
The underlying issue can be further investigated in future.
.reloc section stripping is something we've accounted for previously,
see #18702, and related upstream discussion is in this thread:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011.
Fixes #25726.
Guix Build (x86_64):
Guix Build (arm64):