threading: fix shutdown of IPS autofp modes

For IPS modes with a verdict thread in autofp there was an issue with
the verdict thread not shutting down, leading to a long shutdown time
until an error condition was reached.

The problem was that when the packet threads, of which the verdict
thread is one, were told to enter their flow timeout loop the verdict
thread got stuck as it immediately progressed to THV_RUNNING_DONE
instead of the expected THV_FLOW_LOOP.

This patch updates the shutdown logic to only apply the flow timeout
logic to the relevant threads, and skip the verdict thread(s).

Add TM_FLAG_VERDICT_TM to indicate a thread has a verdict module to more
explicitly shut it down.

Fixes: 12f8f03532e5 ("threads: fix autofp shutdown race condition")
Bug: #7681.
(cherry picked from commit bdac028fc7ea97f0e50fbd0a24d995bced81c490)

diff --git a/src/source-ipfw.c b/src/source-ipfw.c
index 7f31de71a84aeb85115fda104d29f1b30d6d849e..1a32eaef93f650019aa8449fef974f5bac040436 100644 (file)
--- a/src/source-ipfw.c
+++ b/src/source-ipfw.c
@@ -76,6 +76,7 @@ void TmModuleVerdictIPFWRegister (void)
     tmm_modules[TMM_VERDICTIPFW].Func = NULL;
     tmm_modules[TMM_VERDICTIPFW].ThreadExitPrintStats = NULL;
     tmm_modules[TMM_VERDICTIPFW].ThreadDeinit = NULL;
+    tmm_modules[TMM_VERDICTIPFW].flags = TM_FLAG_VERDICT_TM;
 }
 
 void TmModuleDecodeIPFWRegister (void)
@@ -181,6 +182,7 @@ void TmModuleVerdictIPFWRegister (void)
     tmm_modules[TMM_VERDICTIPFW].ThreadDeinit = VerdictIPFWThreadDeinit;
     tmm_modules[TMM_VERDICTIPFW].cap_flags = SC_CAP_NET_ADMIN | SC_CAP_NET_RAW |
                                              SC_CAP_NET_BIND_SERVICE; /** \todo untested */
+    tmm_modules[TMM_VERDICTIPFW].flags = TM_FLAG_VERDICT_TM;
 }
 
 /**

diff --git a/src/source-nfq.c b/src/source-nfq.c
index 5293f2529a4cb5fd628bc72ffaf78aa0ca0952ee..13eb9365853525735d18d327e8f220ab7724eeaa 100644 (file)
--- a/src/source-nfq.c
+++ b/src/source-nfq.c
@@ -75,6 +75,7 @@ void TmModuleVerdictNFQRegister (void)
     tmm_modules[TMM_VERDICTNFQ].ThreadExitPrintStats = NULL;
     tmm_modules[TMM_VERDICTNFQ].ThreadDeinit = NULL;
     tmm_modules[TMM_VERDICTNFQ].cap_flags = SC_CAP_NET_ADMIN;
+    tmm_modules[TMM_VERDICTNFQ].flags = TM_FLAG_VERDICT_TM;
 }
 
 void TmModuleDecodeNFQRegister (void)
@@ -187,6 +188,7 @@ void TmModuleVerdictNFQRegister (void)
     tmm_modules[TMM_VERDICTNFQ].ThreadInit = VerdictNFQThreadInit;
     tmm_modules[TMM_VERDICTNFQ].Func = VerdictNFQ;
     tmm_modules[TMM_VERDICTNFQ].ThreadDeinit = VerdictNFQThreadDeinit;
+    tmm_modules[TMM_VERDICTNFQ].flags = TM_FLAG_VERDICT_TM;
 }
 
 void TmModuleDecodeNFQRegister (void)

diff --git a/src/source-windivert.c b/src/source-windivert.c
index 347d2e7a0f2dc623c21dddb6b134e85e01ed4771..bd9f5fe7ef18d3a756febc0c1a6915a7521df88f 100644 (file)
--- a/src/source-windivert.c
+++ b/src/source-windivert.c
@@ -69,6 +69,7 @@ void TmModuleVerdictWinDivertRegister(void)
 {
     tmm_modules[TMM_VERDICTWINDIVERT].name = "VerdictWinDivert";
     tmm_modules[TMM_VERDICTWINDIVERT].ThreadInit = NoWinDivertSupportExit;
+    tmm_modules[TMM_VERDICTWINDIVERT].flags = TM_FLAG_VERDICT_TM;
 }
 
 void TmModuleDecodeWinDivertRegister(void)
@@ -382,6 +383,7 @@ void TmModuleVerdictWinDivertRegister(void)
     tm_ptr->ThreadInit = VerdictWinDivertThreadInit;
     tm_ptr->Func = VerdictWinDivert;
     tm_ptr->ThreadDeinit = VerdictWinDivertThreadDeinit;
+    tm_ptr->flags = TM_FLAG_VERDICT_TM;
 }
 
 void TmModuleDecodeWinDivertRegister(void)

diff --git a/src/suricata.c b/src/suricata.c
index 0afb332d61f2012fcb8cefcc1645f243d22319e9..7b1854128540206bb77b508294213905eeb05913 100644 (file)
--- a/src/suricata.c
+++ b/src/suricata.c
@@ -2277,12 +2277,13 @@ void PostRunDeinit(const int runmode, struct timeval *start_time)
     FlowDisableFlowManagerThread();
     /* disable capture */
     TmThreadDisableReceiveThreads();
