From 071c860c2b8191f6dd6833a78c530910439bf2ee Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 7 Sep 2015 14:32:00 +0200 Subject: [PATCH] crypto: replace rwlocks with simple mutexes It was pointed out by Zhou Ran that the Windows XP implementation of uv_rwlock_rdlock() and friends may unlock the inner write mutex on a different thread than the one that locked it, resulting in undefined behavior. The only place that uses rwlocks is the crypto module. Make that use normal (simple) mutexes instead. OpenSSL's critical sections are generally very short, with exclusive access outnumbering shared access by a factor of three or more, so it's not as if using rwlocks gives a decisive performance advantage. PR-URL: https://github.com/nodejs/node/pull/2723 Reviewed-By: Fedor Indutny --- src/node_crypto.cc | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 03650a98c61fda..c81bd807dfce82 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -113,7 +113,7 @@ struct ClearErrorOnReturn { ~ClearErrorOnReturn() { ERR_clear_error(); } }; -static uv_rwlock_t* locks; +static uv_mutex_t* locks; const char* root_certs[] = { #include "node_root_certs.h" // NOLINT(build/include_order) @@ -165,29 +165,22 @@ static void crypto_lock_init(void) { int i, n; n = CRYPTO_num_locks(); - locks = new uv_rwlock_t[n]; + locks = new uv_mutex_t[n]; for (i = 0; i < n; i++) - if (uv_rwlock_init(locks + i)) + if (uv_mutex_init(locks + i)) abort(); } static void crypto_lock_cb(int mode, int n, const char* file, int line) { - assert((mode & CRYPTO_LOCK) || (mode & CRYPTO_UNLOCK)); - assert((mode & CRYPTO_READ) || (mode & CRYPTO_WRITE)); + assert(!(mode & CRYPTO_LOCK) ^ !(mode & CRYPTO_UNLOCK)); + assert(!(mode & CRYPTO_READ) ^ !(mode & CRYPTO_WRITE)); - if (mode & CRYPTO_LOCK) { - if (mode & CRYPTO_READ) - uv_rwlock_rdlock(locks + n); - else - uv_rwlock_wrlock(locks + n); - } else { - if (mode & CRYPTO_READ) - uv_rwlock_rdunlock(locks + n); - else - uv_rwlock_wrunlock(locks + n); - } + if (mode & CRYPTO_LOCK) + uv_mutex_lock(locks + n); + else + uv_mutex_unlock(locks + n); }