Bitcoin Core crashes under clang-13 libc++ with "Bad system call (core dumped)"

MarcoFalke commented at 10:17 am on October 11, 2021: member

Steps to reproduce:

Install fresh Ubuntu Impish
Compile Bitcoin Core with export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git ./bitcoin-core && cd bitcoin-core && apt install libc++abi-13-dev libc++-13-dev clang llvm gdb build-essential libtool libunwind-dev autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 patch bison -y && cd depends && make CC=clang-13 CXX="clang++-13 -stdlib=libc++" NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 -j $(nproc) && cd .. && ./autogen.sh && CONFIG_SITE="$PWD/depends/x86_64-pc-linux-gnu/share/config.site" ./configure CC=clang-13 CXX="clang++-13 -stdlib=libc++" && make -j $(nproc) src/bitcoind
Run gdb: gdb --args ./src/bitcoind -sandbox=log-and-abort -datadir=/tmp -regtest
run

 0(gdb) run
 1Starting program: /bitcoin-core/src/bitcoind -sandbox=log-and-abort -datadir=/tmp -regtest
 2[Thread debugging using libthread_db enabled]
 3Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
 4
 5Program received signal SIGSYS, Bad system call.
 60x00007ffff7c94cbb in mincore () at ../sysdeps/unix/syscall-template.S:117
 7117	../sysdeps/unix/syscall-template.S: No such file or directory.
 8(gdb) bt
 9[#0](/bitcoin-bitcoin/0/)  0x00007ffff7c94cbb in mincore () at ../sysdeps/unix/syscall-template.S:117
10[#1](/bitcoin-bitcoin/1/)  0x00007ffff7ed6a8a in ?? () from /lib/x86_64-linux-gnu/libunwind.so.8
11[#2](/bitcoin-bitcoin/2/)  0x00007ffff7ed6562 in ?? () from /lib/x86_64-linux-gnu/libunwind.so.8
12[#3](/bitcoin-bitcoin/3/)  0x00007ffff7ed6e1a in ?? () from /lib/x86_64-linux-gnu/libunwind.so.8
13[#4](/bitcoin-bitcoin/4/)  0x00007ffff7ed5dd0 in __libunwind_Unwind_RaiseException () from /lib/x86_64-linux-gnu/libunwind.so.8
14[#5](/bitcoin-bitcoin/5/)  0x00007ffff7ec1405 in __cxa_throw () from /lib/x86_64-linux-gnu/libc++abi.so.1
15[#6](/bitcoin-bitcoin/6/)  0x00007ffff7f3a865 in ?? () from /lib/x86_64-linux-gnu/libc++.so.1
16[#7](/bitcoin-bitcoin/7/)  0x00007ffff7f3a82d in std::__1::__basic_string_common<true>::__throw_out_of_range() const () from /lib/x86_64-linux-gnu/libc++.so.1
17[#8](/bitcoin-bitcoin/8/)  0x00007ffff7f3aa46 in ?? () from /lib/x86_64-linux-gnu/libc++.so.1
18[#9](/bitcoin-bitcoin/9/)  0x00007ffff7f3c124 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::at(unsigned long) () from /lib/x86_64-linux-gnu/libc++.so.1
19[#10](/bitcoin-bitcoin/10/) 0x0000555555946314 in sanity_test_range_fmt() ()
20[#11](/bitcoin-bitcoin/11/) 0x00005555559464ca in glibcxx_sanity_test() ()
21[#12](/bitcoin-bitcoin/12/) 0x00005555558c68df in init::SanityChecks() ()
22[#13](/bitcoin-bitcoin/13/) 0x00005555555a4e11 in AppInitSanityChecks() ()
23[#14](/bitcoin-bitcoin/14/) 0x0000555555590ecf in main ()

There are two issues:

log-and-abort doesn’t log
It crashes

MarcoFalke added the label Bug on Oct 11, 2021

MarcoFalke commented at 10:36 am on October 11, 2021: member

cc @practicalswift

MarcoFalke commented at 10:43 am on October 11, 2021: member

There is also a /usr/bin/ld: warning: libunwind.so.1, needed by /lib/x86_64-linux-gnu/libc++.so.1, may conflict with libunwind.so.8, not sure if important.

MarcoFalke commented at 11:29 am on October 11, 2021: member

If someone is bored and wants to bisect a bit more:

:heavy_check_mark: clang-7
:heavy_check_mark: clang-10
:heavy_check_mark: clang-11
:heavy_check_mark: clang-12 (focal) :x: clang-12 (impish)
:x: clang-13

MarcoFalke added the label Utils/log/libs on Oct 11, 2021

practicalswift commented at 12:10 pm on October 11, 2021: contributor

Thanks for reporting!

Does adding allowed_syscalls.insert(__NR_mincore) to AllowAddressSpaceAccess in SeccompPolicyBuilder solve the issue?

MarcoFalke commented at 1:02 pm on October 11, 2021: member

Yes, the following diff fixes that issue. Is there a reason why the system call name wasn’t printed? Maybe we can remove the logging if it doesn’t work reliably anyway. See also #20487 (review)

 0diff --git a/src/util/syscall_sandbox.cpp b/src/util/syscall_sandbox.cpp
 1index 7e76ecbf3..8128a33f4 100644
 2--- a/src/util/syscall_sandbox.cpp
 3+++ b/src/util/syscall_sandbox.cpp
 4@@ -537,6 +537,7 @@ public:
 5 
 6     void AllowAddressSpaceAccess()
 7     {
 8+           allowed_syscalls.insert(__NR_mincore);
 9         allowed_syscalls.insert(__NR_brk);        // change data segment size
10         allowed_syscalls.insert(__NR_madvise);    // give advice about use of memory
11         allowed_syscalls.insert(__NR_membarrier); // issue memory barriers on a set of threads

practicalswift commented at 1:32 pm on October 11, 2021: contributor

@MarcoFalke

Logging should always work as long as the logging machinery itself does not invoke a disallowed syscall. The logging function itself operates under the sandbox restrictions.

It could be that the logging itself is (indirectly) invoking mincore on this system.

I think you can test this hypothesis by adding mincore as an allowed syscall, and then add a dummy call …

0    // The getgroups syscall is assumed NOT to be allowed by the syscall sandbox policy.
1    std::array<gid_t, 1> groups;
2    [[maybe_unused]] int32_t ignored = getgroups(groups.size(), groups.data());

… at the start of init::SanityChecks() (prior to the syscall violation you reported).

getgroups is guaranteed to be disallowed by the syscall sandbox (it is used for testing the sandbox functionality), so starting bitcoind -regtest -sandbox=log-and-abort should give an error message about a disallowed syscall.

You can test if that works with and without mincore as an allowed syscall to see if my hypothesis holds :)

MarcoFalke commented at 1:34 pm on October 11, 2021: member

Btw, another issue happens on Fedora 35:

Install fresh Fedora 35
Compile Bitcoin Core with dnf install gcc-c++ libtool make autoconf automake python3 clang llvm curl wget htop git vim ccache lbzip2 gdb -y && git clone https://github.com/bitcoin/bitcoin.git ./bitcoin-core && cd bitcoin-core && ( cd depends && make CC=clang CXX=clang++ DEBUG=1 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 -j $(nproc) ) && ./autogen.sh && CONFIG_SITE="$PWD/depends/x86_64-pc-linux-gnu/share/config.site" ./configure CC=clang CXX=clang++ && make -j $(nproc) src/bitcoind`
Run gdb --args ./src/bitcoind -sandbox=log-and-abort -datadir=/tmp -regtest
run

 0(gdb) run
 1Starting program: /bitcoin-core/src/bitcoind -sandbox=log-and-abort -datadir=/tmp -regtest
 2[Thread debugging using libthread_db enabled]
 3Using host libthread_db library "/lib64/libthread_db.so.1".
 42021-10-11T13:30:02Z Bitcoin Core version v22.99.0-1790a8ddacae (release build)
 52021-10-11T13:30:02Z Validating signatures for all blocks.
 62021-10-11T13:30:02Z Setting nMinimumChainWork=0000000000000000000000000000000000000000000000000000000000000000
 72021-10-11T13:30:02Z Experimental syscall sandbox enabled (-sandbox=log-and-abort): bitcoind will terminate if an unexpected (not allowlisted) syscall is invoked.
 82021-10-11T13:30:02Z Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
 92021-10-11T13:30:02Z Using RdSeed as additional entropy source
102021-10-11T13:30:02Z Using RdRand as an additional entropy source
112021-10-11T13:30:02Z No wallet support compiled in!
122021-10-11T13:30:02Z Default data directory /root/.bitcoin
132021-10-11T13:30:02Z Using data directory /tmp/regtest
142021-10-11T13:30:02Z Config file: /tmp/bitcoin.conf (not found, skipping)
152021-10-11T13:30:02Z Command-line arg: datadir="/tmp"
162021-10-11T13:30:02Z Command-line arg: regtest=""
172021-10-11T13:30:02Z Command-line arg: sandbox="log-and-abort"
182021-10-11T13:30:02Z Using at most 125 automatic connections (524288 file descriptors available)
192021-10-11T13:30:02Z Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
202021-10-11T13:30:02Z Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
212021-10-11T13:30:02Z Script verification uses 7 additional threads
22
23Program received signal SIGSYS, Bad system call.
240x00007ffff7bb368d in clone3 () from /lib64/libc.so.6
25Missing separate debuginfos, use: dnf debuginfo-install glibc-2.34-7.fc35.x86_64 libgcc-11.2.1-1.fc35.x86_64 libstdc++-11.2.1-1.fc35.x86_64
26(gdb) bt
27[#0](/bitcoin-bitcoin/0/)  0x00007ffff7bb368d in clone3 () from /lib64/libc.so.6
28[#1](/bitcoin-bitcoin/1/)  0x00007ffff7bb36f1 in __clone_internal () from /lib64/libc.so.6
29[#2](/bitcoin-bitcoin/2/)  0x00007ffff7b2e785 in create_thread () from /lib64/libc.so.6
30[#3](/bitcoin-bitcoin/3/)  0x00007ffff7b2f22b in pthread_create@GLIBC_2.2.5 () from /lib64/libc.so.6
31[#4](/bitcoin-bitcoin/4/)  0x00007ffff7e7af3a in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) () from /lib64/libstdc++.so.6
32[#5](/bitcoin-bitcoin/5/)  0x0000555555946cd7 in void std::__cxx1998::vector<std::thread, std::allocator<std::thread> >::_M_realloc_insert<CCheckQueue<CScriptCheck>::StartWorkerThreads(int)::{lambda()#1}>(__gnu_cxx::__normal_iterator<std::thread*, std::__cxx1998::vector<std::thread, std::allocator<std::thread> > >, CCheckQueue<CScriptCheck>::StartWorkerThreads(int)::{lambda()#1}&&) ()
33[#6](/bitcoin-bitcoin/6/)  0x0000555555946b58 in std::thread& std::__cxx1998::vector<std::thread, std::allocator<std::thread> >::emplace_back<CCheckQueue<CScriptCheck>::StartWorkerThreads(int)::{lambda()#1}>(CCheckQueue<CScriptCheck>::StartWorkerThreads(int)::{lambda()#1}&&) ()
34[#7](/bitcoin-bitcoin/7/)  0x00005555559397d3 in CCheckQueue<CScriptCheck>::StartWorkerThreads(int) ()
35[#8](/bitcoin-bitcoin/8/)  0x00005555559090c3 in StartScriptCheckWorkerThreads(int) ()
36[#9](/bitcoin-bitcoin/9/)  0x00005555555c148c in AppInitMain(NodeContext&, interfaces::BlockAndHeaderTipInfo*) ()
37[#10](/bitcoin-bitcoin/10/) 0x00005555555a80af in main ()

MarcoFalke commented at 1:39 pm on October 11, 2021: member

You can test if that works with and without mincore as an allowed syscall to see if my hypothesis holds :)

Yes, that worked:

0$ ./src/bitcoind -sandbox=log-and-abort -datadir=/tmp -regtest 
1ERROR: The syscall "getgroups" (syscall number 115) is not allowed by the syscall sandbox in thread "init". Please report.
2libc++abi: terminating
3Bad system call (core dumped)

However, I am still wondering if logging can be removed, since it might break sometimes (inevitably) and would simplify maintenance (e.g. #23196 wouldn’t be needed)

practicalswift commented at 1:49 pm on October 11, 2021: contributor

@MarcoFalke

0x00007ffff7bb368d in clone3 () from /lib64/libc.so.6

Sounds like allowed_syscalls.insert(__NR_clone3) is needed in AllowProcessStartOrDeath() :)

Thanks a lot for testing on newer and diverse systems. My testing has been somewhat focused on various Ubuntu based systems (primarily LTS editions).

FWIW from the original PR #20487 (comment): As have been noted in previous discussions above syscall sandboxing is a type of feature that needs experimentation and fine-tuning for different environments to catch potential diversity in syscall use across systems. Such experimentation was required when sandboxing using seccomp-bpf was introduced in projects such as Chromium, OpenSSH and Tor, and it will be required for Bitcoin Core too. There is really no way around it :)

MarcoFalke commented at 1:52 pm on October 11, 2021: member

Fedora can be fixed with:

 0diff --git a/src/util/syscall_sandbox.cpp b/src/util/syscall_sandbox.cpp
 1index 7e76ecbf3..ee1b13142 100644
 2--- a/src/util/syscall_sandbox.cpp
 3+++ b/src/util/syscall_sandbox.cpp
 4@@ -115,6 +115,7 @@ const std::map<uint32_t, std::string> LINUX_SYSCALLS{
 5     {__NR_clock_nanosleep, "clock_nanosleep"},
 6     {__NR_clock_settime, "clock_settime"},
 7     {__NR_clone, "clone"},
 8+    {__NR_clone3, "clone3"},
 9     {__NR_close, "close"},
10     {__NR_connect, "connect"},
11     {__NR_copy_file_range, "copy_file_range"},
12@@ -705,6 +706,7 @@ public:
13     void AllowProcessStartOrDeath()
14     {
15         allowed_syscalls.insert(__NR_clone);      // create a child process
16+        allowed_syscalls.insert(__NR_clone3);      // create a child process
17         allowed_syscalls.insert(__NR_exit);       // terminate the calling process
18         allowed_syscalls.insert(__NR_exit_group); // exit all threads in a process
19         allowed_syscalls.insert(__NR_fork);       // create a child process

practicalswift commented at 1:59 pm on October 11, 2021: contributor

Note that as we add “new” syscalls such as clone3 which may not be available in the user’s (older) headers then these must be added with …

0#ifndef __NR_<syscall>
1#define __NR_<syscall> <syscall_number>
2#endif

… as we’ve done with statx, getrandom, membarrier and other newly introduced syscalls :)

This requirement holds regardless of if we choose to remove the logging functionality or not.

See this comment in syscall_sandbox.cpp:

0// Define system call numbers for x86_64 that are referenced in the system call profile
1// but not provided by the kernel headers used in the GUIX build.

However, I am still wondering if logging can be removed, since it might break sometimes (inevitably) and would simplify maintenance (e.g. #23196 wouldn’t be needed)

Please note the analysis in #23196 (comment): if that holds the we shouldn’t see any more compile-time issues going forward :)

MarcoFalke commented at 2:09 pm on October 11, 2021: member

Ok, see #23255

MarcoFalke closed this on Oct 11, 2021

MarcoFalke deleted a comment on Oct 11, 2021

fanquake referenced this in commit e418a8e675 on Oct 12, 2021

sidhujag referenced this in commit 2da5fde964 on Oct 12, 2021

DrahtBot locked this on Oct 30, 2022

Bitcoin Core crashes under clang-13 libc++ with “Bad system call (core dumped)” #23248