-    /* tell packet threads to enter flow timeout loop */
-    TmThreadDisablePacketThreads(THV_REQ_FLOW_LOOP, THV_FLOW_LOOP);
+    /* tell relevant packet threads to enter flow timeout loop */
+    TmThreadDisablePacketThreads(
+            THV_REQ_FLOW_LOOP, THV_FLOW_LOOP, (TM_FLAG_RECEIVE_TM | TM_FLAG_DETECT_TM));
     /* run cleanup on the flow hash */
     FlowForceReassembly();
-    /* gracefully shut down packet threads */
-    TmThreadDisablePacketThreads(THV_KILL, THV_RUNNING_DONE);
+    /* gracefully shut down all packet threads */
+    TmThreadDisablePacketThreads(THV_KILL, THV_RUNNING_DONE, TM_FLAG_PACKET_ALL);
     SCPrintElapsedTime(start_time);
     FlowDisableFlowRecyclerThread();
 

diff --git a/src/tm-modules.h b/src/tm-modules.h
index 4642ff46a6ca4afd618eb761b438e891287055c4..2e2e70ce65c17ec6fef77ad422638978285eba7e 100644 (file)
--- a/src/tm-modules.h
+++ b/src/tm-modules.h
@@ -35,6 +35,12 @@
 #define TM_FLAG_LOGAPI_TM       0x10 /**< TM is run by Log API */
 #define TM_FLAG_MANAGEMENT_TM   0x20
 #define TM_FLAG_COMMAND_TM      0x40
+#define TM_FLAG_VERDICT_TM      0x80
+
+/* all packet modules combined */
+#define TM_FLAG_PACKET_ALL                                                                         \
+    (TM_FLAG_RECEIVE_TM | TM_FLAG_DECODE_TM | TM_FLAG_STREAM_TM | TM_FLAG_DETECT_TM |              \
+            TM_FLAG_VERDICT_TM)
 
 typedef TmEcode (*ThreadInitFunc)(ThreadVars *, const void *, void **);
 typedef TmEcode (*ThreadDeinitFunc)(ThreadVars *, void *);

