Move malloc failure testing into OPENSSL_malloc Rather than trying to override the actual malloc symbol, just intercept OPENSSL_malloc and gate it on a build flag. (When we first wrote these, OPENSSL_malloc was just an alias for malloc.) This has several benefits: - This is cross platform. We don't interfere with sanitizers or the libc, or have to mess with global symbols. - This removes the reason bssl_shim and handshaker linked test_support_lib, so we can fix the tes_support_lib / gtest dependency. - If we ever reduce the scope of fallible mallocs, we'll want to constrain the tests to only the ones that are fallible. An interception strategy like this can do it. Hopefully that will also take less time to run in the future. Also fix the ssl malloc failure tests, as they haven't been working for a while. (Malloc failure tests still take far too long to run to the end though. My immediate motivation is less malloc failure and more to tidy up the build.) Bug: 563 Change-Id: I32165b8ecbebfdcfde26964e06a404762edd28e3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56925 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/mem.c b/crypto/mem.c index 6ee5b0b..abba4a4 100644 --- a/crypto/mem.c +++ b/crypto/mem.c
@@ -58,6 +58,7 @@ #include <assert.h> #include <stdarg.h> +#include <stdlib.h> #include <stdio.h> #include <openssl/err.h> @@ -68,6 +69,12 @@ OPENSSL_MSVC_PRAGMA(warning(pop)) #endif +#if defined(BORINGSSL_MALLOC_FAILURE_TESTING) +#include <errno.h> +#include <signal.h> +#include <unistd.h> +#endif + #include "internal.h" @@ -134,7 +141,68 @@ 3, 0, }; +#if defined(BORINGSSL_MALLOC_FAILURE_TESTING) +static struct CRYPTO_STATIC_MUTEX malloc_failure_lock = + CRYPTO_STATIC_MUTEX_INIT; +static uint64_t current_malloc_count = 0; +static uint64_t malloc_number_to_fail = 0; +static int malloc_failure_enabled = 0, break_on_malloc_fail = 0; + +static void malloc_exit_handler(void) { + CRYPTO_STATIC_MUTEX_lock_read(&malloc_failure_lock); + if (malloc_failure_enabled && current_malloc_count > malloc_number_to_fail) { + _exit(88); + } + CRYPTO_STATIC_MUTEX_unlock_read(&malloc_failure_lock); +} + +static void init_malloc_failure(void) { + const char *env = getenv("MALLOC_NUMBER_TO_FAIL"); + if (env != NULL && env[0] != 0) { + char *endptr; + malloc_number_to_fail = strtoull(env, &endptr, 10); + if (*endptr == 0) { + malloc_failure_enabled = 1; + atexit(malloc_exit_handler); + } + } + break_on_malloc_fail = getenv("MALLOC_BREAK_ON_FAIL") != NULL; +} + +// should_fail_allocation returns one if the current allocation should fail and +// zero otherwise. +static int should_fail_allocation() { + static CRYPTO_once_t once = CRYPTO_ONCE_INIT; + CRYPTO_once(&once, init_malloc_failure); + if (!malloc_failure_enabled) { + return 0; + } + + // We lock just so multi-threaded tests are still correct, but we won't test + // every malloc exhaustively. + CRYPTO_STATIC_MUTEX_lock_write(&malloc_failure_lock); + int should_fail = current_malloc_count == malloc_number_to_fail; + current_malloc_count++; + CRYPTO_STATIC_MUTEX_unlock_write(&malloc_failure_lock); + + if (should_fail && break_on_malloc_fail) { + raise(SIGTRAP); + } + if (should_fail) { + errno = ENOMEM; + } + return should_fail; +} + +#else +static int should_fail_allocation(void) { return 0; } +#endif + void *OPENSSL_malloc(size_t size) { + if (should_fail_allocation()) { + return NULL; + } + if (OPENSSL_memory_alloc != NULL) { assert(OPENSSL_memory_free != NULL); assert(OPENSSL_memory_get_size != NULL); @@ -194,6 +262,10 @@ } void *OPENSSL_realloc(void *orig_ptr, size_t new_size) { + if (should_fail_allocation()) { + return NULL; + } + if (orig_ptr == NULL) { return OPENSSL_malloc(new_size); }
diff --git a/crypto/test/CMakeLists.txt b/crypto/test/CMakeLists.txt index b968fd7..bf20863 100644 --- a/crypto/test/CMakeLists.txt +++ b/crypto/test/CMakeLists.txt
@@ -5,7 +5,6 @@ abi_test.cc file_test.cc - malloc.cc test_util.cc wycheproof_util.cc )
diff --git a/crypto/test/malloc.cc b/crypto/test/malloc.cc deleted file mode 100644 index 1718939..0000000 --- a/crypto/test/malloc.cc +++ /dev/null
@@ -1,143 +0,0 @@ -/* Copyright (c) 2014, Google Inc. - * - * Permission to use, copy, modify, and/or distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY - * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION - * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN - * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ - -#include <openssl/base.h> - -#if defined(__GLIBC__) && !defined(__UCLIBC__) -#define OPENSSL_GLIBC -#endif - -// This file isn't built on ARM or Aarch64 because we link statically in those -// builds and trying to override malloc in a static link doesn't work. It also -// requires glibc. It's also disabled on ASan builds as this interferes with -// ASan's malloc interceptor. -// -// TODO(davidben): See if this and ASan's and MSan's interceptors can be made to -// coexist. -#if defined(__linux__) && defined(OPENSSL_GLIBC) && !defined(OPENSSL_ARM) && \ - !defined(OPENSSL_AARCH64) && !defined(OPENSSL_ASAN) && \ - !defined(OPENSSL_MSAN) && !defined(OPENSSL_TSAN) - -#include <errno.h> -#include <signal.h> -#include <stdint.h> -#include <stdio.h> -#include <stdlib.h> -#include <unistd.h> - -#include <new> - - -// This file defines overrides for the standard allocation functions that allow -// a given allocation to be made to fail for testing. If the program is run -// with MALLOC_NUMBER_TO_FAIL set to a base-10 number then that allocation will -// return NULL. If MALLOC_BREAK_ON_FAIL is also defined then the allocation -// will signal SIGTRAP rather than return NULL. -// -// This code is not thread safe. - -static uint64_t current_malloc_count = 0; -static uint64_t malloc_number_to_fail = 0; -static bool failure_enabled = false, break_on_fail = false, in_call = false; - -extern "C" { -// These are other names for the standard allocation functions. -extern void *__libc_malloc(size_t size); -extern void *__libc_calloc(size_t num_elems, size_t size); -extern void *__libc_realloc(void *ptr, size_t size); -} - -static void exit_handler(void) { - if (failure_enabled && current_malloc_count > malloc_number_to_fail) { - _exit(88); - } -} - -static void cpp_new_handler() { - // Return to try again. It won't fail a second time. - return; -} - -// should_fail_allocation returns true if the current allocation should fail. -static bool should_fail_allocation() { - static bool init = false; - - if (in_call) { - return false; - } - - in_call = true; - - if (!init) { - const char *env = getenv("MALLOC_NUMBER_TO_FAIL"); - if (env != NULL && env[0] != 0) { - char *endptr; - malloc_number_to_fail = strtoull(env, &endptr, 10); - if (*endptr == 0) { - failure_enabled = true; - atexit(exit_handler); - std::set_new_handler(cpp_new_handler); - } - } - break_on_fail = (NULL != getenv("MALLOC_BREAK_ON_FAIL")); - init = true; - } - - in_call = false; - - if (!failure_enabled) { - return false; - } - - bool should_fail = (current_malloc_count == malloc_number_to_fail); - current_malloc_count++; - - if (should_fail && break_on_fail) { - raise(SIGTRAP); - } - return should_fail; -} - -extern "C" { - -void *malloc(size_t size) { - if (should_fail_allocation()) { - errno = ENOMEM; - return NULL; - } - - return __libc_malloc(size); -} - -void *calloc(size_t num_elems, size_t size) { - if (should_fail_allocation()) { - errno = ENOMEM; - return NULL; - } - - return __libc_calloc(num_elems, size); -} - -void *realloc(void *ptr, size_t size) { - if (should_fail_allocation()) { - errno = ENOMEM; - return NULL; - } - - return __libc_realloc(ptr, size); -} - -} // extern "C" - -#endif // defined(linux) && GLIBC && !ARM && !AARCH64 && !ASAN && !TSAN