MinGW: mitigate potential abort on rwlocks using PTHREAD_RWLOCK_INITI…

…ALIZER (#1530)

### Description of changes: 
* On some platforms/pthreads-implementations, there are [race
conditions](https://github.com/ldx/winpthreads/blob/fecf6566606c9111628441ed2426e7efdc5faf0e/src/rwlock.c#L161)
on the first use of rwlocks initialized with
`PTHREAD_RWLOCK_INITIALIZER`.
* This change allows for a re-attempt on lock acquisition when it
initially fails on MinGW, The current logic aborts the process when this
occurs.
* A test is added to attempt to reproduce the race-condition, although
success of it can be a false-negative for such a bug.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

---------

Co-authored-by: Justin Smith <[email protected]>

CMakeLists.txt

@@ -894,6 +894,7 @@ if(BUILD_TESTING)
     set(MEM_SET_TEST_EXEC mem_set_test.so)
     set(INTEGRATION_TEST_EXEC integration_test.so)
     set(DYNAMIC_LOADING_TEST_EXEC dynamic_loading_test.so)
+    set(RWLOCK_STATIC_INIT_TEST_EXEC rwlock_static_init.so)
   else()
     set(CRYPTO_TEST_EXEC crypto_test)
     set(RANDOM_TEST_EXEC urandom_test)
@@ -902,6 +903,7 @@ if(BUILD_TESTING)
     set(MEM_SET_TEST_EXEC mem_set_test)
     set(INTEGRATION_TEST_EXEC integration_test)
     set(DYNAMIC_LOADING_TEST_EXEC dynamic_loading_test)
+    set(RWLOCK_STATIC_INIT_TEST_EXEC rwlock_static_init)
   endif()
 
   add_subdirectory(util/fipstools/acvp/modulewrapper)

crypto/CMakeLists.txt

@@ -806,6 +806,20 @@ if(BUILD_TESTING)
   endif()
 
   add_dependencies(all_tests ${DYNAMIC_LOADING_TEST_EXEC})
+
+  message(STATUS "Generating test executable ${RWLOCK_STATIC_INIT_TEST_EXEC}.")
+  add_executable(${RWLOCK_STATIC_INIT_TEST_EXEC} rwlock_static_init.cc)
+  add_dependencies(${RWLOCK_STATIC_INIT_TEST_EXEC} crypto)
+
+  target_link_libraries(${RWLOCK_STATIC_INIT_TEST_EXEC} crypto)
+  target_include_directories(${RWLOCK_STATIC_INIT_TEST_EXEC} BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)
+  if(MSVC)
+    target_link_libraries(${RWLOCK_STATIC_INIT_TEST_EXEC} ws2_32)
+  else()
+    target_compile_options(${RWLOCK_STATIC_INIT_TEST_EXEC} PUBLIC -Wno-deprecated-declarations)
+  endif()
+  add_dependencies(all_tests ${RWLOCK_STATIC_INIT_TEST_EXEC})
+
 endif()
 
 install(TARGETS crypto

crypto/rwlock_static_init.cc

@@ -0,0 +1,43 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0 OR ISC
+
+// This test attempts to setup a potential race condition that can cause the
+// first use of an rwlock to fail when initialized using
+// PTHREAD_RWLOCK_INITIALIZER.
+// See: https://sourceforge.net/p/mingw-w64/bugs/883/
+
+#include <openssl/rand.h>
+
+#include <cstdint>
+#include <iostream>
+#include <thread>
+#include <cassert>
+#include <cstdlib>
+
+static void thread_task_rand(bool *myFlag) {
+  uint8_t buf[16];
+  if(1 == RAND_bytes(buf, sizeof(buf))) {
+    *myFlag = true;
+  }
+}
+
+int main(int _argc, char** _argv) {
+  constexpr size_t kNumThreads = 16;
+  bool myFlags[kNumThreads] = {};
+  std::thread myThreads[kNumThreads];
+
+  for (size_t i = 0; i < kNumThreads; i++) {
+    bool* myFlag = &myFlags[i];
+    myThreads[i] = std::thread(thread_task_rand, myFlag);
+  }
+  for (size_t i = 0; i < kNumThreads; i++) {
+    myThreads[i].join();
+    if(!myFlags[i]) {
+      std::cerr << "Thread " << i << " failed." << std::endl;
+      exit(1);
+      return 1;
+    }
+  }
+  std::cout << "PASS" << std::endl;
+  return 0;
+}

crypto/thread_pthread.c

@@ -14,6 +14,8 @@
 
 // Ensure we can't call OPENSSL_malloc circularly.
 #define _BORINGSSL_PROHIBIT_OPENSSL_MALLOC
+#include <errno.h>
+
 #include "internal.h"
 
 #if defined(OPENSSL_PTHREADS)
@@ -64,14 +66,42 @@ void CRYPTO_MUTEX_cleanup(CRYPTO_MUTEX *lock) {
   pthread_rwlock_destroy((pthread_rwlock_t *) lock);
 }
 
+// Some MinGW pthreads implementations might fail on first use of
+// locks initialized using PTHREAD_RWLOCK_INITIALIZER.
+// See: https://sourceforge.net/p/mingw-w64/bugs/883/
+typedef int (*pthread_rwlock_func_ptr)(pthread_rwlock_t *);
+static int rwlock_EINVAL_fallback_retry(const pthread_rwlock_func_ptr func_ptr, pthread_rwlock_t* lock) {
+  int result = EINVAL;
+#ifdef __MINGW32__
+  const int MAX_ATTEMPTS = 10;
+  int attempt_num = 0;
+  do {
+    sched_yield();
+    attempt_num += 1;
+    result = func_ptr(lock);
+  } while(result == EINVAL && attempt_num < MAX_ATTEMPTS);
+#endif
+  return result;
+}
+
 void CRYPTO_STATIC_MUTEX_lock_read(struct CRYPTO_STATIC_MUTEX *lock) {
-  if (pthread_rwlock_rdlock(&lock->lock) != 0) {
+  const int result = pthread_rwlock_rdlock(&lock->lock);
+  if (result != 0) {
+    if (result == EINVAL &&
+        0 == rwlock_EINVAL_fallback_retry(pthread_rwlock_rdlock, &lock->lock)) {
+      return;
+    }
     abort();
   }
 }
 
 void CRYPTO_STATIC_MUTEX_lock_write(struct CRYPTO_STATIC_MUTEX *lock) {
-  if (pthread_rwlock_wrlock(&lock->lock) != 0) {
+  const int result = pthread_rwlock_wrlock(&lock->lock);
+  if (result != 0) {
+    if (result == EINVAL &&
+        0 == rwlock_EINVAL_fallback_retry(pthread_rwlock_wrlock, &lock->lock)) {
+      return;
+    }
     abort();
   }
 }

util/all_tests.json

@@ -114,5 +114,10 @@
     "cmd": [
       "crypto/dynamic_loading_test"
     ]
+  },
+  {
+    "cmd": [
+      "crypto/rwlock_static_init"
+    ]
   }
 ]