From 7ebd1e6433bfdf7c66d70156f82ad4940a186c70 Mon Sep 17 00:00:00 2001
From: Victor Julien <victor@inliniac.net>
Date: Thu, 31 Oct 2013 11:39:59 +0100
Subject: [PATCH] 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.
---
 src/detect-engine.c | 29 +++++++++++++++++++++++++++--
 src/detect.h        |  2 ++
 src/suricata.c      |  1 +
 src/tm-threads.c    |  1 -
 4 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/detect-engine.c b/src/detect-engine.c
index 641834b9c0..f7b37de31b 100644
--- 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 6733d5eaf0..0129edf593 100644
--- 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 070edaa764..4084c80062 100644
--- 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 a712b53d27..fa2a8c987e 100644
--- 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);