From df6bde031b24112abf3a94337a2c096698acde6e Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 4 May 2020 08:19:44 +0800 Subject: 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 : 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 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 : 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 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 ``` --- configure.ac | 33 --------------------------------- 1 file changed, 33 deletions(-) (limited to 'configure.ac') 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 - #if HAVE_SYS_SELECT_H - #include - #endif - ]],[[ - #if HAVE_SYS_SELECT_H - fd_set fds; - FD_ZERO(&fds); - #endif - ]])], - [ AC_MSG_RESULT(no) ], - [ - AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ - #include - #if HAVE_SYS_SELECT_H - #include - #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 #include ] -- cgit v1.2.3