ubsan: misaligned-pointer-use in crc32c/src/crc32c

fanquake commented at 12:14 pm on January 4, 2024: member

master @ 65c05db660b2ca1d0076b0d8573a6760b3228068. Running time FILE_ENV="./ci/test/00_setup_env_native_fuzz.sh" ./ci/test_run_all.sh on Rawhide aarch64.

 0Run coincontrol with args ['/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/coincontrol')]crc32c/src/crc32c_arm64.cc:101:26: runtime error: load of misaligned address 0x52d000000406 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
 10x52d000000406: note: pointer points here
 2 b9 c5 22 00 01 01  1a 6c 65 76 65 6c 64 62  2e 42 79 74 65 77 69 73  65 43 6f 6d 70 61 72 61  74 6f
 3             ^ 
 4    [#0](/bitcoin-bitcoin/0/) 0xaaaab69963e4 in crc32c::ExtendArm64(unsigned int, unsigned char const*, unsigned long) src/crc32c/src/crc32c_arm64.cc:101:26
 5    [#1](/bitcoin-bitcoin/1/) 0xaaaab68cd024 in leveldb::crc32c::Value(char const*, unsigned long) src/./leveldb/util/crc32c.h:20:60
 6    [#2](/bitcoin-bitcoin/2/) 0xaaaab68cd024 in leveldb::log::Reader::ReadPhysicalRecord(leveldb::Slice*) src/leveldb/db/log_reader.cc:246:29
 7    [#3](/bitcoin-bitcoin/3/) 0xaaaab68cb594 in leveldb::log::Reader::ReadRecord(leveldb::Slice*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) src/leveldb/db/log_reader.cc:72:38
 8    [#4](/bitcoin-bitcoin/4/) 0xaaaab68fb1b0 in leveldb::VersionSet::Recover(bool*) src/leveldb/db/version_set.cc:910:19
 9    [#5](/bitcoin-bitcoin/5/) 0xaaaab6878208 in leveldb::DBImpl::Recover(leveldb::VersionEdit*, bool*) src/leveldb/db/db_impl.cc:320:18
10    [#6](/bitcoin-bitcoin/6/) 0xaaaab68a9ec8 in leveldb::DB::Open(leveldb::Options const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, leveldb::DB**) src/leveldb/db/db_impl.cc:1484:20
11    [#7](/bitcoin-bitcoin/7/) 0xaaaab4ca3858 in CDBWrapper::CDBWrapper(DBParams const&) src/dbwrapper.cpp:247:30
12    [#8](/bitcoin-bitcoin/8/) 0xaaaab4b53fb0 in kernel::BlockTreeDB::BlockTreeDB(DBParams const&) src/./node/blockstorage.h:53:23
13    [#9](/bitcoin-bitcoin/9/) 0xaaaab4b53fb0 in std::__detail::_MakeUniq<kernel::BlockTreeDB>::__single_object std::make_unique<kernel::BlockTreeDB, DBParams>(DBParams&&) /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1070:34
14    [#10](/bitcoin-bitcoin/10/) 0xaaaab4b53fb0 in ChainTestingSetup::ChainTestingSetup(ChainType, std::vector<char const*, std::allocator<char const*>> const&) src/test/util/setup_common.cpp:198:51
15    [#11](/bitcoin-bitcoin/11/) 0xaaaab4b5932c in TestingSetup::TestingSetup(ChainType, std::vector<char const*, std::allocator<char const*>> const&, bool, bool) src/test/util/setup_common.cpp:250:7
16    [#12](/bitcoin-bitcoin/12/) 0xaaaab44ae3b8 in std::__detail::_MakeUniq<TestingSetup const>::__single_object std::make_unique<TestingSetup const, ChainType const&, std::vector<char const*, std::allocator<char const*>> const&>(ChainType const&, std::vector<char const*, std::allocator<char const*>> const&) /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1070:34
17    [#13](/bitcoin-bitcoin/13/) 0xaaaab44ae3b8 in std::unique_ptr<TestingSetup const, std::default_delete<TestingSetup const>> MakeNoLogFileContext<TestingSetup const>(ChainType, std::vector<char const*, std::allocator<char const*>> const&) src/./test/util/setup_common.h:225:12
18    [#14](/bitcoin-bitcoin/14/) 0xaaaab44a44e8 in wallet::(anonymous namespace)::initialize_coincontrol() src/wallet/test/fuzz/coincontrol.cpp:19:39
19    [#15](/bitcoin-bitcoin/15/) 0xaaaab4ba0f94 in std::function<void ()>::operator()() const /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
20    [#16](/bitcoin-bitcoin/16/) 0xaaaab4ba0f94 in initialize() src/test/fuzz/fuzz.cpp:130:5
21    [#17](/bitcoin-bitcoin/17/) 0xaaaab4ba2fc4 in LLVMFuzzerInitialize src/test/fuzz/fuzz.cpp:186:5
22    [#18](/bitcoin-bitcoin/18/) 0xaaaab43962d4 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x19862d4) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
23    [#19](/bitcoin-bitcoin/19/) 0xaaaab43c0cc8 in main (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x19b0cc8) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
24    [#20](/bitcoin-bitcoin/20/) 0xffffaab57580  (/lib/aarch64-linux-gnu/libc.so.6+0x27580) (BuildId: ee37e53ec1958e9bebf5b3d5f81a3039cf0a1851)
25    [#21](/bitcoin-bitcoin/21/) 0xffffaab57654 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x27654) (BuildId: ee37e53ec1958e9bebf5b3d5f81a3039cf0a1851)
26    [#22](/bitcoin-bitcoin/22/) 0xaaaab438e26c in _start (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x197e26c) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
27
28SUMMARY: UndefinedBehaviorSanitizer: misaligned-pointer-use crc32c/src/crc32c_arm64.cc:101:26 in 
29
30crc32c/src/crc32c_arm64.cc:101:26: runtime error: load of misaligned address 0x52d000000406 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
310x52d000000406: note: pointer points here
32 b9 c5 22 00 01 01  1a 6c 65 76 65 6c 64 62  2e 42 79 74 65 77 69 73  65 43 6f 6d 70 61 72 61  74 6f
33             ^ 
34    [#0](/bitcoin-bitcoin/0/) 0xaaaab69963e4 in crc32c::ExtendArm64(unsigned int, unsigned char const*, unsigned long) src/crc32c/src/crc32c_arm64.cc:101:26
35    [#1](/bitcoin-bitcoin/1/) 0xaaaab68cd024 in leveldb::crc32c::Value(char const*, unsigned long) src/./leveldb/util/crc32c.h:20:60
36    [#2](/bitcoin-bitcoin/2/) 0xaaaab68cd024 in leveldb::log::Reader::ReadPhysicalRecord(leveldb::Slice*) src/leveldb/db/log_reader.cc:246:29
37    [#3](/bitcoin-bitcoin/3/) 0xaaaab68cb594 in leveldb::log::Reader::ReadRecord(leveldb::Slice*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) src/leveldb/db/log_reader.cc:72:38
38    [#4](/bitcoin-bitcoin/4/) 0xaaaab68fb1b0 in leveldb::VersionSet::Recover(bool*) src/leveldb/db/version_set.cc:910:19
39    [#5](/bitcoin-bitcoin/5/) 0xaaaab6878208 in leveldb::DBImpl::Recover(leveldb::VersionEdit*, bool*) src/leveldb/db/db_impl.cc:320:18
40    [#6](/bitcoin-bitcoin/6/) 0xaaaab68a9ec8 in leveldb::DB::Open(leveldb::Options const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, leveldb::DB**) src/leveldb/db/db_impl.cc:1484:20
41    [#7](/bitcoin-bitcoin/7/) 0xaaaab4ca3858 in CDBWrapper::CDBWrapper(DBParams const&) src/dbwrapper.cpp:247:30
42    [#8](/bitcoin-bitcoin/8/) 0xaaaab4b53fb0 in kernel::BlockTreeDB::BlockTreeDB(DBParams const&) src/./node/blockstorage.h:53:23
43    [#9](/bitcoin-bitcoin/9/) 0xaaaab4b53fb0 in std::__detail::_MakeUniq<kernel::BlockTreeDB>::__single_object std::make_unique<kernel::BlockTreeDB, DBParams>(DBParams&&) /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1070:34
44    [#10](/bitcoin-bitcoin/10/) 0xaaaab4b53fb0 in ChainTestingSetup::ChainTestingSetup(ChainType, std::vector<char const*, std::allocator<char const*>> const&) src/test/util/setup_common.cpp:198:51
45    [#11](/bitcoin-bitcoin/11/) 0xaaaab4b5932c in TestingSetup::TestingSetup(ChainType, std::vector<char const*, std::allocator<char const*>> const&, bool, bool) src/test/util/setup_common.cpp:250:7
46    [#12](/bitcoin-bitcoin/12/) 0xaaaab44ae3b8 in std::__detail::_MakeUniq<TestingSetup const>::__single_object std::make_unique<TestingSetup const, ChainType const&, std::vector<char const*, std::allocator<char const*>> const&>(ChainType const&, std::vector<char const*, std::allocator<char const*>> const&) /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1070:34
47    [#13](/bitcoin-bitcoin/13/) 0xaaaab44ae3b8 in std::unique_ptr<TestingSetup const, std::default_delete<TestingSetup const>> MakeNoLogFileContext<TestingSetup const>(ChainType, std::vector<char const*, std::allocator<char const*>> const&) src/./test/util/setup_common.h:225:12
48    [#14](/bitcoin-bitcoin/14/) 0xaaaab44a44e8 in wallet::(anonymous namespace)::initialize_coincontrol() src/wallet/test/fuzz/coincontrol.cpp:19:39
49    [#15](/bitcoin-bitcoin/15/) 0xaaaab4ba0f94 in std::function<void ()>::operator()() const /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
50    [#16](/bitcoin-bitcoin/16/) 0xaaaab4ba0f94 in initialize() src/test/fuzz/fuzz.cpp:130:5
51    [#17](/bitcoin-bitcoin/17/) 0xaaaab4ba2fc4 in LLVMFuzzerInitialize src/test/fuzz/fuzz.cpp:186:5
52    [#18](/bitcoin-bitcoin/18/) 0xaaaab43962d4 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x19862d4) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
53    [#19](/bitcoin-bitcoin/19/) 0xaaaab43c0cc8 in main (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x19b0cc8) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
54    [#20](/bitcoin-bitcoin/20/) 0xffffaab57580  (/lib/aarch64-linux-gnu/libc.so.6+0x27580) (BuildId: ee37e53ec1958e9bebf5b3d5f81a3039cf0a1851)
55    [#21](/bitcoin-bitcoin/21/) 0xffffaab57654 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x27654) (BuildId: ee37e53ec1958e9bebf5b3d5f81a3039cf0a1851)
56    [#22](/bitcoin-bitcoin/22/) 0xaaaab438e26c in _start (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x197e26c) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
57
58SUMMARY: UndefinedBehaviorSanitizer: misaligned-pointer-use crc32c/src/crc32c_arm64.cc:101:26 in 
59
60Target ['/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/coincontrol')] failed with exit code 1

CPU Info:

0cat /proc/cpuinfo
1processor	: 0
2BogoMIPS	: 50.00
3Features	: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm lrcpc dcpop asimddp ssbs
4CPU implementer	: 0x41
5CPU architecture: 8
6CPU variant	: 0x3
7CPU part	: 0xd0c
8CPU revision	: 1

maflcko commented at 2:23 pm on January 4, 2024: member

Same with ./ci/test/00_setup_env_native_asan.sh.

I somehow assumed that this was fixed by:

maflcko commented at 3:02 pm on January 4, 2024: member

I re-tried commit 7b45d171f549595a831489827c28e8493f36c00c and at least Asan is passing. Fuzz and Tsan are failing for different reasons.

hebasto commented at 4:29 pm on January 4, 2024: member

Should we use the -mstrict-align option?

maflcko commented at 9:48 am on January 5, 2024: member

 0# git bisect bad
 1228d6a2969e4fcee573c9df7aad31550eab9c8d4 is the first bad commit
 2commit 228d6a2969e4fcee573c9df7aad31550eab9c8d4
 3Author: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
 4Date:   Mon Nov 20 13:37:44 2023 +0000
 5
 6    build: Fix regression in "ARMv8 CRC32 intrinsics" test
 7    
 8    The `vmull_p64` is a part of the Crypto extensions from the ACLE. They
 9    are optional extensions, so they get enabled with a `+crypto` for
10    architecture flags.
11
12 configure.ac | 2 +-
13 1 file changed, 1 insertion(+), 1 deletion(-)

hebasto commented at 5:29 pm on January 14, 2024: member

I can reproduce the issue on Debian 11, aarch64.

hebasto referenced this in commit 32197d151e on Jan 14, 2024

hebasto commented at 8:28 pm on January 14, 2024: member

A possible solution suggested in https://github.com/bitcoin-core/crc32c-subtree/pull/6.

theuni commented at 4:57 pm on January 16, 2024: member

FWIW @hebasto’s solution looks correct (and necessary) to me.

An alternative would be to use c++20’s std::assume_aligned if we could guarantee the input’s alignment, but looking at the usage that’s definitely not the case.

fanquake commented at 5:12 pm on January 16, 2024: member

@hebasto Can you update https://github.com/bitcoin-core/crc32c-subtree/pull/6 so that the commit includes a description of the problem, an explanation of the fix, and any further information. We should probably add a comment inline explaining why a memcpy is being added to that code.

theuni commented at 5:23 pm on January 16, 2024: member

It’d be worth taking upstream as well.

fanquake commented at 5:27 pm on January 16, 2024: member

It’d be worth taking upstream as well.

We could open a PR, but I’m not sure if we’ll get any traction there. The last change upstream https://github.com/google/crc32c was ~ 3 years ago, and the readme recommends the use of https://github.com/google/highwayhash, which is actively maintained.

hebasto referenced this in commit 43180d73db on Jan 20, 2024

hebasto commented at 12:33 pm on January 20, 2024: member

Can you update bitcoin-core/crc32c-subtree#6 so that the commit includes a description of the problem, an explanation of the fix, and any further information. We should probably add a comment inline explaining why a memcpy is being added to that code.

Thanks! Done.

hebasto referenced this in commit 4d32620158 on Feb 14, 2024

hebasto referenced this in commit db7b18359a on Feb 16, 2024

hebasto referenced this in commit 6b61672966 on Feb 16, 2024

hebasto referenced this in commit 4b5d72a0d7 on Feb 16, 2024

hebasto referenced this in commit 70a8d491cc on Feb 16, 2024

hebasto referenced this in commit 68ab55af2e on Feb 16, 2024

hebasto referenced this in commit 1ac401e32b on Feb 16, 2024

fanquake referenced this in commit b60d2b7334 on Feb 27, 2024

fanquake closed this on Feb 28, 2024

fanquake referenced this in commit d752831e64 on Feb 28, 2024

ubsan: misaligned-pointer-use in crc32c/src/crc32c_arm64.cc #29178