]> git.puffer.fish Git - matthieu/frr.git/commitdiff
lib: crash handlers must be allowed on threads
authorDavid Lamparter <equinox@opensourcerouting.org>
Fri, 7 Feb 2025 12:22:25 +0000 (13:22 +0100)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Tue, 11 Feb 2025 17:28:50 +0000 (17:28 +0000)
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)

lib/frr_pthread.c
lib/frrcu.c
lib/sigevent.h

index dd675bbb858b6fc3a5d49729aea9a9c6c6200a6e..21406654a5bcf1cc0961aeb9a23afe80daf6f7df 100644 (file)
@@ -30,6 +30,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");
@@ -165,10 +166,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);
index 0e717a98a5d98ad588f19b29e7c635368c758a75..1dfdd44eed64aad601f36fa630448dcd7991b9b3 100644 (file)
@@ -53,6 +53,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");
@@ -349,7 +350,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;
index dd1ee99587b510f009d44bbfc2a1864b9d298010..304bbdae55e3024fb28c3a2273104fd547f96d31 100644 (file)
@@ -60,6 +60,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