lib: crash handlers must be allowed on threads

Blocking all signals on non-main threads is not the way to go, at least
the handlers for SIGSEGV, SIGBUS, SIGILL, SIGABRT and SIGFPE need to run
so we get backtraces.  Otherwise the process just exits.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 13a6ac5b4ca8fc08b348f64de64a787982f24250)

diff --git a/lib/frr_pthread.c b/lib/frr_pthread.c
index 3a4bc712fc529a35c81e62d104e723f06d3702f4..0b4d7c77ae4d3e07ab10b3fcce2ae90858bf6adc 100644 (file)
--- a/lib/frr_pthread.c
+++ b/lib/frr_pthread.c
@@ -20,6 +20,7 @@
 #include "zlog.h"
 #include "libfrr.h"
 #include "libfrr_trace.h"
+#include "sigevent.h"
 
 DEFINE_MTYPE_STATIC(LIB, FRR_PTHREAD, "FRR POSIX Thread");
 DEFINE_MTYPE_STATIC(LIB, PTHREAD_PRIM, "POSIX sync primitives");
@@ -185,10 +186,9 @@ int frr_pthread_run(struct frr_pthread *fpt, const pthread_attr_t *attr)
 
        assert(frr_is_after_fork || !"trying to start thread before fork()");
 
-       /* Ensure we never handle signals on a background thread by blocking
-        * everything here (new thread inherits signal mask)
-        */
-       sigfillset(&blocksigs);
+       sigemptyset(&blocksigs);
+       frr_sigset_add_mainonly(&blocksigs);
+       /* new thread inherits mask */
        pthread_sigmask(SIG_BLOCK, &blocksigs, &oldsigs);
 
        frrtrace(1, frr_libfrr, frr_pthread_run, fpt->name);

diff --git a/lib/frrcu.c b/lib/frrcu.c
index b85c525c5879d6037049a913f383e19349d22a74..1e7ed99eff25f598ff26c9ace06c22162fac16fa 100644 (file)
--- a/lib/frrcu.c
+++ b/lib/frrcu.c
@@ -42,6 +42,7 @@
 #include "frrcu.h"
 #include "seqlock.h"
 #include "atomlist.h"
+#include "sigevent.h"
 
 DEFINE_MTYPE_STATIC(LIB, RCU_THREAD,    "RCU thread");
 DEFINE_MTYPE_STATIC(LIB, RCU_NEXT,      "RCU sequence barrier");
@@ -346,7 +347,19 @@ static void rcu_start(void)
         */
        sigset_t oldsigs, blocksigs;
 
-       sigfillset(&blocksigs);
+       /* technically, the RCU thread is very poorly suited to run even just a
+        * crashlog handler, since zlog_sigsafe() could deadlock on transiently
+        * invalid (due to RCU) logging data structures
+        *
+        * but given that when we try to write a crashlog, we're already in
+        * b0rked territory anyway - give the crashlog handler a chance.
+        *
+        * (also cf. the SIGALRM usage in writing crashlogs to avoid hung
+        * processes on any kind of deadlock in crash handlers)
+        */
+       sigemptyset(&blocksigs);
+       frr_sigset_add_mainonly(&blocksigs);
+       /* new thread inherits mask */
        pthread_sigmask(SIG_BLOCK, &blocksigs, &oldsigs);
 
        rcu_active = true;

diff --git a/lib/sigevent.h b/lib/sigevent.h
index 0b07f594c1c4cdaca1f30bbb2e0f51d5e68c4bbd..2c51ba37673b9521b37f15e3a697e747b3b05046 100644 (file)
--- a/lib/sigevent.h
+++ b/lib/sigevent.h
@@ -45,6 +45,33 @@ bool frr_sigevent_check(sigset_t *setp);
 /* check whether there are signals to handle, process any found */
 extern int frr_sigevent_process(void);
 
+/* Ensure we don't handle "application-type" signals on a secondary thread by
+ * blocking these signals when creating threads
+ *
+ * NB: SIGSEGV, SIGABRT, etc. must be allowed on all threads or we get no
+ * crashlogs.  Since signals vary a little bit between platforms, below is a
+ * list of known things to go to the main thread.  Any unknown signals should
+ * stay thread-local.
+ */
+static inline void frr_sigset_add_mainonly(sigset_t *blocksigs)
+{
+       /* signals we actively handle */
+       sigaddset(blocksigs, SIGHUP);
+       sigaddset(blocksigs, SIGINT);
+       sigaddset(blocksigs, SIGTERM);
+       sigaddset(blocksigs, SIGUSR1);
+
+       /* signals we don't actively use but that semantically belong */
+       sigaddset(blocksigs, SIGUSR2);
+       sigaddset(blocksigs, SIGQUIT);
+       sigaddset(blocksigs, SIGCHLD);
+       sigaddset(blocksigs, SIGPIPE);
+       sigaddset(blocksigs, SIGTSTP);
+       sigaddset(blocksigs, SIGTTIN);
+       sigaddset(blocksigs, SIGTTOU);
+       sigaddset(blocksigs, SIGWINCH);
+}
+
 #ifdef __cplusplus
 }
 #endif