flow/bypass: use_cnt desync'd on bypassed flows

Locally bypassed flows had unsafe updates to `Flow::use_cnt` leading to a race
issue. For a packet it would do the flow lookup, attach the flow to the packet,
increment the `use_cnt`. Then it would detect that the flow is in the bypass
state, and unlock it while holding a reference (so alos not decrementing the
`use_cnt`). When the packet was then returned to the packet pool, the flow would
be disconnected from the packet, which would decrement `use_cnt` without holding
the flow lock.

This patch addresses this issue by disconnecting the flow from the packet
immediately when the bypassed state is detected. This moves the `use_cnt`
decrement to within the lock.

Bug: #4766.

diff --git a/src/flow-worker.c b/src/flow-worker.c
index ba7b956a4b1b5d584cc67b0c8b0bb6eb57edfadc..fbad5671427ffa3d6480eaf906ce72a661b6d183 100644 (file)
--- a/src/flow-worker.c
+++ b/src/flow-worker.c
@@ -217,15 +217,23 @@ static inline TmEcode FlowUpdate(ThreadVars *tv, FlowWorkerThreadData *fw, Packe
     int state = p->flow->flow_state;
     switch (state) {
 #ifdef CAPTURE_OFFLOAD
-        case FLOW_STATE_CAPTURE_BYPASSED:
+        case FLOW_STATE_CAPTURE_BYPASSED: {
             StatsAddUI64(tv, fw->both_bypass_pkts, 1);
             StatsAddUI64(tv, fw->both_bypass_bytes, GET_PKT_LEN(p));
+            Flow *f = p->flow;
+            FlowDeReference(&p->flow);
+            FLOWLOCK_UNLOCK(f);
             return TM_ECODE_DONE;
+        }
 #endif
-        case FLOW_STATE_LOCAL_BYPASSED:
+        case FLOW_STATE_LOCAL_BYPASSED: {
             StatsAddUI64(tv, fw->local_bypass_pkts, 1);
             StatsAddUI64(tv, fw->local_bypass_bytes, GET_PKT_LEN(p));
+            Flow *f = p->flow;
+            FlowDeReference(&p->flow);
+            FLOWLOCK_UNLOCK(f);
             return TM_ECODE_DONE;
+        }
         default:
             return TM_ECODE_OK;
     }
@@ -501,7 +509,6 @@ static TmEcode FlowWorker(ThreadVars *tv, Packet *p, void *data)
         if (likely(p->flow != NULL)) {
             DEBUG_ASSERT_FLOW_LOCKED(p->flow);
             if (FlowUpdate(tv, fw, p) == TM_ECODE_DONE) {
-                FLOWLOCK_UNLOCK(p->flow);
                 return TM_ECODE_OK;
             }
         }