commit 02d696f2a1d48de7e129c4b73f12bffdbfec0c8e
author Fred Gylys-Colwell <fredgc@google.com> Wed Feb 07 17:11:18 2018 -0800
committer Adam Langley <agl@google.com> Tue Feb 20 19:53:24 2018 +0000
tree 3f951e024db907eda2e0b99331726590551e33f8
parent ed626ec99bce0570dbf72c612182dd921e880a43

Delete |pthread_key_t| on dlclose.

When OPENSSL_DANGEROUS_RELEASE_PTHREAD_KEY is defined during the build,
this change adds a destructor function that is called when BoringSSL is
unloaded via |dlclose| or during process exit. Using |dlclose| with
BoringSSL is not supported and will leak memory, but this change allows
some code that is already doing it to survive longer.

Change-Id: Ifc6d6aae61ed0f15d61cd3dbb4ea9f8006e43dba
Reviewed-on: https://boringssl-review.googlesource.com/25784
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Fred Gylys-Colwell <fredgc@google.com>

crypto/thread_pthread.c

diff --git a/crypto/thread_pthread.c b/crypto/thread_pthread.c
index 90b3d60..f8bf595 100644
--- a/crypto/thread_pthread.c
+++ b/crypto/thread_pthread.c
@@ -94,6 +94,8 @@
 static pthread_mutex_t g_destructors_lock = PTHREAD_MUTEX_INITIALIZER;
 static thread_local_destructor_t g_destructors[NUM_OPENSSL_THREAD_LOCALS];
 
+// thread_local_destructor is called when a thread exits. It releases thread
+// local data for that thread only.
 static void thread_local_destructor(void *arg) {
   if (arg == NULL) {
     return;
@@ -119,16 +121,44 @@
 
 static pthread_once_t g_thread_local_init_once = PTHREAD_ONCE_INIT;
 static pthread_key_t g_thread_local_key;
-static int g_thread_local_failed = 0;
+static int g_thread_local_key_created = 0;
+
+// OPENSSL_DANGEROUS_RELEASE_PTHREAD_KEY can be defined to cause
+// |pthread_key_delete| to be called in a destructor function. This can be
+// useful for programs that dlclose BoringSSL.
+//
+// Note that dlclose()ing BoringSSL is not supported and will leak memory:
+// thread-local values will be leaked as well as anything initialised via a
+// once. The |pthread_key_t| is destroyed because they run out very quickly,
+// while the other leaks are slow, and this allows code that happens to use
+// dlclose() despite all the problems to continue functioning.
+//
+// This is marked "dangerous" because it can cause multi-threaded processes to
+// crash (even if they don't use dlclose): if the destructor runs while other
+// threads are still executing then they may end up using an invalid key to
+// access thread-local variables.
+//
+// This may be removed after February 2020.
+#if defined(OPENSSL_DANGEROUS_RELEASE_PTHREAD_KEY) && \
+    (defined(__GNUC__) || defined(__clang__))
+// thread_key_destructor is called when the library is unloaded with dlclose.
+static void thread_key_destructor(void) __attribute__((destructor, unused));
+static void thread_key_destructor(void) {
+  if (g_thread_local_key_created) {
+    g_thread_local_key_created = 0;
+    pthread_key_delete(g_thread_local_key);
+  }
+}
+#endif
 
 static void thread_local_init(void) {
-  g_thread_local_failed =
-      pthread_key_create(&g_thread_local_key, thread_local_destructor) != 0;
+  g_thread_local_key_created =
+      pthread_key_create(&g_thread_local_key, thread_local_destructor) == 0;
 }
 
 void *CRYPTO_get_thread_local(thread_local_data_t index) {
   CRYPTO_once(&g_thread_local_init_once, thread_local_init);
-  if (g_thread_local_failed) {
+  if (!g_thread_local_key_created) {
     return NULL;
   }
 
@@ -142,7 +172,7 @@
 int CRYPTO_set_thread_local(thread_local_data_t index, void *value,
                             thread_local_destructor_t destructor) {
   CRYPTO_once(&g_thread_local_init_once, thread_local_init);
-  if (g_thread_local_failed) {
+  if (!g_thread_local_key_created) {
     destructor(value);
     return 0;
   }