Attachment 227732 Details for Bug 258335 – upstream fix

[patch] upstream fix

suricata-lock.diff (text/plain), 3.55 KB, created by Franco Fichtner on 2021-09-07 11:37:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Franco Fichtner

Created: 2021-09-07 11:37:22 UTC

Size: 3.55 KB

patch

obsolete

>diff --git a/security/suricata/Makefile b/security/suricata/Makefile
>index e450a344652f..d2887b48c134 100644
>--- a/security/suricata/Makefile
>+++ b/security/suricata/Makefile
>@@ -1,5 +1,6 @@
> PORTNAME=	suricata
> DISTVERSION=	6.0.3
>+PORTREVISION=	1
> CATEGORIES=	security
> MASTER_SITES=	https://www.openinfosecfoundation.org/download/
> 
>diff --git a/security/suricata/files/patch-3c53a1601 b/security/suricata/files/patch-3c53a1601
>new file mode 100644
>index 000000000000..d70b3c563e5a
>--- /dev/null
>+++ b/security/suricata/files/patch-3c53a1601
>@@ -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;

Flags:

franco: maintainer-approval+

Actions: View | Diff

Attachments on bug 258335: 227732