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.

diff --git a/src/counters.c b/src/counters.c
index bdced6243a5fc07aa8bb317884e679ab6f9c2725..007b093d99c3f471e90f2187b4248f9a3aa17b99 100644 (file)
--- a/src/counters.c
+++ b/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;

diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c
index 233bbce3e4a1be020cb686a58923747cf471188d..cb07dc61677083ef5c137cf0ff39cdf6769190c6 100644 (file)
--- a/src/detect-engine-loader.c
+++ b/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);
 

diff --git a/src/flow-manager.c b/src/flow-manager.c
index 47484f9dc5e47c0c6d25ea3aeabb257c3539bf65..2c58fd4b8709a05f9e5944de4f348da2db1b2e76 100644 (file)
--- a/src/flow-manager.c
+++ b/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;

diff --git a/src/suricata.c b/src/suricata.c
index d78c71a2c877353c380f20d0a6193099c19ce8f8..631a1b6a06dcb01bbd44455e79af0a41ca3ea637 100644 (file)
--- a/src/suricata.c
+++ b/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();
 

diff --git a/src/tm-threads.c b/src/tm-threads.c
index 07b2bf1b4f2bed1443406aaadef86a782a2f144a..522b0c9475ce9196e3c75c86559b8484e057ee93 100644 (file)
--- a/src/tm-threads.c
+++ b/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;

diff --git a/src/util-signal.c b/src/util-signal.c
index dada13316a3d61f0198ca41589abecd2daabb0f3..9e3ec0cdf81263497b7336cbd26aeb3e4dd38ddb 100644 (file)
--- a/src/util-signal.c
+++ b/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;

diff --git a/src/util-signal.h b/src/util-signal.h
index 171d1b553692b27077af6a8920562b6fc187ef42..1d5ba3973e6f821cd8946c140ca07bc92aee0167 100644 (file)
--- a/src/util-signal.h
+++ b/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));