Make bssl_shim's setup logic infallible Trying to handle malloc failures here is a bit tedious. Just suppress malloc failures because nothing useful can progress when we can't even allocate ex_data. Change-Id: Ieaf417bcf9285783a76097319782282ce74e4734 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66648 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/internal.h b/crypto/internal.h index 5894d18..b5e27f5 100644 --- a/crypto/internal.h +++ b/crypto/internal.h
@@ -271,8 +271,20 @@ // should be called in between independent tests, at a point where failure from // a previous test will not impact subsequent ones. OPENSSL_EXPORT void OPENSSL_reset_malloc_counter_for_testing(void); + +// OPENSSL_disable_malloc_failures_for_testing, when malloc testing is enabled, +// disables simulated malloc failures. Calls to |OPENSSL_malloc| will not +// increment the malloc counter or synthesize failures. This may be used to skip +// simulating malloc failures in some region of code. +OPENSSL_EXPORT void OPENSSL_disable_malloc_failures_for_testing(void); + +// OPENSSL_enable_malloc_failures_for_testing, when malloc testing is enabled, +// re-enables simulated malloc failures. +OPENSSL_EXPORT void OPENSSL_enable_malloc_failures_for_testing(void); #else OPENSSL_INLINE void OPENSSL_reset_malloc_counter_for_testing(void) {} +OPENSSL_INLINE void OPENSSL_disable_malloc_failures_for_testing(void) {} +OPENSSL_INLINE void OPENSSL_enable_malloc_failures_for_testing(void) {} #endif #if defined(__has_builtin)
diff --git a/crypto/mem.c b/crypto/mem.c index 9e81476..0f286f8 100644 --- a/crypto/mem.c +++ b/crypto/mem.c
@@ -138,7 +138,7 @@ 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, - any_malloc_failed = 0; + any_malloc_failed = 0, disable_malloc_failures = 0; static void malloc_exit_handler(void) { CRYPTO_MUTEX_lock_read(&malloc_failure_lock); @@ -168,7 +168,7 @@ static int should_fail_allocation() { static CRYPTO_once_t once = CRYPTO_ONCE_INIT; CRYPTO_once(&once, init_malloc_failure); - if (!malloc_failure_enabled) { + if (!malloc_failure_enabled || disable_malloc_failures) { return 0; } @@ -195,6 +195,20 @@ CRYPTO_MUTEX_unlock_write(&malloc_failure_lock); } +void OPENSSL_disable_malloc_failures_for_testing(void) { + CRYPTO_MUTEX_lock_write(&malloc_failure_lock); + BSSL_CHECK(!disable_malloc_failures); + disable_malloc_failures = 1; + CRYPTO_MUTEX_unlock_write(&malloc_failure_lock); +} + +void OPENSSL_enable_malloc_failures_for_testing(void) { + CRYPTO_MUTEX_lock_write(&malloc_failure_lock); + BSSL_CHECK(disable_malloc_failures); + disable_malloc_failures = 0; + CRYPTO_MUTEX_unlock_write(&malloc_failure_lock); +} + #else static int should_fail_allocation(void) { return 0; } #endif
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 71812c5..d359894 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -531,30 +531,35 @@ return true; } -static CRYPTO_once_t once = CRYPTO_ONCE_INIT; -static int g_config_index = 0; -static CRYPTO_BUFFER_POOL *g_pool = nullptr; +static CRYPTO_BUFFER_POOL *BufferPool() { + static CRYPTO_BUFFER_POOL *pool = [&] { + OPENSSL_disable_malloc_failures_for_testing(); + CRYPTO_BUFFER_POOL *ret = CRYPTO_BUFFER_POOL_new(); + BSSL_CHECK(ret != nullptr); + OPENSSL_enable_malloc_failures_for_testing(); + return ret; + }(); + return pool; +} -static bool InitGlobals() { - CRYPTO_once(&once, [] { - g_config_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL); - g_pool = CRYPTO_BUFFER_POOL_new(); - }); - return g_config_index >= 0 && g_pool != nullptr; +static int TestConfigExDataIndex() { + static int index = [&] { + OPENSSL_disable_malloc_failures_for_testing(); + int ret = SSL_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr); + BSSL_CHECK(ret >= 0); + OPENSSL_enable_malloc_failures_for_testing(); + return ret; + }(); + return index; } bool SetTestConfig(SSL *ssl, const TestConfig *config) { - if (!InitGlobals()) { - return false; - } - return SSL_set_ex_data(ssl, g_config_index, (void *)config) == 1; + return SSL_set_ex_data(ssl, TestConfigExDataIndex(), (void *)config) == 1; } const TestConfig *GetTestConfig(const SSL *ssl) { - if (!InitGlobals()) { - return nullptr; - } - return static_cast<const TestConfig *>(SSL_get_ex_data(ssl, g_config_index)); + return static_cast<const TestConfig *>( + SSL_get_ex_data(ssl, TestConfigExDataIndex())); } static int LegacyOCSPCallback(SSL *ssl, void *arg) { @@ -1416,17 +1421,13 @@ } bssl::UniquePtr<SSL_CTX> TestConfig::SetupCtx(SSL_CTX *old_ctx) const { - if (!InitGlobals()) { - return nullptr; - } - bssl::UniquePtr<SSL_CTX> ssl_ctx( SSL_CTX_new(is_dtls ? DTLS_method() : TLS_method())); if (!ssl_ctx) { return nullptr; } - SSL_CTX_set0_buffer_pool(ssl_ctx.get(), g_pool); + SSL_CTX_set0_buffer_pool(ssl_ctx.get(), BufferPool()); std::string cipher_list = "ALL:TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256"; if (!cipher.empty()) {