summaryrefslogtreecommitdiffstats
path: root/rpc/rpc-transport
diff options
context:
space:
mode:
authorJeff Darcy <jdarcy@redhat.com>2015-03-31 14:34:22 -0400
committerVijay Bellur <vbellur@redhat.com>2015-04-09 09:55:37 +0000
commit8830e90fa1b131057e4ee1742cb83d78102714c0 (patch)
treec83304de5eb99302f294e45039b356bcc87f69d1 /rpc/rpc-transport
parent9df5fdb357da74de38cb4e8c2cea776023641164 (diff)
socket: use OpenSSL multi-threading interfaces
OpenSSL isn't thread-safe unless you register these locking and thread ID functions. Most often the crashes would occur around X509_verify_cert, even though it's insane that the certificate parsing functions wouldn't be thread-safe. The bug for this was filed over two years ago, but it didn't seem like a high priority because the bug didn't bite anyone until it caused a spurious regression-test failure. Ironically, that was on a test for a *different* spurious regression-test failure, which I guess is just deserts[1] for leaving this on the to-do list so long. [1] Yes, it really is "deserts" in that phrase - not as in very dry places, but from late Latin "deservire" meaning to serve well or zealously. Aren't commit messages educational? Change-Id: I2a6c0e9b361abf54efa10ffbbbe071404f82b0d9 BUG: 906763 Signed-off-by: Jeff Darcy <jdarcy@redhat.com> Reviewed-on: http://review.gluster.org/10075 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
Diffstat (limited to 'rpc/rpc-transport')
-rw-r--r--rpc/rpc-transport/socket/src/socket-mem-types.h1
-rw-r--r--rpc/rpc-transport/socket/src/socket.c72
2 files changed, 70 insertions, 3 deletions
diff --git a/rpc/rpc-transport/socket/src/socket-mem-types.h b/rpc/rpc-transport/socket/src/socket-mem-types.h
index e5553b172a2..3181406625d 100644
--- a/rpc/rpc-transport/socket/src/socket-mem-types.h
+++ b/rpc/rpc-transport/socket/src/socket-mem-types.h
@@ -15,6 +15,7 @@
typedef enum gf_sock_mem_types_ {
gf_sock_connect_error_state_t = gf_common_mt_end + 1,
+ gf_sock_mt_lock_array,
gf_sock_mt_end
} gf_sock_mem_types_t;
diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c
index e128afb5e8d..82a84dd5fcc 100644
--- a/rpc/rpc-transport/socket/src/socket.c
+++ b/rpc/rpc-transport/socket/src/socket.c
@@ -3681,6 +3681,63 @@ out:
}
+/*
+ * Unlike the stuff in init, this only needs to be called once GLOBALLY no
+ * matter how many translators/sockets we end up with. Conveniently,
+ * __attribute__(constructor) provides exactly those semantics in a pretty
+ * portable fashion.
+ */
+
+static pthread_mutex_t *lock_array = NULL;
+static gf_boolean_t constructor_ok = _gf_false;
+
+static void
+locking_func (int mode, int type, const char *file, int line)
+{
+ if (mode & CRYPTO_UNLOCK) {
+ pthread_mutex_unlock (&lock_array[type]);
+ } else {
+ pthread_mutex_lock (&lock_array[type]);
+ }
+}
+
+static void
+threadid_func (CRYPTO_THREADID *id)
+{
+ /*
+ * We're not supposed to know whether a pthread_t is a number or a
+ * pointer, but we definitely need an unsigned long. Even though it
+ * happens to be an unsigned long already on Linux, do the cast just in
+ * case that's not so on another platform. Note that this can still
+ * break if any platforms are left where a pointer is larger than an
+ * unsigned long. In that case there's not much we can do; hopefully
+ * anyone porting to such a platform will be aware enough to notice the
+ * compile warnings about truncating the pointer value.
+ */
+ CRYPTO_THREADID_set_numeric (id, (unsigned long)pthread_self());
+}
+
+static void __attribute__((constructor))
+init_openssl_mt (void)
+{
+ int num_locks = CRYPTO_num_locks();
+ int i;
+
+ lock_array = GF_CALLOC (num_locks, sizeof(pthread_mutex_t),
+ gf_sock_mt_lock_array);
+ if (lock_array) {
+ for (i = 0; i < num_locks; ++i) {
+ pthread_mutex_init (&lock_array[i], NULL);
+ }
+ CRYPTO_set_locking_callback (locking_func);
+ CRYPTO_THREADID_set_callback (threadid_func);
+ constructor_ok = _gf_true;
+ }
+
+ SSL_library_init();
+ SSL_load_error_strings();
+}
+
static int
socket_init (rpc_transport_t *this)
{
@@ -3910,8 +3967,18 @@ socket_init (rpc_transport_t *this)
}
if (priv->ssl_enabled || priv->mgmt_ssl) {
- SSL_library_init();
- SSL_load_error_strings();
+ /*
+ * The right time to check this is after all of our relevant
+ * fields have been set, but before we start issuing OpenSSL
+ * calls for the current translator. In other words, now.
+ */
+ if (!constructor_ok) {
+ gf_log (this->name, GF_LOG_ERROR,
+ "can't initialize TLS socket (%s)",
+ "static constructor failed");
+ goto err;
+ }
+
priv->ssl_meth = (SSL_METHOD *)TLSv1_2_method();
priv->ssl_ctx = SSL_CTX_new(priv->ssl_meth);
@@ -4015,7 +4082,6 @@ fini (rpc_transport_t *this)
this->private = NULL;
}
-
int32_t
init (rpc_transport_t *this)
{