Skip to content

Commit d15d709

Browse files
authored
Merge pull request #18104 from FRRouting/mergify/bp/stable/10.0/pr-18060
lib: crash handlers must be allowed on threads (backport #18060)
2 parents 11a65e1 + fd04241 commit d15d709

File tree

3 files changed

+45
-5
lines changed

3 files changed

+45
-5
lines changed

lib/frr_pthread.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "zlog.h"
2121
#include "libfrr.h"
2222
#include "libfrr_trace.h"
23+
#include "sigevent.h"
2324

2425
DEFINE_MTYPE_STATIC(LIB, FRR_PTHREAD, "FRR POSIX Thread");
2526
DEFINE_MTYPE_STATIC(LIB, PTHREAD_PRIM, "POSIX sync primitives");
@@ -155,10 +156,9 @@ int frr_pthread_run(struct frr_pthread *fpt, const pthread_attr_t *attr)
155156

156157
assert(frr_is_after_fork || !"trying to start thread before fork()");
157158

158-
/* Ensure we never handle signals on a background thread by blocking
159-
* everything here (new thread inherits signal mask)
160-
*/
161-
sigfillset(&blocksigs);
159+
sigemptyset(&blocksigs);
160+
frr_sigset_add_mainonly(&blocksigs);
161+
/* new thread inherits mask */
162162
pthread_sigmask(SIG_BLOCK, &blocksigs, &oldsigs);
163163

164164
frrtrace(1, frr_libfrr, frr_pthread_run, fpt->name);

lib/frrcu.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include "frrcu.h"
4343
#include "seqlock.h"
4444
#include "atomlist.h"
45+
#include "sigevent.h"
4546

4647
DEFINE_MTYPE_STATIC(LIB, RCU_THREAD, "RCU thread");
4748
DEFINE_MTYPE_STATIC(LIB, RCU_NEXT, "RCU sequence barrier");
@@ -346,7 +347,19 @@ static void rcu_start(void)
346347
*/
347348
sigset_t oldsigs, blocksigs;
348349

349-
sigfillset(&blocksigs);
350+
/* technically, the RCU thread is very poorly suited to run even just a
351+
* crashlog handler, since zlog_sigsafe() could deadlock on transiently
352+
* invalid (due to RCU) logging data structures
353+
*
354+
* but given that when we try to write a crashlog, we're already in
355+
* b0rked territory anyway - give the crashlog handler a chance.
356+
*
357+
* (also cf. the SIGALRM usage in writing crashlogs to avoid hung
358+
* processes on any kind of deadlock in crash handlers)
359+
*/
360+
sigemptyset(&blocksigs);
361+
frr_sigset_add_mainonly(&blocksigs);
362+
/* new thread inherits mask */
350363
pthread_sigmask(SIG_BLOCK, &blocksigs, &oldsigs);
351364

352365
rcu_active = true;

lib/sigevent.h

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,33 @@ bool frr_sigevent_check(sigset_t *setp);
4545
/* check whether there are signals to handle, process any found */
4646
extern int frr_sigevent_process(void);
4747

48+
/* Ensure we don't handle "application-type" signals on a secondary thread by
49+
* blocking these signals when creating threads
50+
*
51+
* NB: SIGSEGV, SIGABRT, etc. must be allowed on all threads or we get no
52+
* crashlogs. Since signals vary a little bit between platforms, below is a
53+
* list of known things to go to the main thread. Any unknown signals should
54+
* stay thread-local.
55+
*/
56+
static inline void frr_sigset_add_mainonly(sigset_t *blocksigs)
57+
{
58+
/* signals we actively handle */
59+
sigaddset(blocksigs, SIGHUP);
60+
sigaddset(blocksigs, SIGINT);
61+
sigaddset(blocksigs, SIGTERM);
62+
sigaddset(blocksigs, SIGUSR1);
63+
64+
/* signals we don't actively use but that semantically belong */
65+
sigaddset(blocksigs, SIGUSR2);
66+
sigaddset(blocksigs, SIGQUIT);
67+
sigaddset(blocksigs, SIGCHLD);
68+
sigaddset(blocksigs, SIGPIPE);
69+
sigaddset(blocksigs, SIGTSTP);
70+
sigaddset(blocksigs, SIGTTIN);
71+
sigaddset(blocksigs, SIGTTOU);
72+
sigaddset(blocksigs, SIGWINCH);
73+
}
74+
4875
#ifdef __cplusplus
4976
}
5077
#endif

0 commit comments

Comments
 (0)