detect/mt: Prevent deadlock when adding tenants

This commit modifies the call path for registering MT tenants to avoid
deadlocks on the master->lock

When performing tenant operations, e.g., using suricatasc to send a
register-tenant command, a deadlock occurs when

- DetectEngineMTApply: acquires master->lock
- Calls DetectEngineReloadThreads
- Within DetectEngineReloadThreads, calls DetectEngineMultiTenantEnabled
- Which first acquires master->lock

Commit 2bea5af introduced changes to the master->lock usage leading to
the deadlock situation.

Issue: 7819

diff --git a/src/detect-engine.c b/src/detect-engine.c
index f8e07885829c47a6294a316890fdacb641f21451..408e90bc89f8dbbefbdfa7224ce48e98f3a0933e 100644 (file)
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -106,6 +106,7 @@ static uint32_t DetectEngineTenantGetIdFromLivedev(const void *ctx, const Packet
 static uint32_t DetectEngineTenantGetIdFromVlanId(const void *ctx, const Packet *p);
 static uint32_t DetectEngineTenantGetIdFromPcap(const void *ctx, const Packet *p);
 
+static bool DetectEngineMultiTenantEnabledWithLock(void);
 static DetectEngineAppInspectionEngine *g_app_inspect_engines = NULL;
 static DetectEnginePktInspectionEngine *g_pkt_inspect_engines = NULL;
 static DetectEngineFrameInspectionEngine *g_frame_inspect_engines = NULL;
@@ -3153,7 +3154,6 @@ static void DetectEngineThreadCtxDeinitKeywords(DetectEngineCtx *de_ctx, DetectE
 static TmEcode DetectEngineThreadCtxInitForMT(ThreadVars *tv, DetectEngineThreadCtx *det_ctx)
 {
     DetectEngineMasterCtx *master = &g_master_de_ctx;
-    SCMutexLock(&master->lock);
 
     DetectEngineTenantMapping *map_array = NULL;
     uint32_t map_array_size = 0;
@@ -3164,7 +3164,6 @@ static TmEcode DetectEngineThreadCtxInitForMT(ThreadVars *tv, DetectEngineThread
     if (master->tenant_selector == TENANT_SELECTOR_UNKNOWN) {
         SCLogError("no tenant selector set: "
                    "set using multi-detect.selector");
-        SCMutexUnlock(&master->lock);
         return TM_ECODE_FAILED;
     }
 
@@ -3258,7 +3257,6 @@ static TmEcode DetectEngineThreadCtxInitForMT(ThreadVars *tv, DetectEngineThread
             break;
     }
 
-    SCMutexUnlock(&master->lock);
     return TM_ECODE_OK;
 error:
     if (map_array != NULL)
@@ -3266,7 +3264,6 @@ error:
     if (mt_det_ctxs_hash != NULL)
         HashTableFree(mt_det_ctxs_hash);
 
-    SCMutexUnlock(&master->lock);
     return TM_ECODE_FAILED;
 }
 
@@ -3429,10 +3426,14 @@ TmEcode DetectEngineThreadCtxInit(ThreadVars *tv, void *initdata, void **data)
 #endif
 
     if (DetectEngineMultiTenantEnabled()) {
+        DetectEngineMasterCtx *master = &g_master_de_ctx;
+        SCMutexLock(&master->lock);
         if (DetectEngineThreadCtxInitForMT(tv, det_ctx) != TM_ECODE_OK) {
             DetectEngineThreadCtxDeinit(tv, det_ctx);
+            SCMutexUnlock(&master->lock);
             return TM_ECODE_FAILED;
         }
+        SCMutexUnlock(&master->lock);
     }
 
     /* pass thread data back to caller */
@@ -3485,7 +3486,7 @@ DetectEngineThreadCtx *DetectEngineThreadCtxInitForReload(
     det_ctx->counter_match_list = StatsRegisterAvgCounter("detect.match_list", tv);
 #endif
 
-    if (mt && DetectEngineMultiTenantEnabled()) {
+    if (mt && DetectEngineMultiTenantEnabledWithLock()) {
         if (DetectEngineThreadCtxInitForMT(tv, det_ctx) != TM_ECODE_OK) {
             DetectEngineDeReference(&det_ctx->de_ctx);
             SCFree(det_ctx);
@@ -3865,11 +3866,17 @@ DetectEngineCtx *DetectEngineReference(DetectEngineCtx *de_ctx)
     return de_ctx;
 }
 
+static bool DetectEngineMultiTenantEnabledWithLock(void)
+{
+    DetectEngineMasterCtx *master = &g_master_de_ctx;
+    return master->multi_tenant_enabled;
+}
+
 bool DetectEngineMultiTenantEnabled(void)
 {
     DetectEngineMasterCtx *master = &g_master_de_ctx;
     SCMutexLock(&master->lock);
-    bool enabled = master->multi_tenant_enabled;
+    bool enabled = DetectEngineMultiTenantEnabledWithLock();
     SCMutexUnlock(&master->lock);
     return enabled;
 }