summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Lamparter <equinox@opensourcerouting.org>2025-02-07 13:22:25 +0100
committerMergify <37929162+mergify[bot]@users.noreply.github.com>2025-02-11 17:28:51 +0000
commit4fdb1f0496a97c3e54bfcaed121d24143862fb6d (patch)
tree8e49b72c2f59b8b9eb7b48172b4c5ca2d2c2d654
parent8aca554935c374e909a44af060f84346942214ef (diff)
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)
-rw-r--r--lib/frr_pthread.c8
-rw-r--r--lib/frrcu.c15
-rw-r--r--lib/sigevent.h27
3 files changed, 45 insertions, 5 deletions
diff --git a/lib/frr_pthread.c b/lib/frr_pthread.c
index c4ead01bf6..2dfe06724f 100644
--- a/lib/frr_pthread.c
+++ b/lib/frr_pthread.c
@@ -17,6 +17,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");
@@ -152,10 +153,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 c7cc655e09..6719bc9d87 100644
--- 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");
@@ -338,7 +339,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 0b07f594c1..2c51ba3767 100644
--- 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