Skip to content

Commit

Permalink
Merge pull request #18060 from opensourcerouting/lib-crashlog-signals
Browse files Browse the repository at this point in the history
lib: crash handlers must be allowed on threads
  • Loading branch information
donaldsharp authored Feb 11, 2025
2 parents c2159b7 + 13a6ac5 commit f64104b
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 5 deletions.
8 changes: 4 additions & 4 deletions lib/frr_pthread.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down
15 changes: 14 additions & 1 deletion lib/frrcu.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down
27 changes: 27 additions & 0 deletions lib/sigevent.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f64104b

Please sign in to comment.