From ec7e0561e8356371c7ec1c2b285f267424558f81 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 20 Oct 2021 13:20:32 +0200 Subject: [PATCH] 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. --- src/flow-worker.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/flow-worker.c b/src/flow-worker.c index ba7b956a4b..fbad567142 100644 --- 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; } }