From 436c3cf92536ef89f87a8112d3a6c682b1f0ea59 Mon Sep 17 00:00:00 2001 From: bneradt Date: Mon, 20 Apr 2026 11:40:41 -0500 Subject: [PATCH] Fix lock-order inversion deadlock in Diags::tag_activated This fixes a deadlock between Diags::tag_table_lock and the dynamic loader lock. The issue occurs when Regex::exec() lazily constructs a thread_local RegexContext whose destructor registration via __cxa_thread_atexit_impl acquires the dl loader lock. Holding tag_table_lock across exec() creates a lock-order inversion with dlopen() callers that construct a DbgCtl during plugin static initialization (which takes tag_table_lock while already holding the dl loader lock). This changes activated_tags to use std::shared_ptr and snapshots the regex under the lock before releasing it and calling exec(). The same pattern is applied to activate_taglist() which now compiles the new regex outside the lock. Fixes: #13101 --- include/tscore/DiagsTypes.h | 7 ++++--- src/tscore/Diags.cc | 42 +++++++++++++++++++++---------------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/include/tscore/DiagsTypes.h b/include/tscore/DiagsTypes.h index f770e871c4f..1b3713160b5 100644 --- a/include/tscore/DiagsTypes.h +++ b/include/tscore/DiagsTypes.h @@ -32,6 +32,7 @@ #pragma once #include +#include #include #include #include "tsutil/DbgCtl.h" @@ -242,9 +243,9 @@ class Diags : public DebugInterface IpAddr debug_client_ip; private: - const std::string prefix_str; - mutable ink_mutex tag_table_lock; // prevents reconfig/read races - Regex *activated_tags[2]; // 1 table for debug, 1 for action + const std::string prefix_str; + mutable ink_mutex tag_table_lock; // prevents reconfig/read races + std::shared_ptr activated_tags[2]; // 1 table for debug, 1 for action // These are the default logfile permissions int diags_logfile_perm = -1; diff --git a/src/tscore/Diags.cc b/src/tscore/Diags.cc index 14a1b3436e0..6fe5b79cf8b 100644 --- a/src/tscore/Diags.cc +++ b/src/tscore/Diags.cc @@ -328,19 +328,26 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level, const SourceLocat bool Diags::tag_activated(const char *tag, DiagsTagType mode) const { - bool activated = false; - if (tag == nullptr) { - return (true); + return true; } - lock(); - if (activated_tags[mode]) { - activated = activated_tags[mode]->exec(tag, RE_ANCHORED); + // Snapshot the regex under the lock, then release the lock before running + // Regex::exec(). exec() can lazily construct a thread_local RegexContext + // whose destructor registration via __cxa_thread_atexit_impl takes the + // dynamic loader lock. Holding tag_table_lock across exec() therefore + // creates a lock-order inversion with dlopen() callers that construct a + // DbgCtl during a plugin's static initialization (which takes + // tag_table_lock while already holding the dl loader lock). See the + // analogous fix in DbgCtl::_new_reference. + std::shared_ptr regex; + { + lock(); + regex = activated_tags[mode]; + unlock(); } - unlock(); - return (activated); + return regex ? regex->exec(tag, RE_ANCHORED) : false; } ////////////////////////////////////////////////////////////////////////////// @@ -358,13 +365,15 @@ void Diags::activate_taglist(const char *taglist, DiagsTagType mode) { if (taglist) { - lock(); - if (activated_tags[mode]) { - delete activated_tags[mode]; + // Compile the new regex outside tag_table_lock to avoid holding the lock + // across work that may touch thread_local state (see tag_activated). + auto new_regex = std::make_shared(); + new_regex->compile(taglist); + { + lock(); + activated_tags[mode] = std::move(new_regex); + unlock(); } - activated_tags[mode] = new Regex; - activated_tags[mode]->compile(taglist); - unlock(); } if ((DiagsTagType_Debug == mode) && (this == diags())) { DbgCtl::update([&](const char *tag) -> bool { return tag_activated(tag, DiagsTagType_Debug); }); @@ -385,10 +394,7 @@ void Diags::deactivate_all(DiagsTagType mode) { lock(); - if (activated_tags[mode]) { - delete activated_tags[mode]; - activated_tags[mode] = nullptr; - } + activated_tags[mode].reset(); unlock(); if ((DiagsTagType_Debug == mode) && (this == diags())) { DbgCtl::update([&](const char *tag) -> bool { return tag_activated(tag, DiagsTagType_Debug); });