Counters: fix delayed-detect counter registration

Make sure we register the detect.alerts counter before packet runtime starts
even in delayed detect mode. The registration of new counters at packet
runtime is not supported by the counters api and might lead to crashes as there
is no proper locking to allow for this operation.

This changes how delayed detect works a bit. Now we call the ThreadInit
callback twice. The first call will only register the counter. The 2nd call
will do all the other setup. This way the counter is registered before the
counters api starts operating in the packet runtime.

Fixes the segv reported in ticket #1018.

diff --git a/src/detect-engine.c b/src/detect-engine.c
index 641834b9c0e14d64e1087a3c93613bddf6b0178d..f7b37de31b80d4fc0a162eb634052f38156e9d75 100644 (file)
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -1210,12 +1210,38 @@ static TmEcode ThreadCtxDoInit (DetectEngineCtx *de_ctx, DetectEngineThreadCtx *
     return TM_ECODE_OK;
 }
 
+/** \brief initialize thread specific detection engine context
+ *
+ *  \note there is a special case when using delayed detect. In this case the
+ *        function is called twice per thread. The first time the rules are not
+ *        yet loaded. de_ctx->delayed_detect_initialized will be 0. The 2nd
+ *        time they will be loaded. de_ctx->delayed_detect_initialized will be 1.
+ *        This is needed to do the per thread counter registration before the
+ *        packet runtime starts. In delayed detect mode, the first call will
+ *        return a NULL ptr through the data ptr.
+ *
+ *  \param tv ThreadVars for this thread
+ *  \param initdata pointer to de_ctx
+ *  \param data[out] pointer to store our thread detection ctx
+ *
+ *  \retval TM_ECODE_OK if all went well
+ *  \retval TM_ECODE_FAILED on serious erro
+ */
 TmEcode DetectEngineThreadCtxInit(ThreadVars *tv, void *initdata, void **data)
 {
     DetectEngineCtx *de_ctx = (DetectEngineCtx *)initdata;
     if (de_ctx == NULL)
         return TM_ECODE_FAILED;
 
+    /* first register the counter. In delayed detect mode we exit right after if the
+     * rules haven't been loaded yet. */
+    uint16_t counter_alerts = SCPerfTVRegisterCounter("detect.alert", tv,
+                                                      SC_PERF_TYPE_UINT64, "NULL");
+    if (de_ctx->delayed_detect == 1 && de_ctx->delayed_detect_initialized == 0) {
+        *data = NULL;
+        return TM_ECODE_OK;
+    }
+
     DetectEngineThreadCtx *det_ctx = SCMalloc(sizeof(DetectEngineThreadCtx));
     if (unlikely(det_ctx == NULL))
         return TM_ECODE_FAILED;
@@ -1228,8 +1254,7 @@ TmEcode DetectEngineThreadCtxInit(ThreadVars *tv, void *initdata, void **data)
         return TM_ECODE_FAILED;
 
     /** alert counter setup */
-    det_ctx->counter_alerts = SCPerfTVRegisterCounter("detect.alert", tv,
-                                                      SC_PERF_TYPE_UINT64, "NULL");
+    det_ctx->counter_alerts = counter_alerts;
 
     /* pass thread data back to caller */
     *data = (void *)det_ctx;

diff --git a/src/detect.h b/src/detect.h
index 6733d5eaf0509d13dcf5bd9145c83c03d3d29aba..0129edf593bb735a779614576ee382816609cac8 100644 (file)
--- a/src/detect.h
+++ b/src/detect.h
@@ -719,6 +719,8 @@ typedef struct DetectEngineCtx_ {
 
     /** Is detect engine using a delayed init */
     int delayed_detect;
+    /** Did we load the signatures? */
+    int delayed_detect_initialized;
 
     /** list of keywords that need thread local ctxs */
     DetectEngineThreadKeywordCtxItem *keyword_list;

diff --git a/src/suricata.c b/src/suricata.c
index 070edaa76463815bfcfc0fc5c645ea2ae8892ac4..4084c800628699e8bf7d6354b7400cbdc3cd4220 100644 (file)
--- a/src/suricata.c
+++ b/src/suricata.c
@@ -2139,6 +2139,7 @@ int main(int argc, char **argv)
     if (suri.delayed_detect) {
         if (LoadSignatures(de_ctx, &suri) != TM_ECODE_OK)
             exit(EXIT_FAILURE);
+        de_ctx->delayed_detect_initialized = 1;
         TmThreadActivateDummySlot();
         SCLogNotice("Signature(s) loaded, Detect thread(s) activated.");
     }

diff --git a/src/tm-threads.c b/src/tm-threads.c
index a712b53d276629b6089367d218e8af3645665b58..fa2a8c987e98ae12b7ac641e7ea00244c91a748e 100644 (file)
--- a/src/tm-threads.c
+++ b/src/tm-threads.c
@@ -1063,7 +1063,6 @@ void TmSlotSetFuncAppendDelayed(ThreadVars *tv, TmModule *tm, void *data,
     dslot->SlotFunc = SC_ATOMIC_GET(slot->SlotFunc);
     (void)SC_ATOMIC_SET(slot->SlotFunc, TmDummyFunc);
     dslot->SlotThreadInit = slot->SlotThreadInit;
-    slot->SlotThreadInit = NULL;
     dslot->slot = slot;
 
     TAILQ_INSERT_TAIL(&dummy_slots, dslot, next);