signal: use centralized pthread_sigmask for signals

according to its man page, sigprocmask has undefined behavior in
multithreaded environments. Instead of explictly blocking the handling
of SIGUSR2 in every thread, direct block handling SIGUSR2 before
creating the threads and enable again the handling of this signal
afterwards. In this way, only the main thread will be able to manage
this signal properly.

src/counters.c

@@ -323,10 +323,6 @@ static void StatsReleaseCtx(void)
  */
 static void *StatsMgmtThread(void *arg)
 {
-#ifndef OS_WIN32
-    /* block usr2.  usr2 to be handled by the main thread only */
-    UtilSignalBlock(SIGUSR2);
-#endif
     ThreadVars *tv_local = (ThreadVars *)arg;
     uint8_t run = 1;
     struct timespec cond_time;
@@ -411,10 +407,6 @@ static void *StatsMgmtThread(void *arg)
  */
 static void *StatsWakeupThread(void *arg)
 {
-#ifndef OS_WIN32
-    /* block usr2.  usr2 to be handled by the main thread only */
-    UtilSignalBlock(SIGUSR2);
-#endif
     ThreadVars *tv_local = (ThreadVars *)arg;
     uint8_t run = 1;
     ThreadVars *tv = NULL;

src/detect-engine-loader.c

@@ -564,10 +564,6 @@ static TmEcode DetectLoaderThreadDeinit(ThreadVars *t, void *data)
 
 static TmEcode DetectLoader(ThreadVars *th_v, void *thread_data)
 {
-#ifndef OS_WIN32
-    /* block usr2. usr2 to be handled by the main thread only */
-    UtilSignalBlock(SIGUSR2);
-#endif
     DetectLoaderThreadData *ftd = (DetectLoaderThreadData *)thread_data;
     BUG_ON(ftd == NULL);
 

src/flow-manager.c

@@ -658,10 +658,6 @@ static TmEcode FlowManagerThreadDeinit(ThreadVars *t, void *data)
  */
 static TmEcode FlowManager(ThreadVars *th_v, void *thread_data)
 {
-#ifndef OS_WIN32
-    /* block usr2.  usr1 to be handled by the main thread only */
-    UtilSignalBlock(SIGUSR2);
-#endif
     FlowManagerThreadData *ftd = thread_data;
     struct timeval ts;
     uint32_t established_cnt = 0, new_cnt = 0, closing_cnt = 0;
@@ -887,10 +883,6 @@ static TmEcode FlowRecyclerThreadDeinit(ThreadVars *t, void *data)
  */
 static TmEcode FlowRecycler(ThreadVars *th_v, void *thread_data)
 {
-#ifndef OS_WIN32
-    /* block usr2. usr2 to be handled by the main thread only */
-    UtilSignalBlock(SIGUSR2);
-#endif
     struct timeval ts;
     struct timespec cond_time;
     int flow_update_delay_sec = FLOW_NORMAL_MODE_UPDATE_DELAY_SEC;

src/suricata.c

@@ -2145,7 +2145,6 @@ static int InitSignalHandler(SCInstance *suri)
     UtilSignalHandlerSetup(SIGINT, SignalHandlerSigint);
     UtilSignalHandlerSetup(SIGTERM, SignalHandlerSigterm);
 #ifndef OS_WIN32
-    UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2);
     UtilSignalHandlerSetup(SIGHUP, SignalHandlerSigHup);
     UtilSignalHandlerSetup(SIGPIPE, SIG_IGN);
     UtilSignalHandlerSetup(SIGSYS, SIG_IGN);
@@ -2458,8 +2457,10 @@ static void PostRunStartedDetectSetup(SCInstance *suri)
      * can't call it during the first sig load phase or while threads are still
      * starting up. */
     if (DetectEngineEnabled() && suri->sig_file == NULL &&
-            suri->delayed_detect == 0)
+            suri->delayed_detect == 0) {
         UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2);
+        UtilSignalUnblock(SIGUSR2);
+    }
 #endif
     if (suri->delayed_detect) {
         /* force 'reload', this will load the rules and swap engines */
@@ -2774,6 +2775,18 @@ int main(int argc, char **argv)
 
     (void)SCSetThreadName("Suricata-Main");
 
+    /* Ignore SIGUSR2 as early as possble. We redeclare interest
+     * once we're done launching threads. The goal is to either die
+     * completely or handle any and all SIGUSR2s correctly.
+     */
+#ifndef OS_WIN32
+    UtilSignalHandlerSetup(SIGUSR2, SIG_IGN);
+    if (UtilSignalBlock(SIGUSR2)) {
+        SCLogError(SC_ERR_INITIALIZATION, "SIGUSR2 initialization error");
+        exit(EXIT_FAILURE);
+    }
+#endif
+
     ParseSizeInit();
     RunModeRegisterRunModes();
 

src/tm-threads.c

@@ -270,10 +270,6 @@ static int TmThreadTimeoutLoop(ThreadVars *tv, TmSlot *s)
 
 static void *TmThreadsSlotPktAcqLoop(void *td)
 {
-#ifndef OS_WIN32
-    /* block usr2.  usr2 to be handled by the main thread only */
-    UtilSignalBlock(SIGUSR2);
-#endif
     ThreadVars *tv = (ThreadVars *)td;
     TmSlot *s = tv->tm_slots;
     char run = 1;
@@ -522,10 +518,6 @@ error:
  */
 static void *TmThreadsSlotVar(void *td)
 {
-#ifndef OS_WIN32
-    /* block usr2.  usr2 to be handled by the main thread only */
-    UtilSignalBlock(SIGUSR2);
-#endif
     ThreadVars *tv = (ThreadVars *)td;
     TmSlot *s = (TmSlot *)tv->tm_slots;
     Packet *p = NULL;
@@ -684,10 +676,6 @@ error:
 
 static void *TmThreadsManagement(void *td)
 {
-#ifndef OS_WIN32
-    /* block usr2.  usr2 to be handled by the main thread only */
-    UtilSignalBlock(SIGUSR2);
-#endif
     ThreadVars *tv = (ThreadVars *)td;
     TmSlot *s = (TmSlot *)tv->tm_slots;
     TmEcode r = TM_ECODE_OK;

src/util-signal.c

@@ -34,7 +34,24 @@ int UtilSignalBlock(int signum)
         return -1;
     if (sigaddset(&x, signum) < 0)
         return -1;
-    if (sigprocmask(SIG_BLOCK, &x, NULL) < 0)
+    /* don't use sigprocmask(), as it's undefined for
+     * multithreaded programs. Use phtread_sigmask().
+     */
+    if (pthread_sigmask(SIG_BLOCK, &x, NULL) != 0)
+        return -1;
+#endif
+    return 0;
+}
+
+int UtilSignalUnblock(int signum)
+{
+#ifndef OS_WIN32
+    sigset_t x;
+    if (sigemptyset(&x) < 0)
+        return -1;
+    if (sigaddset(&x, signum) < 0)
+        return -1;
+    if (pthread_sigmask(SIG_UNBLOCK, &x, NULL) != 0)
         return -1;
 #endif
     return 0;
@@ -42,7 +59,7 @@ int UtilSignalBlock(int signum)
 
 void UtilSignalHandlerSetup(int sig, void (*handler)(int))
 {
-#if defined (OS_WIN32)
+#ifdef OS_WIN32
 	signal(sig, handler);
 #else
     struct sigaction action;

src/util-signal.h

@@ -25,6 +25,7 @@
 #define __UTIL_SIGNAL_H__
 
 int UtilSignalBlock(int);
+int UtilSignalUnblock(int);
 void UtilSignalHandlerSetup(int, void (*handler)(int));
 #if 0
 int UtilSignalIsHandler(int sig, void (*handler)(int));