aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorfanquake <fanquake@gmail.com>2020-05-04 08:19:44 +0800
committerfanquake <fanquake@gmail.com>2020-05-07 15:45:09 +0800
commitdf6bde031b24112abf3a94337a2c096698acde6e (patch)
tree29357cb3c4e262ab4d188694f764ef5e9535c6b6
parent8bf1540cc235fb8fb5330a7ae8ab638247ceb177 (diff)
test: remove glibc fdelt sanity check
As is, this sanity check doesn't seem to be testing fdelt_chk, because passing a value of "0" to FD_SET wont cause the compiler to insert any calls to fdelt_chk(). The documentation is a little misleading. If we actually triggered fdelt_chk at runtime, bitcoind would abort. I think this check would be better replaced (if possible) by additional checks in security-check.py. The compiler may insert a call to fdelt_warn() (aliased with fdelt_chk in glibc) at compile time if it can determine that an invalid value is being passed to FD_SET. These checks are essentially; value < 0 or value >= FD_SETSIZE along with a check for wether the value is a compile time constant. If the compiler can determine an invalid value is being passed, a call to fdelt_warn will be inserted. Passing 0 should never cause a call to be inserted. You can check this after compiling: ```bash objdump -dC bitcoind | grep sanity_fdelt ... 0000000000399d20 <sanity_test_fdelt()>: 399d20: 48 81 ec 98 00 00 00 sub $0x98,%rsp 399d27: b9 10 00 00 00 mov $0x10,%ecx 399d2c: 64 48 8b 04 25 28 00 mov %fs:0x28,%rax 399d33: 00 00 399d35: 48 89 84 24 88 00 00 mov %rax,0x88(%rsp) 399d3c: 00 399d3d: 31 c0 xor %eax,%eax 399d3f: 48 89 e7 mov %rsp,%rdi 399d42: fc cld 399d43: f3 48 ab rep stos %rax,%es:(%rdi) 399d46: 48 8b 84 24 88 00 00 mov 0x88(%rsp),%rax 399d4d: 00 399d4e: 64 48 33 04 25 28 00 xor %fs:0x28,%rax 399d55: 00 00 399d57: 75 0d jne 399d66 <sanity_test_fdelt()+0x46> 399d59: b8 01 00 00 00 mov $0x1,%eax 399d5e: 48 81 c4 98 00 00 00 add $0x98,%rsp 399d65: c3 retq 399d66: e8 85 df c8 ff callq 27cf0 <__stack_chk_fail@plt> 399d6b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ``` To test, you could modify this test to pass -1 to FD_SET, and check that a call to fdelt_warn() is inserted, and that running bitcoind fails. i.e: ```bash 0000000000399d20 <sanity_test_fdelt()>: 399d20: 48 81 ec 98 00 00 00 sub $0x98,%rsp 399d27: b9 10 00 00 00 mov $0x10,%ecx 399d2c: 64 48 8b 04 25 28 00 mov %fs:0x28,%rax 399d33: 00 00 399d35: 48 89 84 24 88 00 00 mov %rax,0x88(%rsp) 399d3c: 00 399d3d: 31 c0 xor %eax,%eax 399d3f: 48 89 e7 mov %rsp,%rdi 399d42: fc cld 399d43: f3 48 ab rep stos %rax,%es:(%rdi) 399d46: 48 c7 c7 ff ff ff ff mov $0xffffffffffffffff,%rdi 399d4d: e8 3e ff ff ff callq 399c90 <__fdelt_warn> 399d52: 0f b6 04 24 movzbl (%rsp),%eax 399d56: 83 e0 01 and $0x1,%eax 399d59: 48 8b 94 24 88 00 00 mov 0x88(%rsp),%rdx 399d60: 00 399d61: 64 48 33 14 25 28 00 xor %fs:0x28,%rdx 399d68: 00 00 399d6a: 75 08 jne 399d74 <sanity_test_fdelt()+0x54> 399d6c: 48 81 c4 98 00 00 00 add $0x98,%rsp 399d73: c3 retq 399d74: e8 77 df c8 ff callq 27cf0 <__stack_chk_fail@plt> 399d79: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) ``` ```bash ./src/bitcoind *** buffer overflow detected ***: src/bitcoind terminated Aborted ```
-rw-r--r--configure.ac33
-rw-r--r--src/Makefile.am1
-rw-r--r--src/compat/glibc_sanity.cpp8
-rw-r--r--src/compat/glibc_sanity_fdelt.cpp26
4 files changed, 0 insertions, 68 deletions
diff --git a/configure.ac b/configure.ac
index de351899eb..22a3bc1c21 100644
--- a/configure.ac
+++ b/configure.ac
@@ -797,39 +797,6 @@ fi
AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h stdio.h stdlib.h unistd.h strings.h sys/types.h sys/stat.h sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h])
-dnl FD_ZERO may be dependent on a declaration of memcpy, e.g. in SmartOS
-dnl check that it fails to build without memcpy, then that it builds with
-AC_MSG_CHECKING(FD_ZERO memcpy dependence)
-AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
- #include <cstddef>
- #if HAVE_SYS_SELECT_H
- #include <sys/select.h>
- #endif
- ]],[[
- #if HAVE_SYS_SELECT_H
- fd_set fds;
- FD_ZERO(&fds);
- #endif
- ]])],
- [ AC_MSG_RESULT(no) ],
- [
- AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
- #include <cstring>
- #if HAVE_SYS_SELECT_H
- #include <sys/select.h>
- #endif
- ]], [[
- #if HAVE_SYS_SELECT_H
- fd_set fds;
- FD_ZERO(&fds);
- #endif
- ]])],
- [ AC_MSG_RESULT(yes); AC_DEFINE(HAVE_CSTRING_DEPENDENT_FD_ZERO, 1, [Define this symbol if FD_ZERO is dependent of a memcpy declaration being available]) ],
- [ AC_MSG_ERROR(failed with cstring include) ]
- )
- ]
-)
-
AC_CHECK_DECLS([getifaddrs, freeifaddrs],,,
[#include <sys/types.h>
#include <ifaddrs.h>]
diff --git a/src/Makefile.am b/src/Makefile.am
index 627df97cad..13812de0e7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -496,7 +496,6 @@ libbitcoin_util_a_SOURCES = \
support/lockedpool.cpp \
chainparamsbase.cpp \
clientversion.cpp \
- compat/glibc_sanity_fdelt.cpp \
compat/glibc_sanity.cpp \
compat/glibcxx_sanity.cpp \
compat/strnlen.cpp \
diff --git a/src/compat/glibc_sanity.cpp b/src/compat/glibc_sanity.cpp
index cc74f28899..0367b9a53f 100644
--- a/src/compat/glibc_sanity.cpp
+++ b/src/compat/glibc_sanity.cpp
@@ -8,10 +8,6 @@
#include <cstddef>
-#if defined(HAVE_SYS_SELECT_H)
-bool sanity_test_fdelt();
-#endif
-
extern "C" void* memcpy(void* a, const void* b, size_t c);
void* memcpy_int(void* a, const void* b, size_t c)
{
@@ -45,9 +41,5 @@ bool sanity_test_memcpy()
bool glibc_sanity_test()
{
-#if defined(HAVE_SYS_SELECT_H)
- if (!sanity_test_fdelt())
- return false;
-#endif
return sanity_test_memcpy<1025>();
}
diff --git a/src/compat/glibc_sanity_fdelt.cpp b/src/compat/glibc_sanity_fdelt.cpp
deleted file mode 100644
index 87140d0c71..0000000000
--- a/src/compat/glibc_sanity_fdelt.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright (c) 2009-2019 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#if defined(HAVE_CONFIG_H)
-#include <config/bitcoin-config.h>
-#endif
-
-#if defined(HAVE_SYS_SELECT_H)
-#ifdef HAVE_CSTRING_DEPENDENT_FD_ZERO
-#include <cstring>
-#endif
-#include <sys/select.h>
-
-// trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined
-// as >0 and optimizations must be set to at least -O2.
-// test: Add a file descriptor to an empty fd_set. Verify that it has been
-// correctly added.
-bool sanity_test_fdelt()
-{
- fd_set fds;
- FD_ZERO(&fds);
- FD_SET(0, &fds);
- return FD_ISSET(0, &fds);
-}
-#endif