Fix segv conditions caused by broken flow cleanup code.

src/detect-engine-state.c

@@ -261,12 +261,12 @@ static void DeStateSignatureAppend(DetectEngineState *state, Signature *s, SigMa
 int DeStateFlowHasState(Flow *f) {
     SCEnter();
     int r = 0;
-    SCMutexLock(&f->m);
+    SCMutexLock(&f->de_state_m);
     if (f->de_state == NULL || f->de_state->cnt == 0)
         r = 0;
     else
         r = 1;
-    SCMutexUnlock(&f->m);
+    SCMutexUnlock(&f->de_state_m);
     SCReturnInt(r);
 }
 
@@ -364,7 +364,6 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
             sm, umatch, dmatch);
 
     SCMutexLock(&f->de_state_m);
-    SCMutexLock(&f->m);
     /* match or no match, we store the state anyway
      * "sm" here is either NULL (complete match) or
      * the last SigMatch that didn't match */
@@ -375,7 +374,6 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
         DeStateSignatureAppend(f->de_state, s, sm, umatch, dmatch);
     }
 
-    SCMutexUnlock(&f->m);
     SCMutexUnlock(&f->de_state_m);
 
     SCReturnInt(r);
@@ -543,13 +541,11 @@ int DeStateRestartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx, DetectEngin
 
     /* first clear the existing state as it belongs
      * to the previous transaction */
-    SCMutexLock(&f->m);
     SCMutexLock(&f->de_state_m);
     if (f->de_state != NULL) {
         DetectEngineStateReset(f->de_state);
     }
     SCMutexUnlock(&f->de_state_m);
-    SCMutexUnlock(&f->m);
 
     SCReturnInt(0);
 }

src/flow-hash.c

@@ -348,7 +348,7 @@ static Flow *FlowGetNew(Packet *p) {
 
         /* flow is initialized but *unlocked* */
     } else {
-        FLOW_RECYCLE(f);
+        /* flow has been recycled before it went into the spare queue */
 
         /* flow is initialized (recylced) but *unlocked* */
     }

src/flow-util.h

@@ -53,11 +53,12 @@
         (f)->tag_list = NULL; \
     } while (0)
 
+/** \brief macro to recycle a flow before it goes into the spare queue for reuse.
+ *
+ *  Note that the lnext, lprev, hnext, hprev fields are untouched, those are
+ *  managed by the queueing code. Same goes for fb (FlowBucket ptr) field.
+ */
 #define FLOW_RECYCLE(f) do { \
-        (f)->lnext = NULL; \
-        (f)->lprev = NULL; \
-        (f)->hnext = NULL; \
-        (f)->hprev = NULL; \
         (f)->sp = 0; \
         (f)->dp = 0; \
         (f)->flags = 0; \
@@ -70,18 +71,18 @@
         (f)->flowvar = NULL; \
         (f)->protoctx = NULL; \
         SC_ATOMIC_RESET((f)->use_cnt); \
-        SCMutexLock(&(f)->de_state_m); \
         if ((f)->de_state != NULL) { \
             DetectEngineStateReset((f)->de_state); \
+            (f)->de_state = NULL; \
         } \
-        (f)->de_state = NULL; \
-        SCMutexUnlock(&(f)->de_state_m); \
         (f)->sgh_toserver = NULL; \
         (f)->sgh_toclient = NULL; \
         AppLayerParserCleanupState(f); \
         FlowL7DataPtrFree(f); \
-        SCFree((f)->aldata); \
-        (f)->aldata = NULL; \
+        if ((f)->aldata != NULL) { \
+            SCFree((f)->aldata); \
+            (f)->aldata = NULL; \
+        } \
         (f)->alflags = 0; \
         (f)->alproto = 0; \
         DetectTagDataListFree((f)->tag_list); \
@@ -90,21 +91,21 @@
 
 #define FLOW_DESTROY(f) do { \
         SCMutexDestroy(&(f)->m); \
+        SCMutexDestroy(&(f)->de_state_m); \
         GenericVarFree((f)->flowvar); \
         (f)->flowvar = NULL; \
         (f)->protoctx = NULL; \
         SC_ATOMIC_DESTROY((f)->use_cnt); \
-        SCMutexLock(&(f)->de_state_m); \
         if ((f)->de_state != NULL) { \
             DetectEngineStateFree((f)->de_state); \
         } \
         (f)->de_state = NULL; \
-        SCMutexUnlock(&(f)->de_state_m); \
-        SCMutexDestroy(&(f)->de_state_m); \
         AppLayerParserCleanupState(f); \
         FlowL7DataPtrFree(f); \
-        SCFree((f)->aldata); \
-        (f)->aldata = NULL; \
+        if ((f)->aldata != NULL) { \
+            SCFree((f)->aldata); \
+            (f)->aldata = NULL; \
+        } \
         (f)->alflags = 0; \
         (f)->alproto = 0; \
         DetectTagDataListFree((f)->tag_list); \

src/flow.c

@@ -1351,7 +1351,7 @@ static int FlowClearMemory(Flow* f, uint8_t proto_map) {
         flow_proto[proto_map].Freefunc(f->protoctx);
     }
 
-    FLOW_DESTROY(f);
+    FLOW_RECYCLE(f);
 
     SCReturnInt(1);
 }