From a44d42b124f5ba82bb2d63828f0122d64fec73fc Mon Sep 17 00:00:00 2001 From: Anoop Saldanha Date: Tue, 30 Jul 2013 19:32:11 +0530 Subject: [PATCH] Fixes segv inside rule swap under low mem conditions. We now gracefully exit rule swap on any allocation or other failures. --- src/detect-engine.c | 131 ++++++++++++++++++++++++++++++++------------ 1 file changed, 95 insertions(+), 36 deletions(-) diff --git a/src/detect-engine.c b/src/detect-engine.c index 1dd22c6cfb..bc5a830338 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -386,6 +386,11 @@ static void *DetectEngineLiveRuleSwap(void *arg) { SCEnter(); + int i = 0; + int no_of_detect_tvs = 0; + DetectEngineCtx *old_de_ctx = NULL; + ThreadVars *tv = NULL; + if (SCSetThreadName("LiveRuleSwap") < 0) { SCLogWarning(SC_ERR_THREAD_INIT, "Unable to set thread name"); } @@ -432,7 +437,49 @@ static void *DetectEngineLiveRuleSwap(void *arg) ConfDump(); #endif + SCMutexLock(&tv_root_lock); + + tv = tv_root[TVT_PPT]; + while (tv) { + /* obtain the slots for this TV */ + TmSlot *slots = tv->tm_slots; + while (slots != NULL) { + TmModule *tm = TmModuleGetById(slots->tm_id); + + if (suricata_ctl_flags != 0) { + TmThreadsSetFlag(tv_local, THV_CLOSED); + + SCLogInfo("===== Live rule swap premature exit, since " + "engine is in shutdown phase ====="); + + UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2); + SCMutexUnlock(&tv_root_lock); + pthread_exit(NULL); + } + + if (!(tm->flags & TM_FLAG_DETECT_TM)) { + slots = slots->slot_next; + continue; + } + no_of_detect_tvs++; + break; + } + + tv = tv->next; + } + + DetectEngineThreadCtx *old_det_ctx[no_of_detect_tvs]; + DetectEngineThreadCtx *new_det_ctx[no_of_detect_tvs]; + ThreadVars *detect_tvs[no_of_detect_tvs]; + + SCMutexUnlock(&tv_root_lock); + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + if (de_ctx == NULL) { + SCLogError(SC_ERR_LIVE_RULE_SWAP, "Allocation failure in live " + "swap. Let's get out of here."); + goto error; + } SCClassConfLoadClassficationConfigFile(de_ctx); SCRConfLoadReferenceConfigFile(de_ctx); @@ -448,10 +495,10 @@ static void *DetectEngineLiveRuleSwap(void *arg) DetectEngineCtxFree(de_ctx); SCLogError(SC_ERR_LIVE_RULE_SWAP, "Failure encountered while " "loading new ruleset with live swap."); - SCLogInfo("===== Live rule swap DONE ====="); + SCLogInfo("===== Live rule swap FAILURE ====="); TmThreadsSetFlag(tv_local, THV_CLOSED); + UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2); pthread_exit(NULL); - return NULL; } SCThresholdConfInitContext(de_ctx, NULL); @@ -460,8 +507,8 @@ static void *DetectEngineLiveRuleSwap(void *arg) SCMutexLock(&tv_root_lock); - int no_of_detect_tvs = 0; - ThreadVars *tv = tv_root[TVT_PPT]; + /* all receive threads are part of packet processing threads */ + tv = tv_root[TVT_PPT]; while (tv) { /* obtain the slots for this TV */ TmSlot *slots = tv->tm_slots; @@ -484,51 +531,51 @@ static void *DetectEngineLiveRuleSwap(void *arg) continue; } - no_of_detect_tvs++; - - slots = slots->slot_next; + old_det_ctx[i] = SC_ATOMIC_GET(slots->slot_data); + detect_tvs[i] = tv; + TmEcode r = DetectEngineThreadCtxInitForLiveRuleSwap(tv, (void *)de_ctx, + (void **)&new_det_ctx[i]); + i++; + if (r == TM_ECODE_FAILED) { + SCLogError(SC_ERR_LIVE_RULE_SWAP, "Detect engine thread init " + "failure in live rule swap. Let's get out of here"); + SCMutexUnlock(&tv_root_lock); + goto error; + } + SCLogDebug("live rule swap created new det_ctx - %p and de_ctx " + "- %p\n", new_det_ctx, de_ctx); + break; } tv = tv->next; } - DetectEngineThreadCtx *old_det_ctx[no_of_detect_tvs]; - DetectEngineThreadCtx *new_det_ctx[no_of_detect_tvs]; - ThreadVars *detect_tvs[no_of_detect_tvs]; - - /* all receive threads are part of packet processing threads */ + i = 0; tv = tv_root[TVT_PPT]; - int i = 0; while (tv) { - /* obtain the slots for this TV */ TmSlot *slots = tv->tm_slots; while (slots != NULL) { - TmModule *tm = TmModuleGetById(slots->tm_id); + if (suricata_ctl_flags != 0) { + TmThreadsSetFlag(tv_local, THV_CLOSED); + + SCLogInfo("===== Live rule swap premature exit, since " + "engine is in shutdown phase ====="); + UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2); + SCMutexUnlock(&tv_root_lock); + pthread_exit(NULL); + } + + TmModule *tm = TmModuleGetById(slots->tm_id); if (!(tm->flags & TM_FLAG_DETECT_TM)) { slots = slots->slot_next; continue; } - - old_det_ctx[i] = SC_ATOMIC_GET(slots->slot_data); - detect_tvs[i] = tv; - - DetectEngineThreadCtx *det_ctx = NULL; - DetectEngineThreadCtxInitForLiveRuleSwap(tv, (void *)de_ctx, - (void **)&det_ctx); - SCLogDebug("live rule swap done with new det_ctx - %p and de_ctx " - "- %p\n", det_ctx, de_ctx); - - new_det_ctx[i] = det_ctx; - i++; - - SCLogDebug("swapping new det_ctx - %p with older one - %p", det_ctx, - SC_ATOMIC_GET(slots->slot_data)); - (void)SC_ATOMIC_SET(slots->slot_data, det_ctx); - - slots = slots->slot_next; + SCLogDebug("swapping new det_ctx - %p with older one - %p", + new_det_ctx[i], SC_ATOMIC_GET(slots->slot_data)); + (void)SC_ATOMIC_SET(slots->slot_data, new_det_ctx[i++]); + break; } - tv = tv->next; } @@ -599,7 +646,7 @@ static void *DetectEngineLiveRuleSwap(void *arg) } /* free all the ctxs */ - DetectEngineCtx *old_de_ctx = old_det_ctx[0]->de_ctx; + old_de_ctx = old_det_ctx[0]->de_ctx; for (i = 0; i < no_of_detect_tvs; i++) { SCLogDebug("Freeing old_det_ctx - %p used by detect", old_det_ctx[i]); @@ -617,7 +664,17 @@ static void *DetectEngineLiveRuleSwap(void *arg) SCLogInfo("===== Live rule swap DONE ====="); pthread_exit(NULL); - return NULL; + + error: + for (i = 0; i < no_of_detect_tvs; i++) { + if (new_det_ctx[i] != NULL) + DetectEngineThreadCtxDeinit(NULL, new_det_ctx[i]); + } + DetectEngineCtxFree(de_ctx); + TmThreadsSetFlag(tv_local, THV_CLOSED); + UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2); + SCLogInfo("===== Live rule swap FAILURE ====="); + pthread_exit(NULL); } void DetectEngineSpawnLiveRuleSwapMgmtThread(void) @@ -1187,6 +1244,8 @@ TmEcode DetectEngineThreadCtxInit(ThreadVars *tv, void *initdata, void **data) */ static TmEcode DetectEngineThreadCtxInitForLiveRuleSwap(ThreadVars *tv, void *initdata, void **data) { + *data = NULL; + DetectEngineCtx *de_ctx = (DetectEngineCtx *)initdata; if (de_ctx == NULL) return TM_ECODE_FAILED;