commit f4ac688b2bc19a5675dfd9cc17e796a9b8b692af
author David Benjamin <davidben@google.com> Tue Feb 27 14:59:21 2024 -0500
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Mar 05 19:30:31 2024 +0000
tree 08c5b5be9736f538114bfeb0c6e356c275958eac
parent ad914954425b6f095bc974784c088ec00b0be8f2

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>

crypto/internal.h

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)

crypto/mem.c

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

ssl/test/test_config.cc

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()) {