diff --git a/src/tm-threads.c b/src/tm-threads.c
index 1885b16cbe9904f38be81754eacca10cbc397653..c8f746b73594b85cc441d0914ea3a0201e01b231 100644 (file)
--- a/src/tm-threads.c
+++ b/src/tm-threads.c
@@ -507,7 +507,7 @@ static void *TmThreadsSlotVar(void *td)
             TmThreadsHandleInjectedPackets(tv);
         }
 
-        if (TmThreadsCheckFlag(tv, THV_REQ_FLOW_LOOP)) {
+        if (TmThreadsCheckFlag(tv, (THV_KILL | THV_REQ_FLOW_LOOP))) {
             run = 0;
         }
     }
@@ -1482,10 +1482,21 @@ static void TmThreadDebugValidateNoMorePackets(void)
 #endif
 }
 
+/** \internal
+ *  \brief check if a thread has any of the modules indicated by TM_FLAG_*
+ *  \param tv thread
+ *  \param flags TM_FLAG_*'s
+ *  \retval bool true if at least on of the flags is present */
+static inline bool CheckModuleFlags(const ThreadVars *tv, const uint8_t flags)
+{
+    return (tv->tmm_flags & flags) != 0;
+}
+
 /**
  * \brief Disable all packet threads
  * \param set flag to set
  * \param check flag to check
+ * \param module_flags bitflags of TmModule's to apply the `set` flag to.
  *
  * Support 2 stages in shutting down the packet threads:
  * 1. set THV_REQ_FLOW_LOOP and wait for THV_FLOW_LOOP
@@ -1493,8 +1504,11 @@ static void TmThreadDebugValidateNoMorePackets(void)
  *
  * During step 1 the main loop is exited, and the flow loop logic is entered.
  * During step 2, the flow loop logic is done and the thread closes.
+ *
+ * `module_flags` limits which threads are disabled
  */
-void TmThreadDisablePacketThreads(const uint16_t set, const uint16_t check)
+void TmThreadDisablePacketThreads(
+        const uint16_t set, const uint16_t check, const uint8_t module_flags)
 {
     struct timeval start_ts;
     struct timeval cur_ts;
@@ -1518,6 +1532,11 @@ again:
     /* loop through the packet threads and kill them */
     SCMutexLock(&tv_root_lock);
     for (ThreadVars *tv = tv_root[TVT_PPT]; tv != NULL; tv = tv->next) {
+        /* only set flow worker threads to THV_REQ_FLOW_LOOP */
+        if (!CheckModuleFlags(tv, module_flags)) {
+            SCLogDebug("%s does not have any of the modules %02x, skip", tv->name, module_flags);
+            continue;
+        }
         TmThreadsSetFlag(tv, set);
 
         /* separate worker threads (autofp) will still wait at their
@@ -1533,8 +1552,9 @@ again:
         }
 
         /* wait for it to reach the expected state */
-        while (!TmThreadsCheckFlag(tv, check)) {
+        if (!TmThreadsCheckFlag(tv, check)) {
             SCMutexUnlock(&tv_root_lock);
+            SCLogDebug("%s did not reach state %u, again", tv->name, check);
 
             SleepMsec(1);
             goto again;

diff --git a/src/tm-threads.h b/src/tm-threads.h
index 7698c3519aba903e219076e2502aecbd08aa8dc0..9250a4d60339bea4e93f2ae239431095830bfcd1 100644 (file)
--- a/src/tm-threads.h
+++ b/src/tm-threads.h
@@ -121,7 +121,8 @@ void TmThreadWaitForFlag(ThreadVars *, uint32_t);
 
 TmEcode TmThreadsSlotVarRun (ThreadVars *tv, Packet *p, TmSlot *slot);
 
-void TmThreadDisablePacketThreads(const uint16_t set, const uint16_t check);
+void TmThreadDisablePacketThreads(
+        const uint16_t set, const uint16_t check, const uint8_t module_flags);
 void TmThreadDisableReceiveThreads(void);
 
 uint32_t TmThreadCountThreadsByTmmFlags(uint8_t flags);