signals: cleanup signal handling

Simplify handling of USR2 signal. The SCLogInfo usage could lead to
dead locks as the SCLog API can do many complicated things including
memory allocations, syslog calls, libjansson message construction.

If an existing malloc call was interupted, it could lead to the
following dead lock:

 0  __lll_lock_wait_private () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:97
 1  0x0000003140c7d2df in _L_lock_10176 () from /lib64/libc.so.6
 2  0x0000003140c7ab83 in __libc_malloc (bytes=211543457408) at malloc.c:3655
 3  0x0000003140c80ec2 in __strdup (s=0x259ca40 "[%i] %t - (%f:%l) <%d> (%n) -- ") at strdup.c:43
 4  0x000000000059dd4a in SCLogMessageGetBuffer (tval=0x7fff52b47360, color=1, type=SC_LOG_OP_TYPE_REGULAR, buffer=0x7fff52b47370 "", buffer_size=2048,
    log_format=0x259ca40 "[%i] %t - (%f:%l) <%d> (%n) -- ", log_level=SC_LOG_INFO, file=0x63dd00 "suricata.c", line=287, function=0x640f50 "SignalHandlerSigusr2StartingUp", error_code=SC_OK,
    message=0x7fff52b47bb0 "Live rule reload only possible after engine completely started.") at util-debug.c:307
 5  0x000000000059e940 in SCLogMessage (log_level=SC_LOG_INFO, file=0x63dd00 "suricata.c", line=287, function=0x640f50 "SignalHandlerSigusr2StartingUp", error_code=SC_OK,
    message=0x7fff52b47bb0 "Live rule reload only possible after engine completely started.") at util-debug.c:549
 6  0x000000000057e374 in SignalHandlerSigusr2StartingUp (sig=12) at suricata.c:287
 7  <signal handler called>
 8  _int_malloc (av=0x3140f8fe80, bytes=<value optimized out>) at malloc.c:4751
 9  0x0000003140c7ab1c in __libc_malloc (bytes=296) at malloc.c:3657
 10 0x0000000000504d55 in FlowAlloc () at flow-util.c:60
 11 0x00000000004fd909 in FlowInitConfig (quiet=0 '\000') at flow.c:454
 12 0x0000000000584c8e in main (argc=6, argv=0x7fff52b4a3b8) at suricata.c:2300

This patch simply sets a variable and lets the main loop act on that.

diff --git a/src/suricata.c b/src/suricata.c
index 05e5ffde7c5780ab0d04ddfc18ed3b8ee91ee683..57e7afdb067a981a38e59c94f951fd110a679e36 100644 (file)
--- a/src/suricata.c
+++ b/src/suricata.c
@@ -273,6 +273,13 @@ int RunmodeGetCurrent(void)
     return run_mode;
 }
 
+/** signal handlers
+ *
+ *  WARNING: don't use the SCLog* API in the handlers. The API is complex
+ *  with memory allocation possibly happening, calls to syslog, json message
+ *  construction, etc.
+ */
+
 static void SignalHandlerSigint(/*@unused@*/ int sig)
 {
     sigint_count = 1;
@@ -282,26 +289,11 @@ static void SignalHandlerSigterm(/*@unused@*/ int sig)
     sigterm_count = 1;
 }
 
-void SignalHandlerSigusr2StartingUp(int sig)
-{
-    SCLogInfo("Live rule reload only possible after engine completely started.");
-}
-
-void SignalHandlerSigusr2DelayedDetect(int sig)
-{
-    SCLogWarning(SC_ERR_LIVE_RULE_SWAP, "Live rule reload blocked while delayed detect is still loading.");
-}
-
-void SignalHandlerSigusr2SigFileStartup(int sig)
-{
-    SCLogInfo("Live rule reload not possible if -s or -S option used at runtime.");
-}
-
 /**
  * SIGUSR2 handler.  Just set sigusr2_count.  The main loop will act on
  * it.
  */
-void SignalHandlerSigusr2(int sig)
+static void SignalHandlerSigusr2(int sig)
 {
     sigusr2_count = 1;
 }
@@ -1829,6 +1821,7 @@ static int InitSignalHandler(SCInstance *suri)
 {
     /* registering signals we use */
     UtilSignalHandlerSetup(SIGINT, SignalHandlerSigint);
+    UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2);
     UtilSignalHandlerSetup(SIGTERM, SignalHandlerSigterm);
     UtilSignalHandlerSetup(SIGPIPE, SIG_IGN);
     UtilSignalHandlerSetup(SIGSYS, SIG_IGN);
@@ -2163,11 +2156,6 @@ static int PostConfLoadedSetup(SCInstance *suri)
 
     DetectEngineRegisterAppInspectionEngines();
 
-    if (suri->sig_file != NULL)
-        UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2SigFileStartup);
-    else
-        UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2StartingUp);
-
     StorageFinalize();
 
     TmModuleRunInit();
@@ -2422,11 +2410,6 @@ int main(int argc, char **argv)
     if (suri.delayed_detect) {
         /* force 'reload', this will load the rules and swap engines */
         DetectEngineReload(NULL, &suri);
-
-        if (suri.sig_file != NULL)
-            UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2SigFileStartup);
-        else
-            UtilSignalHandlerSetup(SIGUSR2, SignalHandlerSigusr2);
         SCLogNotice("Signature(s) loaded, Detect thread(s) activated.");
     }
 
@@ -2459,11 +2442,21 @@ int main(int argc, char **argv)
         }
 
         if (sigusr2_count > 0) {
-            DetectEngineReload(conf_filename, &suri);
+            if (suri.sig_file != NULL) {
+                SCLogWarning(SC_ERR_LIVE_RULE_SWAP, "Live rule reload not "
+                        "possible if -s or -S option used at runtime.");
+            } else {
+                DetectEngineReload(conf_filename, &suri);
+            }
             sigusr2_count--;
         } else if (DetectEngineReloadIsStart()) {
-            DetectEngineReload(conf_filename, &suri);
-            DetectEngineReloadSetDone();
+            if (suri.sig_file != NULL) {
+                SCLogWarning(SC_ERR_LIVE_RULE_SWAP, "Live rule reload not "
+                        "possible if -s or -S option used at runtime.");
+            } else {
+                DetectEngineReload(conf_filename, &suri);
+                DetectEngineReloadSetDone();
+            }
         }
 
         usleep(10* 1000);

diff --git a/src/suricata.h b/src/suricata.h
index b11239ac2e9ca9b9b8b9add9a29fc94b1fa47b30..4ecab38c86a694b1ce14d2f2d6b1742a50647333 100644 (file)
--- a/src/suricata.h
+++ b/src/suricata.h
@@ -182,11 +182,6 @@ void EngineStop(void);
 void EngineKill(void);
 void EngineDone(void);
 
-/* live rule swap required this to be made static */
-void SignalHandlerSigusr2(int);
-void SignalHandlerSigusr2EngineShutdown(int);
-void SignalHandlerSigusr2Idle(int sig);
-
 int RunmodeIsUnittests(void);
 int RunmodeGetCurrent(void);
 int IsRuleReloadSet(int quiet);