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: 12f8f03532 ("threads: fix autofp shutdown race condition")

Bug: #7681.

src/source-ipfw.c

@@ -67,6 +67,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)
@@ -177,6 +178,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;
 }
 
 /**

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)

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)

src/suricata.c

@@ -2290,12 +2290,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 */
     FlowWorkToDoCleanup();
-    /* 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();
 

src/tm-modules.h

@@ -35,6 +35,12 @@
 #define TM_FLAG_DETECT_TM       0x08
 #define TM_FLAG_MANAGEMENT_TM   0x10
 #define TM_FLAG_COMMAND_TM      0x20
+#define TM_FLAG_VERDICT_TM      0x40
+
+/* 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 *);

src/tm-threads.c

@@ -517,7 +517,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 = false;
         }
     }
@@ -1508,10 +1508,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
@@ -1519,8 +1530,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;
@@ -1544,6 +1558,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
@@ -1559,8 +1578,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;

src/tm-threads.h

@@ -122,7 +122,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);