From f2e3a2e2a720d48fd2e09e52ae9dcb1237ba7119 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 30 Sep 2026 23:44:07 +0000 Subject: [PATCH] flow: also do final flow reassembly for flows stranded on the evicted list PostRunDeinit stops the flow manager, then runs the final flow sweeps. TCP flows that a foreign thread times out during those sweeps are parked on the flow hash row's evicted list for the manager to pick up, but the manager is gone. FlowRemoveHash only walked the row's head list, so those flows were later recycled without FlowFinish, skipping the final reassembly and pseudo packet pass. Any app layer tx that only completes at flow end is then lost: typically a response arriving after the peer's FIN and never ACKed, as that data only becomes usable at eof in non-inline mode. The result is an intermittent count mismatch in the suricata-verify suite, always exactly one short, e.g. "expected 28 matches; got 27" for http-sticky-server and "expected 7 matches; got 6" for smb2-ntlmssp-negotiateflags. The flow event is still logged, with reason "shutdown", so the flow is not lost, only its tail transactions. The lost records can be spotted in a reference eve.json by "pkt_src": "stream (flow timeout)" and no pcap_cnt. Stranding a flow needs a hash row collision plus a foreign thread sweep inside the [last manager sweep, manager stopped] window, and the row mapping is seeded with a per process random value (flow.c hash_rand). That makes it a per run lottery at the default 65536 rows, and deterministic with a small flow.hash-size such as 12 or 20. Unit tests cannot cover it: FlowBelongsToUs returns true in unittests runmode, so the evicted list is never populated there. Also sweep the evicted list, with the same decision point as the head walk: FlowNeedsReassembly sets ffr_ts/ffr_tc, which CheckWorkQueue requires before running FlowFinish. CheckWorkQueue applies its own bypass and drop guards, so a bypassed flow may now take a worker round trip and have its flow event logged there rather than by the recycler. The evicted list has never been swept on this path. Fixes: 244dd11c3415 ("flow/manager: fix flows not evicted & freed in time") Ticket: #9146. --- src/flow-timeout.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/flow-timeout.c b/src/flow-timeout.c index 48cfc08236..f1d6ccfc97 100644 --- a/src/flow-timeout.c +++ b/src/flow-timeout.c @@ -356,6 +356,19 @@ void FlowSendToLocalThread(Flow *f) TmThreadsInjectFlowById(f, (const int)f->thread_id[idx]); } +/** \note f->fb must be locked; cannot reuse RemoveFromHash, as that unlinks + * from the row's head list. */ +static inline void RemoveFromEvictedList(FlowBucket *fb, Flow *f, Flow *prev_f) +{ + if (prev_f != NULL) { + prev_f->next = f->next; + } else { + fb->evicted = f->next; + } + f->next = NULL; + f->fb = NULL; +} + /** * \internal * \brief Remove flows from the hash bucket as they have more work to be done in @@ -412,6 +425,35 @@ static inline void FlowRemoveHash(void) prev_f = f; f = f->next; } + + /* MoveToWorkQueue parks TCP flows it must not handle itself on the + * row's evicted list for the flow manager to pick up. The manager is + * stopped before these sweeps run, so the list is never drained + * afterwards: sweep it here or the flows get recycled without their + * final reassembly work, losing any app layer tx that only completes + * at flow end along with its alerts. */ + f = fb->evicted; + prev_f = NULL; + while (f != NULL) { + Flow *next_f = f->next; + + FLOWLOCK_WRLOCK(f); + + TcpSession *ssn = (TcpSession *)f->protoctx; + if (ssn != NULL && FlowNeedsReassembly(f)) { + RemoveFromEvictedList(fb, f, prev_f); + f->flow_end_flags |= FLOW_END_FLAG_SHUTDOWN; + FlowSendToLocalThread(f); + FLOWLOCK_UNLOCK(f); + f = next_f; + continue; + } + + FLOWLOCK_UNLOCK(f); + + prev_f = f; + f = next_f; + } FBLOCK_UNLOCK(fb); } }