From: David Lamparter <equinox@opensourcerouting.org>
Date: Thu, 22 Apr 2021 10:10:27 +0000 (+0200)
Subject: lib: hard-fail creating threads before fork()
X-Git-Tag: base_8.0~106^2~1
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=38554d3ae6c4ea553fd4e9ab2185c0d79f1144f7;p=matthieu%2Ffrr.git

lib: hard-fail creating threads before fork()

Creating any threads before we fork() into the background (if `-d` is
given) is an extremely dangerous footgun;  the threads are created in
the parent and terminated when that exits.

This is extra dangerous because while testing, you'd often run the
daemon in foreground without `-d`, and everything works as expected.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---

diff --git a/lib/frr_pthread.c b/lib/frr_pthread.c
index 03359f4d18..898fe98aad 100644
--- a/lib/frr_pthread.c
+++ b/lib/frr_pthread.c
@@ -28,6 +28,7 @@
 #include "memory.h"
 #include "linklist.h"
 #include "zlog.h"
+#include "libfrr.h"
 #include "libfrr_trace.h"
 
 DEFINE_MTYPE_STATIC(LIB, FRR_PTHREAD, "FRR POSIX Thread");
@@ -162,6 +163,8 @@ int frr_pthread_run(struct frr_pthread *fpt, const pthread_attr_t *attr)
 	int ret;
 	sigset_t oldsigs, blocksigs;
 
+	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)
 	 */
diff --git a/lib/libfrr.c b/lib/libfrr.c
index 38f994bdfc..970e82c064 100644
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -70,6 +70,8 @@ static char dbfile_default[512];
 #endif
 static char vtypath_default[512];
 
+/* cleared in frr_preinit(), then re-set after daemonizing */
+bool frr_is_after_fork = true;
 bool debug_memstats_at_exit = false;
 static bool nodetach_term, nodetach_daemon;
 static uint64_t startup_fds;
@@ -308,6 +310,7 @@ void frr_init_vtydir(void)
 void frr_preinit(struct frr_daemon_info *daemon, int argc, char **argv)
 {
 	di = daemon;
+	frr_is_after_fork = false;
 
 	/* basename(), opencoded. */
 	char *p = strrchr(argv[0], '/');
@@ -990,6 +993,8 @@ void frr_config_fork(void)
 	if (di->daemon_mode || di->terminal)
 		frr_daemonize();
 
+	frr_is_after_fork = true;
+
 	if (!di->pid_file)
 		di->pid_file = pidfile_default;
 	pid_output(di->pid_file);
diff --git a/lib/libfrr.h b/lib/libfrr.h
index 47ded8f313..3dc5d7af81 100644
--- a/lib/libfrr.h
+++ b/lib/libfrr.h
@@ -172,6 +172,8 @@ extern const char frr_scriptdir[];
 
 extern char frr_protoname[];
 extern char frr_protonameinst[];
+/* always set in the spot where we *would* fork even if we don't do so */
+extern bool frr_is_after_fork;
 
 extern bool debug_memstats_at_exit;