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 <[email protected]>
FRRouting · Feb 7, 2025 · 4d11601 · 4d11601
1 parent 4527320
commit 4d11601
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 5 deletions.
diff --git 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
@@ -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
@@ -45,6 +45,35 @@ 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)
+{
+	sigemptyset(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