Skip to content

Conversation

practicalswift
Copy link
Contributor

Add initialization order fiasco detection in Travis :)

Context: #17670 (comment)

This would have caught the events_hasher initialization order issue introduced in #17573 and fixed in #17670.

Output in case of an initialization order fiasco:

==7934==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x557098d79200 at pc 0x55709796b9a3 bp 0x7ffde524dc30 sp 0x7ffde524dc28
READ of size 8 at 0x557098d79200 thread T0
    #0 0x55709796b9a2 in CSHA256::Finalize(unsigned char*) src/crypto/sha256.cpp:667:25
    #1 0x5570978150e9 in SeedEvents(CSHA512&) src/random.cpp:462:19
    #2 0x5570978145e1 in SeedSlow(CSHA512&) src/random.cpp:482:5
    #3 0x5570978149a3 in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) src/random.cpp:527:5
    #4 0x55709781102d in ProcRand(unsigned char*, int, RNGLevel) src/random.cpp:571:9
    #5 0x557097810d19 in GetRandBytes(unsigned char*, int) src/random.cpp:576:59
    #6 0x557096c2f9d5 in (anonymous namespace)::CSignatureCache::CSignatureCache() src/script/sigcache.cpp:34:9
    #7 0x557096511977 in __cxx_global_var_init.7 src/script/sigcache.cpp:67:24
    #8 0x5570965119f8 in _GLOBAL__sub_I_sigcache.cpp src/script/sigcache.cpp
    #9 0x557097bba4ac in __libc_csu_init (src/bitcoind+0x18554ac)
    #10 0x7f214b1c2b27 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:266
    #11 0x5570965347d9 in _start (src/bitcoind+0x1cf7d9)

0x557098d79200 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x557098d791a0) of size 104
  registered at:
    #0 0x557096545dfd in __asan_register_globals compiler-rt/lib/asan/asan_globals.cpp:360:3
    #1 0x557097817f8b in asan.module_ctor (src/bitcoind+0x14b2f8b)

SUMMARY: AddressSanitizer: initialization-order-fiasco src/crypto/sha256.cpp:667:25 in CSHA256::Finalize(unsigned char*)

@laanwj laanwj added the Tests label Dec 5, 2019
@practicalswift
Copy link
Contributor Author

Fails as expected for the two ASAN jobs in Travis:

make[3]: *** [test/amount_tests.cpp.test] Error 1
Makefile:14006: recipe for target 'test/amount_tests.cpp.test' failed
=================================================================
==25881==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x55e6e8811540 at pc 0x55e6e3f3e333 bp 0x7fff60848af0 sp 0x7fff60848ae8
READ of size 8 at 0x55e6e8811540 thread T0
    #0 0x55e6e3f3e332 in CSHA256::Finalize(unsigned char*) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:667:25
    #1 0x55e6e3e63905 in SeedEvents(CSHA512&) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/random.cpp:462:19
    #2 0x55e6e3e62dd3 in SeedSlow(CSHA512&) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/random.cpp:482:5
    #3 0x55e6e3e634cf in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/random.cpp:527:5
    #4 0x55e6e3e5fddc in ProcRand(unsigned char*, int, RNGLevel) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/random.cpp:571:9
    #5 0x55e6e3e5fba1 in GetRandBytes(unsigned char*, int) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/random.cpp:576:59
    #6 0x55e6e2682563 in (anonymous namespace)::CSignatureCache::CSignatureCache() /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/script/sigcache.cpp:34:9
    #7 0x55e6e2682563 in __cxx_global_var_init.11 /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/script/sigcache.cpp:67
    #8 0x55e6e2682563 in _GLOBAL__sub_I_sigcache.cpp /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/script/sigcache.cpp
    #9 0x55e6e44a2e1c in __libc_csu_init (/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/test/test_bitcoin+0x4911e1c)
    #10 0x7fe2b094eb27 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b27)
    #11 0x55e6e2698bd9 in _start (/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/test/test_bitcoin+0x2b07bd9)
0x55e6e8811540 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x55e6e88114e0) of size 104
  registered at:
    #0 0x55e6e26aa5de in __asan_register_globals (/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/test/test_bitcoin+0x2b195de)
    #1 0x55e6e3e652bb in asan.module_ctor (/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/test/test_bitcoin+0x42d42bb)

@laanwj
Copy link
Member

laanwj commented Dec 5, 2019

Concept ACK!

Does this pass with #17670 or are there other initialization order fiasco in the code?

@practicalswift
Copy link
Contributor Author

practicalswift commented Dec 5, 2019

@laanwj Travis passes with #17670, so this is the only fiasco in our code :)

@promag
Copy link
Contributor

promag commented Dec 5, 2019

Tested ACK 1f9d5af, got

