From: Victor Julien Date: Thu, 24 Mar 2016 10:51:49 +0000 (+0100) Subject: signals: cleanup signal handling X-Git-Tag: suricata-3.0.1~19 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dd98bc353ea81c1626c4ab827a962140c42b7061;p=thirdparty%2Fsuricata.git 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 8 _int_malloc (av=0x3140f8fe80, bytes=) 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 05e5ffde7c..57e7afdb06 100644 --- 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 b11239ac2e..4ecab38c86 100644 --- 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);