security/suricata: Add patch for upstream locking fix
https://redmine.openinfosecfoundation.org/issues/4478 - Suricata 6 may stop forwarding traffic due to lock/unlock executed between CPUs, which is undetermined behaviour. PR: 258335 Approved by: Franco Fichtner (maintainer)
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
PORTNAME= suricata
|
||||
DISTVERSION= 6.0.3
|
||||
PORTREVISION= 1
|
||||
CATEGORIES= security
|
||||
MASTER_SITES= https://www.openinfosecfoundation.org/download/
|
||||
|
||||
|
||||
78
security/suricata/files/patch-3c53a1601
Normal file
78
security/suricata/files/patch-3c53a1601
Normal file
@@ -0,0 +1,78 @@
|
||||
From 3c53a1601b6f861f8b7f0cd0984b18e78291fe85 Mon Sep 17 00:00:00 2001
|
||||
From: Victor Julien <victor@inliniac.net>
|
||||
Date: Wed, 18 Aug 2021 20:14:48 +0200
|
||||
Subject: [PATCH] threading: don't pass locked flow between threads
|
||||
|
||||
Previously the flow manager would share evicted flows with the workers
|
||||
while keeping the flows mutex locked. This reduced the number of unlock/
|
||||
lock cycles while there was guaranteed to be no contention.
|
||||
|
||||
This turns out to be undefined behavior. A lock is supposed to be locked
|
||||
and unlocked from the same thread. It appears that FreeBSD is stricter on
|
||||
this than Linux.
|
||||
|
||||
This patch addresses the issue by unlocking before handing a flow off
|
||||
to another thread, and locking again from the new thread.
|
||||
|
||||
Issue was reported and largely analyzed by Bill Meeks.
|
||||
|
||||
Bug: #4478
|
||||
(cherry picked from commit 9551cd05357925e8bec8e0030d5f98fd07f17839)
|
||||
---
|
||||
src/flow-hash.c | 1 +
|
||||
src/flow-manager.c | 2 +-
|
||||
src/flow-timeout.c | 1 +
|
||||
src/flow-worker.c | 1 +
|
||||
4 files changed, 4 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/flow-hash.c b/src/flow-hash.c
|
||||
index ebbd836e81a..760bc53e0a8 100644
|
||||
--- src/flow-hash.c
|
||||
+++ src/flow-hash.c
|
||||
@@ -669,6 +669,7 @@ static inline void MoveToWorkQueue(ThreadVars *tv, FlowLookupStruct *fls,
|
||||
f->fb = NULL;
|
||||
f->next = NULL;
|
||||
FlowQueuePrivateAppendFlow(&fls->work_queue, f);
|
||||
+ FLOWLOCK_UNLOCK(f);
|
||||
} else {
|
||||
/* implied: TCP but our thread does not own it. So set it
|
||||
* aside for the Flow Manager to pick it up. */
|
||||
diff --git a/src/flow-manager.c b/src/flow-manager.c
|
||||
index d58a49637d6..9228c88490c 100644
|
||||
--- src/flow-manager.c
|
||||
+++ src/flow-manager.c
|
||||
@@ -333,9 +333,9 @@ static uint32_t ProcessAsideQueue(FlowManagerTimeoutThread *td, FlowTimeoutCount
|
||||
FlowForceReassemblyNeedReassembly(f) == 1)
|
||||
{
|
||||
FlowForceReassemblyForFlow(f);
|
||||
+ FLOWLOCK_UNLOCK(f);
|
||||
/* flow ownership is passed to the worker thread */
|
||||
|
||||
- /* flow remains locked */
|
||||
counters->flows_aside_needs_work++;
|
||||
continue;
|
||||
}
|
||||
diff --git a/src/flow-timeout.c b/src/flow-timeout.c
|
||||
index 972b35076bd..d6cca490087 100644
|
||||
--- src/flow-timeout.c
|
||||
+++ src/flow-timeout.c
|
||||
@@ -401,6 +401,7 @@ static inline void FlowForceReassemblyForHash(void)
|
||||
RemoveFromHash(f, prev_f);
|
||||
f->flow_end_flags |= FLOW_END_FLAG_SHUTDOWN;
|
||||
FlowForceReassemblyForFlow(f);
|
||||
+ FLOWLOCK_UNLOCK(f);
|
||||
f = next_f;
|
||||
continue;
|
||||
}
|
||||
diff --git a/src/flow-worker.c b/src/flow-worker.c
|
||||
index 69dbb6ac575..dccf3581dd5 100644
|
||||
--- src/flow-worker.c
|
||||
+++ src/flow-worker.c
|
||||
@@ -168,6 +168,7 @@ static void CheckWorkQueue(ThreadVars *tv, FlowWorkerThreadData *fw,
|
||||
{
|
||||
Flow *f;
|
||||
while ((f = FlowQueuePrivateGetFromTop(fq)) != NULL) {
|
||||
+ FLOWLOCK_WRLOCK(f);
|
||||
f->flow_end_flags |= FLOW_END_FLAG_TIMEOUT; //TODO emerg
|
||||
|
||||
const FlowStateType state = f->flow_state;
|
||||
Reference in New Issue
Block a user