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
```
This commit is contained in:
fanquake 2020-05-04 08:19:44 +08:00
parent 8bf1540cc2
commit df6bde031b
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
4 changed files with 0 additions and 68 deletions

View file

@ -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>]

View file

@ -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 \

View file

@ -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>();
}

View file

@ -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