Use FORTIFY_SOURCE=3 when building libevent in depends. I’ve upstreamed a change to switch libevent from using =2 to =3 as well: https://github.com/libevent/libevent/pull/1418.
Solves half of #27038, by giving us some fortified funcs in bitcoin-cli.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | TheCharlatan |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
FORTIFY_SOURCE directly. That gets as what we want at the moment, without other potential side effects.
Do you have a before/after count for hardened functions?
Using GCC 13 and glibc 2.37 (with only this branch):
0# master
1nm -C src/bitcoind | grep _chk
2 U __fprintf_chk@GLIBC_2.17
3 U __memcpy_chk@GLIBC_2.17
4 U __snprintf_chk@GLIBC_2.17
5 U __stack_chk_fail@GLIBC_2.17
6 U __stack_chk_guard@GLIBC_2.17
7 U __vsnprintf_chk@GLIBC_2.17
8objdump -d src/bitcoind | grep "_chk@plt" | wc -l
932
10
11# this branch
12nm -C src/bitcoind | grep _chk
13 U __fdelt_chk@GLIBC_2.17
14 U __fprintf_chk@GLIBC_2.17
15 U __memcpy_chk@GLIBC_2.17
16 U __memmove_chk@GLIBC_2.17
17 U __memset_chk@GLIBC_2.17
18 U __snprintf_chk@GLIBC_2.17
19 U __stack_chk_fail@GLIBC_2.17
20 U __stack_chk_guard@GLIBC_2.17
21 U __vsnprintf_chk@GLIBC_2.17
22objdump -d src/bitcoind | grep "_chk@plt" | wc -l
2354
If I combine with our own use of FORTIFY_SOURCE=3:
0nm -C src/bitcoind | grep _chk
1 U __fdelt_chk@GLIBC_2.17
2 U __fprintf_chk@GLIBC_2.17
3 U __memcpy_chk@GLIBC_2.17
4 U __memmove_chk@GLIBC_2.17
5 U __memset_chk@GLIBC_2.17
6 U __snprintf_chk@GLIBC_2.17
7 U __stack_chk_fail@GLIBC_2.17
8 U __stack_chk_guard@GLIBC_2.17
9 U __vsnprintf_chk@GLIBC_2.17
10objdump -d src/bitcoind | grep "_chk@plt" | wc -l
1181
Changed the approach here, from using libevents own hardening option
The previous 974e44c0a0e692e1e11e7c067699db94f55ce464 commit, being combined with the current 778e34e8625cc83d0e5a93493c71f01712bef81d one, should work for older compilers as well, no?
should work for older compilers as well, no?
I changed the approach because I don’t want us to use the gcc-hardening option.
This PR introduced a regression when building with depends and disabled hardening:
0$ make -C depends HOST=x86_64-w64-mingw32 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1
1$ ./configure --disable-hardening CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site
2$ make
3...
4/usr/bin/x86_64-w64-mingw32-ld: /home/hebasto/git/gui/depends/x86_64-w64-mingw32/lib/libevent.a(evutil.o):evutil.c:(.text+0x2975): undefined reference to `__memcpy_chk'
5/usr/bin/x86_64-w64-mingw32-ld: /home/hebasto/git/gui/depends/x86_64-w64-mingw32/lib/libevent.a(evutil.o):evutil.c:(.text+0x29d6): undefined reference to `__memcpy_chk'
6/usr/bin/x86_64-w64-mingw32-ld: /home/hebasto/git/gui/depends/x86_64-w64-mingw32/lib/libevent.a(evutil.o):evutil.c:(.text+0x3041): undefined reference to `__strcat_chk'
7/usr/bin/x86_64-w64-mingw32-ld: /home/hebasto/git/gui/depends/x86_64-w64-mingw32/lib/libevent.a(evutil.o):evutil.c:(.text+0x3052): undefined reference to `__strcat_chk'
8/usr/bin/x86_64-w64-mingw32-ld: /home/hebasto/git/gui/depends/x86_64-w64-mingw32/lib/libevent.a(http.o):http.c:(.text+0x6e6): undefined reference to `__memcpy_chk'
9...
This PR introduced a regression when building with depends and disabled hardening:
Followup for discussion in 27406.