==3770==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x0001058f70a0 at pc 0x000105542404 bp 0x7ffeeb534ef0 sp 0x7ffeeb534ee8
READ of size 8 at 0x0001058f70a0 thread T0
    #0 0x105542403 in CSHA256::Finalize(unsigned char*) sha256.cpp:667
    #1 0x105433ca0 in SeedEvents(CSHA512&) random.cpp:462
    #2 0x1054300f6 in ProcRand(unsigned char*, int, RNGLevel) random.cpp:482
    #3 0x10542f99a in GetRandBytes(unsigned char*, int) random.cpp:576
    #4 0x104bfcce5 in _GLOBAL__sub_I_sigcache.cpp sigcache.cpp:34

Probably rebase with #17670 to have CI green.

@maflcko
Copy link
Member

maflcko commented Dec 5, 2019

ACK 1f9d5af 👔

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 1f9d5af4f197e7cc0469a0bb25dcbc51dfa537f4 👔
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUggBgv/WX2CjAhXVFCzZxEXZNk/ypMrUPN0Jg6iclVN5cpR2G7AVGTdiJUCYxCR
TDky/yYSl/soc+Zd8odXNOPIJAP6azPWmD2a7J8ttMB4iCmgQvPQxXarIEKQrC7b
cOU+yNTOe2tNkooir9ZoP6ay8z0GFxa1tsflRipGWeJEZqiLMM1nJo6e9qvfF7lN
MldOOAHo07B4+nmi/7w0Pj6GYfxF9OT17HHkhMiFjaYc+ezBaAbyNRTT/iH7L9dv
7s3d+2Qp2DLE0YvBuofAkNUkeVtg/AU6iKYnUzkNWCxsrhmpmfYG/YLb+wcxYwRI
FwvMNqPQMB+e6E55njqpIkWwsnvhK68ewgAmFTVFJ2mDDmg0mGQV8KEX9Lorjjm3
ZdJD+3DxAn8VfhTQQFS5iS1LTkXPFkZ8y/GHUwX0GBuYNKClpFSGyZe+mKHZK3fU
YkRY7+eVhw6Eg7QkFud9xZ7/IEp8SZWtvfNvBWHAuJGVK/zyzLI+j3bgRUQiXqM+
z2r0XTcX
=3Nui
-----END PGP SIGNATURE-----

Timestamp of file with hash 97c46147814b07ad0e4976949b09e8d622a974ee6005483703d819bc30b4ccb4 -

maflcko pushed a commit that referenced this pull request Dec 5, 2019
1f9d5af tests: Add initialization order fiasco detection in Travis (practicalswift)

Pull request description:

  Add initialization order fiasco detection in Travis :)

  Context: #17670 (comment)

  This would have caught the `events_hasher` initialization order issue introduced in #17573  and fixed in #17670.

  Output in case of an initialization order fiasco:

  ```
  ==7934==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x557098d79200 at pc 0x55709796b9a3 bp 0x7ffde524dc30 sp 0x7ffde524dc28
  READ of size 8 at 0x557098d79200 thread T0
      #0 0x55709796b9a2 in CSHA256::Finalize(unsigned char*) src/crypto/sha256.cpp:667:25
      #1 0x5570978150e9 in SeedEvents(CSHA512&) src/random.cpp:462:19
      #2 0x5570978145e1 in SeedSlow(CSHA512&) src/random.cpp:482:5
      #3 0x5570978149a3 in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) src/random.cpp:527:5
      #4 0x55709781102d in ProcRand(unsigned char*, int, RNGLevel) src/random.cpp:571:9
      #5 0x557097810d19 in GetRandBytes(unsigned char*, int) src/random.cpp:576:59
      #6 0x557096c2f9d5 in (anonymous namespace)::CSignatureCache::CSignatureCache() src/script/sigcache.cpp:34:9
      #7 0x557096511977 in __cxx_global_var_init.7 src/script/sigcache.cpp:67:24
      #8 0x5570965119f8 in _GLOBAL__sub_I_sigcache.cpp src/script/sigcache.cpp
      #9 0x557097bba4ac in __libc_csu_init (src/bitcoind+0x18554ac)
      #10 0x7f214b1c2b27 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:266
      #11 0x5570965347d9 in _start (src/bitcoind+0x1cf7d9)

  0x557098d79200 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x557098d791a0) of size 104
    registered at:
      #0 0x557096545dfd in __asan_register_globals compiler-rt/lib/asan/asan_globals.cpp:360:3
      #1 0x557097817f8b in asan.module_ctor (src/bitcoind+0x14b2f8b)

  SUMMARY: AddressSanitizer: initialization-order-fiasco src/crypto/sha256.cpp:667:25 in CSHA256::Finalize(unsigned char*)
  ```

