You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by cm...@apache.org on 2024/04/11 13:40:50 UTC
(trafficserver) 02/09: Manage storage for ssl hooks (#11224)
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 <ch...@mcfarlen.us>
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