Fix poll race condition on *BSD

5 files changed, 52 insertions, 64 deletions

diff --git a/src/async.c b/src/async.c
index c1c4802..4a2de10 100644
--- a/src/async.c
+++ b/src/async.c
@@ -114,6 +114,9 @@ void fastd_async_handle(void) {
 		exit_errno("fastd_async_handle: recvmsg");
 
 	switch (header.type) {
+	case ASYNC_TYPE_NOP:
+		break;
+
 	case ASYNC_TYPE_RESOLVE_RETURN:
 		handle_resolve_return((const fastd_async_resolve_return_t *)buf);
 		break;
@@ -143,7 +146,7 @@ void fastd_async_enqueue(fastd_async_type_t type, const void *data, size_t len)
 	};
 	struct msghdr msg = {
 		.msg_iov = vec,
-		.msg_iovlen = 2,
+		.msg_iovlen = len ? 2 : 1,
 	};
 
 	if (sendmsg(ctx.async_wfd, &msg, 0) < 0)
diff --git a/src/async.h b/src/async.h
index e3298d4..15fd737 100644
--- a/src/async.h
+++ b/src/async.h
@@ -38,6 +38,7 @@
 
 /** A type of asynchronous notification */
 typedef enum fastd_async_type {
+	ASYNC_TYPE_NOP,				/**< Does nothing (is used to ensure poll returns quickly after a signal has occurred) */
 	ASYNC_TYPE_RESOLVE_RETURN,		/**< A DNS resolver response */
 	ASYNC_TYPE_VERIFY_RETURN,		/**< A on-verify return */
 } fastd_async_type_t;
diff --git a/src/fastd.c b/src/fastd.c
index c8f8e54..0797ad8 100644
--- a/src/fastd.c
+++ b/src/fastd.c
@@ -30,7 +30,6 @@
 */
 
 
-
 #include "fastd.h"
 #include "async.h"
 #include "config.h"
@@ -66,30 +65,41 @@
 fastd_context_t ctx = {};
 
 
-volatile bool fastd_sig_hup = false;		/**< Is set to true when a SIGHUP is received */
-volatile bool fastd_sig_terminate = false;	/**< Is set to true when a SIGTERM, SIGQUIT or SIGINT is received */
-volatile bool fastd_sig_dump = false;		/**< Is set to true when a SIGUSR1 is received */
-volatile bool fastd_sig_chld = false;		/**< Is set to true when a SIGCHLD is received */
+static volatile bool sighup = false;		/**< Is set to true when a SIGHUP is received */
+static volatile bool terminate = false;		/**< Is set to true when a SIGTERM, SIGQUIT or SIGINT is received */
+static volatile bool dump = false;		/**< Is set to true when a SIGUSR1 is received */
+static volatile bool sigchld = false;		/**< Is set to true when a SIGCHLD is received */
 
 
-/** SIGHUP handler */
-static void on_sighup(int signo UNUSED) {
-	fastd_sig_hup = true;
-}
+/** Signal handler; just saves the signals to be handled later */
+static void on_signal(int signo) {
+	switch(signo) {
+	case SIGHUP:
+		sighup = true;
+		break;
 
-/** SIGTERM, SIGQUIT and SIGINT handler */
-static void on_terminate(int signo UNUSED) {
-	fastd_sig_terminate = true;
-}
+	case SIGUSR1:
+		dump = true;
+		break;
 
-/** SIGUSR1 handler */
-static void on_sigusr1(int signo UNUSED) {
-	fastd_sig_dump = true;
-}
+	case SIGCHLD:
+		sigchld = true;
+		break;
+
+	case SIGTERM:
+	case SIGQUIT:
+	case SIGINT:
+		terminate = true;
+		break;
 
-/** SIGCHLD handler */
-static void on_sigchld(int signo UNUSED) {
-	fastd_sig_chld = true;
+	default:
+		return;
+	}
+
+#ifndef USE_EPOLL
+	/* Avoids a race condition between pthread_sigmask() and poll() (FreeBSD doesn't have ppoll() ...) */
+	fastd_async_enqueue(ASYNC_TYPE_NOP, NULL, 0);
+#endif
 }
 
 /** Installs signal handlers */
@@ -99,15 +109,16 @@ static void init_signals(void) {
 	/* block all signals */
 	pthread_sigmask(SIG_SETMASK, &set, NULL);
 
-	struct sigaction action;
-	action.sa_flags = 0;
+	struct sigaction action = {};
 	sigemptyset(&action.sa_mask);
 
-	action.sa_handler = on_sighup;
+	action.sa_handler = on_signal;
 	if (sigaction(SIGHUP, &action, NULL))
 		exit_errno("sigaction");
-
-	action.sa_handler = on_terminate;
+	if (sigaction(SIGUSR1, &action, NULL))
+		exit_errno("sigaction");
+	if (sigaction(SIGCHLD, &action, NULL))
+		exit_errno("sigaction");
 	if (sigaction(SIGTERM, &action, NULL))
 		exit_errno("sigaction");
 	if (sigaction(SIGQUIT, &action, NULL))
@@ -115,14 +126,6 @@ static void init_signals(void) {
 	if (sigaction(SIGINT, &action, NULL))
 		exit_errno("sigaction");
 
-	action.sa_handler = on_sigusr1;
-	if (sigaction(SIGUSR1, &action, NULL))
-		exit_errno("sigaction");
-
-	action.sa_handler = on_sigchld;
-	if (sigaction(SIGCHLD, &action, NULL))
-		exit_errno("sigaction");
-
 	action.sa_handler = SIG_IGN;
 	if (sigaction(SIGPIPE, &action, NULL))
 		exit_errno("sigaction");
@@ -625,8 +628,8 @@ static inline void reap_zombies(void) {
 
 /** The \em real signal handlers */
 static inline void handle_signals(void) {
-	if (fastd_sig_hup) {
-		fastd_sig_hup = false;
+	if (sighup) {
+		sighup = false;
 
 		pr_info("reconfigure triggered");
 
@@ -634,13 +637,13 @@ static inline void handle_signals(void) {
 		init_peers();
 	}
 
-	if (fastd_sig_dump) {
-		fastd_sig_dump = false;
+	if (dump) {
+		dump = false;
 		dump_state();
 	}
 
-	if (fastd_sig_chld) {
-		fastd_sig_chld = false;
+	if (sigchld) {
+		sigchld = false;
 		reap_zombies();
 	}
 }
@@ -699,7 +702,7 @@ static inline void cleanup(void) {
 int main(int argc, char *argv[]) {
 	init(argc, argv);
 
-	while (!fastd_sig_terminate)
+	while (!terminate)
 		run();
 
 	cleanup();
diff --git a/src/fastd.h b/src/fastd.h
index eeb2d3d..f69f2f8 100644
--- a/src/fastd.h
+++ b/src/fastd.h
@@ -292,11 +292,6 @@ struct fastd_string_stack {
 extern fastd_context_t ctx;
 extern fastd_config_t conf;
 
-extern volatile bool fastd_sig_hup;
-extern volatile bool fastd_sig_terminate;
-extern volatile bool fastd_sig_dump;
-extern volatile bool fastd_sig_chld;
-
 
 void fastd_send(const fastd_socket_t *sock, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *remote_addr, fastd_peer_t *peer, fastd_buffer_t buffer, size_t stat_size);
 void fastd_send_handshake(const fastd_socket_t *sock, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *remote_addr, fastd_peer_t *peer, fastd_buffer_t buffer);
@@ -376,11 +371,6 @@ static inline size_t alignto(size_t l, size_t a) {
 	return block_count(l, a)*a;
 }
 
-/** Returns true if any unhandled signal in currently queued */
-static inline bool fastd_signalled(void) {
-	return (fastd_sig_hup || fastd_sig_terminate || fastd_sig_dump || fastd_sig_chld);
-}
-
 /** Returns the maximum payload size \em fastd is configured to transport */
 static inline size_t fastd_max_payload(void) {
 	switch (conf.mode) {
diff --git a/src/poll.c b/src/poll.c
index a74d249..3756162 100644
--- a/src/poll.c
+++ b/src/poll.c
@@ -139,7 +139,7 @@ void fastd_poll_handle(void) {
 	struct epoll_event events[16];
 	int ret = epoll_pwait(ctx.epoll_fd, events, 16, timeout, &set);
 	if (ret < 0 && errno != EINTR)
-		exit_errno("epoll_wait");
+		exit_errno("epoll_pwait");
 
 	fastd_update_time();
 
@@ -253,18 +253,9 @@ void fastd_poll_handle(void) {
 	sigemptyset(&set);
 	pthread_sigmask(SIG_SETMASK, &set, &oldset);
 
-	int ret = -1;
-
-	if (!fastd_signalled()) {
-		/*
-		  There is a race condition here: if a signal occurs after the fastd_signalled() check, but before the poll
-		  call, poll() will be called with its normal timeout, potentially delaying the actual signal handling. On
-		  OpenBSD we could use ppoll() to fix this, but on FreeBSD we're out of luck.
-		 */
-		ret = poll(VECTOR_DATA(ctx.pollfds), VECTOR_LEN(ctx.pollfds), timeout);
-		if (ret < 0 && errno != EINTR)
-			exit_errno("poll");
-	}
+	int ret = poll(VECTOR_DATA(ctx.pollfds), VECTOR_LEN(ctx.pollfds), timeout);
+	if (ret < 0 && errno != EINTR)
+		exit_errno("poll");
 
 	pthread_sigmask(SIG_SETMASK, &oldset, NULL);
 	fastd_update_time();