Repository: kudu Updated Branches: refs/heads/master 3b2df01c0 -> e9448dac5
KUDU-2328. Fix crash at startup with OpenSSL FIPS mode The wrapping of libdl functions added by commit d9e7037138646e3efb331af98c6982de13294c4b has a problem if any other dynamic initializer calls dlopen or dlclose. It turns out that OpenSSL in FIPS mode does indeed do that, leading to a crash with a stack like: #0 0x0000000000000000 in ?? () #1 0x0000000001b45d23 in dlopen () #2 0x00007f1f444967ba in ?? () from /lib64/libcrypto.so.1.0.0 #3 0x00007f1f44496857 in ?? () from /lib64/libcrypto.so.1.0.0 #4 0x00007f1f44496bfe in FIPS_module_mode_set () from /lib64/libcrypto.so.1.0.0 #5 0x00007f1f4437216c in FIPS_mode_set () from /lib64/libcrypto.so.1.0.0 #6 0x00007f1f4436eb60 in OPENSSL_init_library () from /lib64/libcrypto.so.1.0.0 #7 0x00007f1f450a2c0a in call_init.part () from /lib64/ld-linux-x86-64.so.2 #8 0x00007f1f450a2cf3 in _dl_init () from /lib64/ld-linux-x86-64.so.2 #9 0x00007f1f4509518a in _dl_start_user () from /lib64/ld-linux-x86-64.so.2 The fix takes the same approach we already used to workaround a similar issue with the ASAN runtime, but generalizes it to all of our wrapped functions. Change-Id: I10a04126411f51b4d8e290a6b061aa585aad0769 Reviewed-on: http://gerrit.cloudera.org:8080/9460 Tested-by: Todd Lipcon <[email protected]> Reviewed-by: Alexey Serbin <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/e9448dac Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/e9448dac Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/e9448dac Branch: refs/heads/master Commit: e9448dac5d4d70a4c6e05c3d8e98c4c5d8c3fc14 Parents: 3b2df01 Author: Todd Lipcon <[email protected]> Authored: Tue Feb 27 23:41:17 2018 -0800 Committer: Todd Lipcon <[email protected]> Committed: Wed Feb 28 09:08:55 2018 +0000 ---------------------------------------------------------------------- src/kudu/util/debug/unwind_safeness.cc | 38 ++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/e9448dac/src/kudu/util/debug/unwind_safeness.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/debug/unwind_safeness.cc b/src/kudu/util/debug/unwind_safeness.cc index 3339189..4e233ef 100644 --- a/src/kudu/util/debug/unwind_safeness.cc +++ b/src/kudu/util/debug/unwind_safeness.cc @@ -32,8 +32,6 @@ #include <glog/logging.h> -#include "kudu/gutil/port.h" - #define CALL_ORIG(func_name, ...) \ ((decltype(&func_name))g_orig_ ## func_name)(__VA_ARGS__) @@ -41,6 +39,10 @@ typedef int (*dl_iterate_phdr_cbtype)(struct dl_phdr_info *, size_t, void *); namespace { +// Whether InitializeIfNecessary() has been called. +bool g_initted; + +// The original versions of our wrapped functions. void* g_orig_dlopen; void* g_orig_dlclose; #ifndef __APPLE__ @@ -68,13 +70,32 @@ void *dlsym_or_die(const char *sym) { return ret; } +// Initialize the global variables which store the original references. This is +// set up as a constructor so that we're guaranteed to call this before main() +// while we are still single-threaded. +// +// NOTE: We _also_ call explicitly this from each of the wrappers, because +// there are some cases where the constructors of dynamic libraries may call +// dlopen, and there is no guarantee that our own constructor runs before +// the constructor of other libraries. +// +// A couple examples of the above: +// +// 1) In ASAN builds, the sanitizer runtime ends up calling dl_iterate_phdr from its +// initialization. +// 2) OpenSSL in FIPS mode calls dlopen() within its constructor. __attribute__((constructor)) -void Init(void) { +void InitIfNecessary() { + // Dynamic library initialization is always single-threaded, so there's no + // need for any synchronization here. + if (g_initted) return; + g_orig_dlopen = dlsym_or_die("dlopen"); g_orig_dlclose = dlsym_or_die("dlclose"); #ifndef __APPLE__ // This function doesn't exist on macOS. g_orig_dl_iterate_phdr = dlsym_or_die("dl_iterate_phdr"); #endif + g_initted = true; } } // anonymous namespace @@ -92,11 +113,13 @@ bool SafeToUnwindStack() { extern "C" { void *dlopen(const char *filename, int flag) { // NOLINT + InitIfNecessary(); ScopedBumpDepth d; return CALL_ORIG(dlopen, filename, flag); } int dlclose(void *handle) { // NOLINT + InitIfNecessary(); ScopedBumpDepth d; return CALL_ORIG(dlclose, handle); } @@ -105,14 +128,7 @@ int dlclose(void *handle) { // NOLINT // function and blocks signals while calling it. #if !defined(THREAD_SANITIZER) && !defined(__APPLE__) int dl_iterate_phdr(dl_iterate_phdr_cbtype callback, void *data) { // NOLINT - // In ASAN builds, the sanitizer runtime ends up calling dl_iterate_phdr from its - // initialization, which runs before our own constructor functions. In that case, we - // need to call Init() from here rather than via the normal path. There's no need to - // worry about thread-safety since this all happens single-threaded during startup. - if (PREDICT_FALSE(!g_orig_dl_iterate_phdr)) { - Init(); - } - + InitIfNecessary(); ScopedBumpDepth d; return CALL_ORIG(dl_iterate_phdr, callback, data); }
