diff options
| author | Jeff Darcy <jdarcy@redhat.com> | 2015-03-31 14:34:22 -0400 | 
|---|---|---|
| committer | Vijay Bellur <vbellur@redhat.com> | 2015-04-09 09:55:37 +0000 | 
| commit | 8830e90fa1b131057e4ee1742cb83d78102714c0 (patch) | |
| tree | c83304de5eb99302f294e45039b356bcc87f69d1 /rpc | |
| parent | 9df5fdb357da74de38cb4e8c2cea776023641164 (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')
| -rw-r--r-- | rpc/rpc-transport/socket/src/socket-mem-types.h | 1 | ||||
| -rw-r--r-- | rpc/rpc-transport/socket/src/socket.c | 72 | 
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)  {  | 
