This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 8d0a94534390bd30b22f314488c7407ce3253fdb Author: Chris McFarlen <[email protected]> AuthorDate: Wed Apr 10 10:52:24 2024 -0500 Manage storage for ssl hooks (#11224) * Manage storage for ssl hooks * safer THREAD_ALLOC and THREAD_FREE (cherry picked from commit 6d4f2b117fc21e18f6d3884ce92f06a2e883c555) --- include/iocore/eventsystem/ProxyAllocator.h | 5 +++-- include/iocore/eventsystem/Thread.h | 2 +- include/iocore/net/SSLAPIHooks.h | 7 ++----- src/api/APIHooks.cc | 4 +++- src/api/InkAPI.cc | 2 +- src/api/InkAPIInternal.cc | 1 - src/iocore/cache/unit_tests/main.cc | 1 - src/iocore/eventsystem/Thread.cc | 7 +++++++ src/iocore/net/Net.cc | 1 - src/iocore/net/SSLAPIHooks.cc | 10 +++++----- src/iocore/net/SSLNetVConnection.cc | 18 +++++++++--------- src/iocore/net/SSLUtils.cc | 4 ++-- src/iocore/net/TLSSessionResumptionSupport.cc | 2 +- src/iocore/net/unit_tests/unit_test_main.cc | 1 - 14 files changed, 34 insertions(+), 31 deletions(-) diff --git a/include/iocore/eventsystem/ProxyAllocator.h b/include/iocore/eventsystem/ProxyAllocator.h index 851de14624..31043b4f96 100644 --- a/include/iocore/eventsystem/ProxyAllocator.h +++ b/include/iocore/eventsystem/ProxyAllocator.h @@ -73,6 +73,7 @@ void thread_freeup(Allocator &a, ProxyAllocator &l); // #define THREAD_ALLOC(_a, _t, ...) thread_alloc(::_a, _t->_a, ##__VA_ARGS__) #define THREAD_ALLOC_INIT(_a, _t, ...) thread_alloc(::_a, _t->_a, ##__VA_ARGS__) +#define SAFE_THREAD_ALLOC(_a, _t, ...) (_t ? thread_alloc(::_a, _t->_a, ##__VA_ARGS__) : ::_a.alloc(##__VA_ARGS__)) #else @@ -86,8 +87,8 @@ void thread_freeup(Allocator &a, ProxyAllocator &l); #define THREAD_FREE(_p, _a, _tin) \ do { \ ::_a.destroy_if_enabled(_p); \ - if (!cmd_disable_pfreelist) { \ - Thread *_t = (_tin); \ + Thread *_t = (_tin); \ + if (_t && !cmd_disable_pfreelist) { \ *(char **)_p = (char *)_t->_a.freelist; \ _t->_a.freelist = _p; \ _t->_a.allocated++; \ diff --git a/include/iocore/eventsystem/Thread.h b/include/iocore/eventsystem/Thread.h index 372f6d6cb0..31a446186b 100644 --- a/include/iocore/eventsystem/Thread.h +++ b/include/iocore/eventsystem/Thread.h @@ -161,7 +161,7 @@ public: Thread(const Thread &) = delete; Thread &operator=(const Thread &) = delete; - virtual ~Thread() {} + virtual ~Thread(); protected: Thread(); diff --git a/include/iocore/net/SSLAPIHooks.h b/include/iocore/net/SSLAPIHooks.h index 4c4bbe76f1..120dcdb411 100644 --- a/include/iocore/net/SSLAPIHooks.h +++ b/include/iocore/net/SSLAPIHooks.h @@ -52,9 +52,6 @@ private: class SSLAPIHooks : public FeatureAPIHooks<TSSslHookInternalID, TSSslHookInternalID::NUM> { +public: + static SSLAPIHooks *instance(); }; - -// there is no corresponding deinit; we leak the resource on shutdown -void init_global_ssl_hooks(); - -extern SSLAPIHooks *g_ssl_hooks; diff --git a/src/api/APIHooks.cc b/src/api/APIHooks.cc index 9d08a6932d..49eb99c9ae 100644 --- a/src/api/APIHooks.cc +++ b/src/api/APIHooks.cc @@ -23,6 +23,7 @@ #include "api/APIHooks.h" +#include "api/APIHook.h" #include "tscore/Allocator.h" // inkevent @@ -42,7 +43,8 @@ APIHooks::append(INKContInternal *cont) { APIHook *api_hook; - api_hook = THREAD_ALLOC(apiHookAllocator, this_thread()); + Thread *t = this_thread(); + api_hook = SAFE_THREAD_ALLOC(apiHookAllocator, t); api_hook->m_cont = cont; m_hooks.enqueue(api_hook); diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 17c10b3bba..9ef9510302 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -3613,7 +3613,7 @@ TSHttpHookAdd(TSHttpHookID id, TSCont contp) TSSslHookInternalID internalId{id}; if (internalId.is_in_bounds()) { - g_ssl_hooks->append(internalId, icontp); + SSLAPIHooks::instance()->append(internalId, icontp); } else { // Follow through the regular HTTP hook framework http_global_hooks->append(id, icontp); } diff --git a/src/api/InkAPIInternal.cc b/src/api/InkAPIInternal.cc index b1db3fe914..0dd4cc0399 100644 --- a/src/api/InkAPIInternal.cc +++ b/src/api/InkAPIInternal.cc @@ -788,7 +788,6 @@ api_init() TS_HTTP_LEN_S_MAXAGE = HTTP_LEN_S_MAXAGE; init_global_http_hooks(); - init_global_ssl_hooks(); init_global_lifecycle_hooks(); global_config_cbs = new ConfigUpdateCbTable; diff --git a/src/iocore/cache/unit_tests/main.cc b/src/iocore/cache/unit_tests/main.cc index 895b478541..b057c3f210 100644 --- a/src/iocore/cache/unit_tests/main.cc +++ b/src/iocore/cache/unit_tests/main.cc @@ -141,7 +141,6 @@ struct EventProcessorListener : Catch::TestEventListenerBase { ink_assert(GLOBAL_DATA != nullptr); init_global_http_hooks(); - init_global_ssl_hooks(); netProcessor.init(); eventProcessor.start(THREADS); diff --git a/src/iocore/eventsystem/Thread.cc b/src/iocore/eventsystem/Thread.cc index 959ec78c02..9a3dd9d65b 100644 --- a/src/iocore/eventsystem/Thread.cc +++ b/src/iocore/eventsystem/Thread.cc @@ -44,6 +44,13 @@ Thread::Thread() mutex = new_ProxyMutex(); } +Thread::~Thread() +{ + if (this_thread_ptr == this) { + this_thread_ptr = nullptr; + } +} + /////////////////////////////////////////////// // Unix & non-NT Interface impl // /////////////////////////////////////////////// diff --git a/src/iocore/net/Net.cc b/src/iocore/net/Net.cc index 1e42746f57..c7fc304309 100644 --- a/src/iocore/net/Net.cc +++ b/src/iocore/net/Net.cc @@ -149,7 +149,6 @@ ink_net_init(ts::ModuleVersion version) if (!init_called) { configure_net(); register_net_stats(); - init_global_ssl_hooks(); } init_called = 1; diff --git a/src/iocore/net/SSLAPIHooks.cc b/src/iocore/net/SSLAPIHooks.cc index 1411cf0733..b4abb50248 100644 --- a/src/iocore/net/SSLAPIHooks.cc +++ b/src/iocore/net/SSLAPIHooks.cc @@ -22,11 +22,11 @@ */ #include "iocore/net/SSLAPIHooks.h" +#include <memory> -SSLAPIHooks *g_ssl_hooks = nullptr; - -void -init_global_ssl_hooks() +SSLAPIHooks * +SSLAPIHooks::instance() { - g_ssl_hooks = new SSLAPIHooks; + static auto hooks = std::make_unique<SSLAPIHooks>(); + return hooks.get(); } diff --git a/src/iocore/net/SSLNetVConnection.cc b/src/iocore/net/SSLNetVConnection.cc index b695efb175..41febf6974 100644 --- a/src/iocore/net/SSLNetVConnection.cc +++ b/src/iocore/net/SSLNetVConnection.cc @@ -1291,7 +1291,7 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err) Metrics::Counter::increment(ssl_rsb.total_attempts_handshake_count_in); if (!curHook) { Dbg(dbg_ctl_ssl, "Initialize preaccept curHook from NULL"); - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_VCONN_START_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_VCONN_START_HOOK)); } else { curHook = curHook->next(); } @@ -1596,7 +1596,7 @@ SSLNetVConnection::sslClientHandShakeEvent(int &err) Metrics::Counter::increment(ssl_rsb.total_attempts_handshake_count_out); if (!curHook) { Dbg(dbg_ctl_ssl, "Initialize outbound connect curHook from NULL"); - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_VCONN_OUTBOUND_START_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_VCONN_OUTBOUND_START_HOOK)); } else { curHook = curHook->next(); } @@ -1871,7 +1871,7 @@ SSLNetVConnection::callHooks(TSEvent eventId) case HANDSHAKE_HOOKS_CLIENT_HELLO: case HANDSHAKE_HOOKS_CLIENT_HELLO_INVOKE: if (!curHook) { - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_SSL_CLIENT_HELLO_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_SSL_CLIENT_HELLO_HOOK)); } else { curHook = curHook->next(); } @@ -1885,14 +1885,14 @@ SSLNetVConnection::callHooks(TSEvent eventId) // The server verify event addresses ATS to origin handshake // All the other events are for client to ATS if (!curHook) { - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_SSL_VERIFY_SERVER_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_SSL_VERIFY_SERVER_HOOK)); } else { curHook = curHook->next(); } break; case HANDSHAKE_HOOKS_SNI: if (!curHook) { - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_SSL_SERVERNAME_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_SSL_SERVERNAME_HOOK)); } else { curHook = curHook->next(); } @@ -1903,7 +1903,7 @@ SSLNetVConnection::callHooks(TSEvent eventId) case HANDSHAKE_HOOKS_CERT: case HANDSHAKE_HOOKS_CERT_INVOKE: if (!curHook) { - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_SSL_CERT_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_SSL_CERT_HOOK)); } else { curHook = curHook->next(); } @@ -1916,7 +1916,7 @@ SSLNetVConnection::callHooks(TSEvent eventId) case HANDSHAKE_HOOKS_CLIENT_CERT: case HANDSHAKE_HOOKS_CLIENT_CERT_INVOKE: if (!curHook) { - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_SSL_VERIFY_CLIENT_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_SSL_VERIFY_CLIENT_HOOK)); } else { curHook = curHook->next(); } @@ -1926,14 +1926,14 @@ SSLNetVConnection::callHooks(TSEvent eventId) if (eventId == TS_EVENT_VCONN_CLOSE) { sslHandshakeHookState = HANDSHAKE_HOOKS_DONE; if (curHook == nullptr) { - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_VCONN_CLOSE_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_VCONN_CLOSE_HOOK)); } else { curHook = curHook->next(); } } else if (eventId == TS_EVENT_VCONN_OUTBOUND_CLOSE) { sslHandshakeHookState = HANDSHAKE_HOOKS_DONE; if (curHook == nullptr) { - curHook = g_ssl_hooks->get(TSSslHookInternalID(TS_VCONN_OUTBOUND_CLOSE_HOOK)); + curHook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_VCONN_OUTBOUND_CLOSE_HOOK)); } else { curHook = curHook->next(); } diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 4007741f4d..3d74f5b8a0 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -232,7 +232,7 @@ ssl_new_cached_session(SSL *ssl, SSL_SESSION *sess) session_cache->insertSession(sid, sess, ssl); // Call hook after new session is created - APIHook *hook = g_ssl_hooks->get(TSSslHookInternalID(TS_SSL_SESSION_HOOK)); + APIHook *hook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_SSL_SESSION_HOOK)); while (hook) { hook->invoke(TS_EVENT_SSL_SESSION_NEW, &sid); hook = hook->m_link.next; @@ -255,7 +255,7 @@ ssl_rm_cached_session(SSL_CTX *ctx, SSL_SESSION *sess) SSLSessionID sid(id, len); // Call hook before session is removed - APIHook *hook = g_ssl_hooks->get(TSSslHookInternalID(TS_SSL_SESSION_HOOK)); + APIHook *hook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_SSL_SESSION_HOOK)); while (hook) { hook->invoke(TS_EVENT_SSL_SESSION_REMOVE, &sid); hook = hook->m_link.next; diff --git a/src/iocore/net/TLSSessionResumptionSupport.cc b/src/iocore/net/TLSSessionResumptionSupport.cc index 90394d63dc..d6a38d65e0 100644 --- a/src/iocore/net/TLSSessionResumptionSupport.cc +++ b/src/iocore/net/TLSSessionResumptionSupport.cc @@ -151,7 +151,7 @@ TLSSessionResumptionSupport::getSession(SSL *ssl, const unsigned char *id, int l } } - APIHook *hook = g_ssl_hooks->get(TSSslHookInternalID(TS_SSL_SESSION_HOOK)); + APIHook *hook = SSLAPIHooks::instance()->get(TSSslHookInternalID(TS_SSL_SESSION_HOOK)); while (hook) { hook->invoke(TS_EVENT_SSL_SESSION_GET, &sid); hook = hook->m_link.next; diff --git a/src/iocore/net/unit_tests/unit_test_main.cc b/src/iocore/net/unit_tests/unit_test_main.cc index b8dc70375e..5fd592bea8 100644 --- a/src/iocore/net/unit_tests/unit_test_main.cc +++ b/src/iocore/net/unit_tests/unit_test_main.cc @@ -60,7 +60,6 @@ public: main_thread->set_specific(); SSLConfig::startup(); - init_global_ssl_hooks(); } void
