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
pull/13734/head
Jeff Lucovsky 4 months ago committed by Victor Julien
parent f910e3045f
commit 06e89736b3

@ -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;
}

Loading…
Cancel
Save