flow: improve thread safety during timeout checks

Timeout checks would access certain fields w/o locking, which could lead
to thread safety issues.

diff --git a/src/flow-hash.c b/src/flow-hash.c
index bdcc0dc440061d46a89c0f9322f674e45f7c9e4b..95f55b9e0a474f15021cce3066c3f2ccf2219007 100644 (file)
--- a/src/flow-hash.c
+++ b/src/flow-hash.c
@@ -906,16 +906,15 @@ Flow *FlowGetFlowFromHash(ThreadVars *tv, FlowLookupStruct *fls, Packet *p, Flow
     f = fb->head;
     do {
         Flow *next_f = NULL;
+        FLOWLOCK_WRLOCK(f);
         const bool timedout = (fb_nextts < (uint32_t)SCTIME_SECS(p->ts) &&
                                FlowIsTimedOut(f, (uint32_t)SCTIME_SECS(p->ts), emerg));
         if (timedout) {
-            FLOWLOCK_WRLOCK(f);
             next_f = f->next;
             MoveToWorkQueue(tv, fls, fb, f, prev_f);
             FLOWLOCK_UNLOCK(f);
             goto flow_removed;
         } else if (FlowCompare(f, p) != 0) {
-            FLOWLOCK_WRLOCK(f);
             /* found a matching flow that is not timed out */
             if (unlikely(TcpSessionPacketSsnReuse(p, f, f->protoctx))) {
                 Flow *new_f = TcpReuseReplace(tv, fls, fb, f, hash, p);
@@ -933,6 +932,8 @@ Flow *FlowGetFlowFromHash(ThreadVars *tv, FlowLookupStruct *fls, Packet *p, Flow
             FlowReference(dest, f);
             FBLOCK_UNLOCK(fb);
             return f; /* return w/o releasing flow lock */
+        } else {
+            FLOWLOCK_UNLOCK(f);
         }
         /* unless we removed 'f', prev_f needs to point to
          * current 'f' when adding a new flow below. */

diff --git a/src/flow-manager.c b/src/flow-manager.c
index 4c6f165888d74030bd81ee74b12d9723bfe32ad8..99348fc40413ee279b69cd695c005e8be08fc791 100644 (file)
--- a/src/flow-manager.c
+++ b/src/flow-manager.c
@@ -327,6 +327,8 @@ static void FlowManagerHashRowTimeout(FlowManagerTimeoutThread *td, Flow *f, SCT
     do {
         checked++;
 
+        FLOWLOCK_WRLOCK(f);
+
         /* check flow timeout based on lastts and state. Both can be
          * accessed w/o Flow lock as we do have the hash row lock (so flow
          * can't disappear) and flow_state is atomic. lastts can only
@@ -334,6 +336,7 @@ static void FlowManagerHashRowTimeout(FlowManagerTimeoutThread *td, Flow *f, SCT
 
         /* timeout logic goes here */
         if (FlowManagerFlowTimeout(f, ts, next_ts, emergency) == false) {
+            FLOWLOCK_UNLOCK(f);
             counters->flows_notimeout++;
 
             prev_f = f;
@@ -341,8 +344,6 @@ static void FlowManagerHashRowTimeout(FlowManagerTimeoutThread *td, Flow *f, SCT
             continue;
         }
 
-        FLOWLOCK_WRLOCK(f);
-
         Flow *next_flow = f->next;
 
 #ifdef CAPTURE_OFFLOAD