ACKs for top commit:
  promag:
    Tested ACK 1f9d5af, got
  MarcoFalke:
    ACK 1f9d5af 👔

Tree-SHA512: f24ac0a313df7549193bd7f4fcfdf9b72bdfc6a6ee31d0b08e6d0752e5108fbd532106b6c86377ae0641258c9adb4921872e5d9a0154c0284e03315e0777102c
@maflcko maflcko merged commit 1f9d5af into bitcoin:master Dec 5, 2019
maflcko pushed a commit that referenced this pull request Jul 2, 2020
…se of uninitialized memory

870f0cd build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory (practicalswift)

Pull request description:

  Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory.

  First UBSan, then ASan followed by TSan... and now: yes, the wait is over -- **MSan is finally here!** :)

  Some historical context:
  * 2017: Continuous compilation with Clang Thread Safety analysis enabled (#10866, #10923)
  * 2018: Continuous testing with trapping on signed integer overflows (`-ftrapv`) (#12686)
  * 2018: Continuous testing of use of locale dependent functions (#13041)
  * 2018: Continuous testing of format strings (#13705)
  * 2018: Continuous compilation with MSVC `TreatWarningAsError` (#14151)
  * 2018: Continuous testing under UndefinedBehaviorSanitizer – UBSan (#14252, #14673, #17006)
  * 2018: Continuous testing under AddressSanitizer – ASan (#14794, #17205, #17674)
  * 2018: Continuous testing under ThreadSanitizer – TSan (#14829)
  * 2019: Continuous testing in an unsigned char environment (`-funsigned-char`) (#15134)
  * 2019: Continuous compile-time testing of assumptions we're making (#15391)
  * 2019: Continuous testing of fuzz test cases under Valgrind (#17633, #18159, #18166)
  * 2020: Finally... MemorySanitizer – MSAN! :)

  What is the next step? What tools should we add to CI to keep bugs from entering `master`? :)

ACKs for top commit:
  MarcoFalke:
    ACK 870f0cd

Tree-SHA512: 38327c8b75679d97d469fe42e704cacd1217447a5a603701dd8a58ee50b3be2c10248f8d68a479ed081c0c4b254589d3081c9183f991640b06ef689061f75578
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 28, 2020
…Travis

Summary:
1f9d5af4f197e7cc0469a0bb25dcbc51dfa537f4 tests: Add initialization order fiasco detection in Travis (practicalswift)

Pull request description:

  Add initialization order fiasco detection in Travis :)

  Context: bitcoin/bitcoin#17670 (comment)

  This would have caught the `events_hasher` initialization order issue introduced in #17573  and fixed in #17670.

  Output in case of an initialization order fiasco:

  ```
  ==7934==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x557098d79200 at pc 0x55709796b9a3 bp 0x7ffde524dc30 sp 0x7ffde524dc28
  READ of size 8 at 0x557098d79200 thread T0
      #0 0x55709796b9a2 in CSHA256::Finalize(unsigned char*) src/crypto/sha256.cpp:667:25
      #1 0x5570978150e9 in SeedEvents(CSHA512&) src/random.cpp:462:19
      #2 0x5570978145e1 in SeedSlow(CSHA512&) src/random.cpp:482:5
      #3 0x5570978149a3 in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) src/random.cpp:527:5
      #4 0x55709781102d in ProcRand(unsigned char*, int, RNGLevel) src/random.cpp:571:9
      #5 0x557097810d19 in GetRandBytes(unsigned char*, int) src/random.cpp:576:59
      #6 0x557096c2f9d5 in (anonymous namespace)::CSignatureCache::CSignatureCache() src/script/sigcache.cpp:34:9
      #7 0x557096511977 in __cxx_global_var_init.7 src/script/sigcache.cpp:67:24
      #8 0x5570965119f8 in _GLOBAL__sub_I_sigcache.cpp src/script/sigcache.cpp
      #9 0x557097bba4ac in __libc_csu_init (src/bitcoind+0x18554ac)
      #10 0x7f214b1c2b27 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:266
      #11 0x5570965347d9 in _start (src/bitcoind+0x1cf7d9)

  0x557098d79200 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x557098d791a0) of size 104
    registered at:
      #0 0x557096545dfd in __asan_register_globals compiler-rt/lib/asan/asan_globals.cpp:360:3
      #1 0x557097817f8b in asan.module_ctor (src/bitcoind+0x14b2f8b)

  SUMMARY: AddressSanitizer: initialization-order-fiasco src/crypto/sha256.cpp:667:25 in CSHA256::Finalize(unsigned char*)
  ```

---

Backport of Core [[bitcoin/bitcoin#17674 | PR17674]]

Depends on D8756

Test Plan:
  cmake -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DENABLE_SANITIZERS=address
  ninja all check check-functional

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8757
deadalnix pushed a commit to Bitcoin-ABC/secp256k1 that referenced this pull request Dec 29, 2020
…Travis

Summary:
1f9d5af4f197e7cc0469a0bb25dcbc51dfa537f4 tests: Add initialization order fiasco detection in Travis (practicalswift)

Pull request description:

  Add initialization order fiasco detection in Travis :)

  Context: bitcoin/bitcoin#17670 (comment)

  This would have caught the `events_hasher` initialization order issue introduced in #17573  and fixed in #17670.

  Output in case of an initialization order fiasco:

  ```
  ==7934==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x557098d79200 at pc 0x55709796b9a3 bp 0x7ffde524dc30 sp 0x7ffde524dc28
  READ of size 8 at 0x557098d79200 thread T0
      #0 0x55709796b9a2 in CSHA256::Finalize(unsigned char*) src/crypto/sha256.cpp:667:25
      #1 0x5570978150e9 in SeedEvents(CSHA512&) src/random.cpp:462:19
      #2 0x5570978145e1 in SeedSlow(CSHA512&) src/random.cpp:482:5
      #3 0x5570978149a3 in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) src/random.cpp:527:5
      #4 0x55709781102d in ProcRand(unsigned char*, int, RNGLevel) src/random.cpp:571:9
      #5 0x557097810d19 in GetRandBytes(unsigned char*, int) src/random.cpp:576:59
      #6 0x557096c2f9d5 in (anonymous namespace)::CSignatureCache::CSignatureCache() src/script/sigcache.cpp:34:9
      #7 0x557096511977 in __cxx_global_var_init.7 src/script/sigcache.cpp:67:24
      #8 0x5570965119f8 in _GLOBAL__sub_I_sigcache.cpp src/script/sigcache.cpp
      #9 0x557097bba4ac in __libc_csu_init (src/bitcoind+0x18554ac)
      #10 0x7f214b1c2b27 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:266
      #11 0x5570965347d9 in _start (src/bitcoind+0x1cf7d9)

  0x557098d79200 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x557098d791a0) of size 104
    registered at:
      #0 0x557096545dfd in __asan_register_globals compiler-rt/lib/asan/asan_globals.cpp:360:3
      #1 0x557097817f8b in asan.module_ctor (src/bitcoind+0x14b2f8b)

  SUMMARY: AddressSanitizer: initialization-order-fiasco src/crypto/sha256.cpp:667:25 in CSHA256::Finalize(unsigned char*)
  ```

---

Backport of Core [[bitcoin/bitcoin#17674 | PR17674]]

Depends on D8756

Test Plan:
  cmake -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DENABLE_SANITIZERS=address
  ninja all check check-functional

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8757
@practicalswift practicalswift deleted the initialization-order-fiasco branch April 10, 2021 19:39
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Oct 4, 2021
…etect use of uninitialized memory

870f0cd build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory (practicalswift)

Pull request description:

  Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory.

  First UBSan, then ASan followed by TSan... and now: yes, the wait is over -- **MSan is finally here!** :)

  Some historical context:
  * 2017: Continuous compilation with Clang Thread Safety analysis enabled (bitcoin#10866, bitcoin#10923)
  * 2018: Continuous testing with trapping on signed integer overflows (`-ftrapv`) (bitcoin#12686)
  * 2018: Continuous testing of use of locale dependent functions (bitcoin#13041)
  * 2018: Continuous testing of format strings (bitcoin#13705)
  * 2018: Continuous compilation with MSVC `TreatWarningAsError` (bitcoin#14151)
  * 2018: Continuous testing under UndefinedBehaviorSanitizer – UBSan (bitcoin#14252, bitcoin#14673, bitcoin#17006)
  * 2018: Continuous testing under AddressSanitizer – ASan (bitcoin#14794, bitcoin#17205, bitcoin#17674)
  * 2018: Continuous testing under ThreadSanitizer – TSan (bitcoin#14829)
  * 2019: Continuous testing in an unsigned char environment (`-funsigned-char`) (bitcoin#15134)
  * 2019: Continuous compile-time testing of assumptions we're making (bitcoin#15391)
  * 2019: Continuous testing of fuzz test cases under Valgrind (bitcoin#17633, bitcoin#18159, bitcoin#18166)
  * 2020: Finally... MemorySanitizer – MSAN! :)

  What is the next step? What tools should we add to CI to keep bugs from entering `master`? :)

ACKs for top commit:
  MarcoFalke:
    ACK 870f0cd

Tree-SHA512: 38327c8b75679d97d469fe42e704cacd1217447a5a603701dd8a58ee50b3be2c10248f8d68a479ed081c0c4b254589d3081c9183f991640b06ef689061f75578
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants