diff options
author | Spencer Jackson <spencer.jackson@mongodb.com> | 2017-08-17 14:54:10 -0400 |
---|---|---|
committer | Spencer Jackson <spencer.jackson@mongodb.com> | 2017-08-22 12:39:52 -0400 |
commit | 9747381e76070df44d824b037e67a0bfc3472502 (patch) | |
tree | d6869df26ce583d978b7ff1dc8400d80e01413e3 | |
parent | ce350e70c2eba716c4ea95b5660ada98817d3ea2 (diff) | |
download | mongo-9747381e76070df44d824b037e67a0bfc3472502.tar.gz |
SERVER-30643: Ensure thread IDs observed by OpenSSL are uniformly distributed
(cherry picked from commit d40503d57d9fd27d1ad59e4d74aa827191d77564)
-rw-r--r-- | src/mongo/util/net/ssl_manager.cpp | 64 |
1 files changed, 45 insertions, 19 deletions
diff --git a/src/mongo/util/net/ssl_manager.cpp b/src/mongo/util/net/ssl_manager.cpp index 187c0c105e2..4bb0befb4b0 100644 --- a/src/mongo/util/net/ssl_manager.cpp +++ b/src/mongo/util/net/ssl_manager.cpp @@ -37,6 +37,7 @@ #include <boost/thread/thread.hpp> #include <iostream> #include <sstream> +#include <stack> #include <string> #include <vector> @@ -171,21 +172,28 @@ const STACK_OF(X509_EXTENSION) * X509_get0_extensions(const X509* peerCert) { * OpenSSL before version 1.1.0 requires applications provide a callback which emits a thread * identifier. This ID is used to store thread specific ERR information. When a thread is * terminated, it must call ERR_remove_state or ERR_remove_thread_state. These functions may - * themselves invoke the application provided callback. + * themselves invoke the application provided callback. These IDs are stored in a hashtable with + * a questionable hash function. They must be uniformly distributed to prevent collisions. */ class SSLThreadInfo { public: static unsigned long getID() { - enforceCleanupOnShutdown(); + struct ThreadLocalState { + bool firstCall; + unsigned long id; + }; + static MONGO_TRIVIALLY_CONSTRUCTIBLE_THREAD_LOCAL ThreadLocalState state{true, 0}; + if (state.firstCall) { + state.id = _idManager.reserveID(); + unsigned long id = state.id; + boost::this_thread::at_thread_exit([id] { + ERR_remove_state(0); + _idManager.releaseID(id); + }); + state.firstCall = false; + } -#ifdef _WIN32 - return GetCurrentThreadId(); -#else - static_assert(sizeof(void*) == sizeof(unsigned long), - "OpenSSL needs the address of a thread-unique object to be castable to" - "unsigned long"); - return reinterpret_cast<unsigned long>(&errno); -#endif + return state.id; } static void lockingCallback(int mode, int type, const char* file, int line) { @@ -208,21 +216,39 @@ public: private: SSLThreadInfo() = delete; - // When called, ensures that this thread will, on termination, call ERR_remove_state. - static void enforceCleanupOnShutdown() { - static MONGO_TRIVIALLY_CONSTRUCTIBLE_THREAD_LOCAL bool firstCall = true; - if (firstCall) { - boost::this_thread::at_thread_exit([] { ERR_remove_state(0); }); - firstCall = false; - } - } - // Note: see SERVER-8734 for why we are using a recursive mutex here. // Once the deadlock fix in OpenSSL is incorporated into most distros of // Linux, this can be changed back to a nonrecursive mutex. static std::vector<std::unique_ptr<stdx::recursive_mutex>> _mutex; + + class ThreadIDManager { + public: + unsigned long reserveID() { + stdx::unique_lock<stdx::mutex> lock(_idMutex); + if (!_idLast.empty()) { + unsigned long ret = _idLast.top(); + _idLast.pop(); + return ret; + } + return ++_idNext; + } + + void releaseID(unsigned long id) { + stdx::unique_lock<stdx::mutex> lock(_idMutex); + _idLast.push(id); + } + + private: + // Machinery for producing IDs that are unique for the life of a thread. + stdx::mutex _idMutex; // Protects _idNext and _idLast. + unsigned long _idNext = 0; // Stores the next thread ID to use, if none already allocated. + std::stack<unsigned long, std::vector<unsigned long>> + _idLast; // Stores old thread IDs, for reuse. + }; + static ThreadIDManager _idManager; }; std::vector<std::unique_ptr<stdx::recursive_mutex>> SSLThreadInfo::_mutex; +SSLThreadInfo::ThreadIDManager SSLThreadInfo::_idManager; // We only want to free SSL_CTX objects if they have been populated. OpenSSL seems to perform this // check before freeing them, but because it does not document this, we should protect ourselves. |