From 144353ab004bcd3e85f4cdd0a848add7811df2fe Mon Sep 17 00:00:00 2001
From: Tavian Barnes <tavianator@tavianator.com>
Date: Sun, 7 Jul 2024 12:24:08 -0400
Subject: sighook: Replace sigtables with RCU-protected linked lists

This fixes an ABA problem where sigdispatch() could think no handlers
are registered for a signal even when there are.

Link: https://unix.stackexchange.com/a/779594/56202
Fixes: 375caac ("sighook: New utilities for hooking signals")
---
 tests/sighook.c | 94 ++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 60 insertions(+), 34 deletions(-)

(limited to 'tests/sighook.c')

diff --git a/tests/sighook.c b/tests/sighook.c
index c94526e..b938edf 100644
--- a/tests/sighook.c
+++ b/tests/sighook.c
@@ -6,45 +6,50 @@
 #include "sighook.h"
 #include "atomic.h"
 #include "thread.h"
+#include <errno.h>
 #include <pthread.h>
 #include <signal.h>
 #include <sys/time.h>
 
+/** Counts SIGALRM deliveries. */
 static atomic size_t count = 0;
 
+/** Keeps the background thread alive. */
+static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+static bool done = false;
+
 /** SIGALRM handler. */
 static void alrm_hook(int sig, siginfo_t *info, void *arg) {
 	fetch_add(&count, 1, relaxed);
 }
 
-/** Swap out an old hook for a new hook. */
-static int swap_hooks(struct sighook **hook) {
-	struct sighook *next = sighook(SIGALRM, alrm_hook, NULL, SH_CONTINUE);
-	if (!bfs_echeck(next, "sighook(SIGALRM)")) {
-		return -1;
+/** Background thread that receives signals. */
+static void *hook_thread(void *ptr) {
+	mutex_lock(&mutex);
+	while (!done) {
+		cond_wait(&cond, &mutex);
 	}
-
-	sigunhook(*hook);
-	*hook = next;
-	return 0;
+	mutex_unlock(&mutex);
+	return NULL;
 }
 
-/** Background thread that rapidly (un)registers signal hooks. */
-static void *hook_thread(void *ptr) {
-	struct sighook *hook = sighook(SIGALRM, alrm_hook, NULL, SH_CONTINUE);
-	if (!bfs_echeck(hook, "sighook(SIGALRM)")) {
-		return NULL;
+/** Block a signal in this thread. */
+static int block_signal(int sig, sigset_t *old) {
+	sigset_t set;
+	if (sigemptyset(&set) != 0) {
+		return -1;
+	}
+	if (sigaddset(&set, sig) != 0) {
+		return -1;
 	}
 
-	while (load(&count, relaxed) < 1000) {
-		if (swap_hooks(&hook) != 0) {
-			sigunhook(hook);
-			return NULL;
-		}
+	errno = pthread_sigmask(SIG_BLOCK, &set, old);
+	if (errno != 0) {
+		return -1;
 	}
 
-	sigunhook(hook);
-	return &count;
+	return 0;
 }
 
 bool check_sighook(void) {
@@ -56,34 +61,54 @@ bool check_sighook(void) {
 		goto done;
 	}
 
-	struct itimerval ival = {
-		.it_value = {
-			.tv_usec = 100,
-		},
-		.it_interval = {
-			.tv_usec = 100,
-		},
-	};
+	// Create a timer that sends SIGALRM every 100 microseconds
+	struct itimerval ival = {0};
+	ival.it_value.tv_usec = 100;
+	ival.it_interval.tv_usec = 100;
 	ret &= bfs_echeck(setitimer(ITIMER_REAL, &ival, NULL) == 0);
 	if (!ret) {
 		goto unhook;
 	}
 
+	// Create a background thread to receive signals
 	pthread_t thread;
 	ret &= bfs_echeck(thread_create(&thread, NULL, hook_thread, NULL) == 0);
 	if (!ret) {
 		goto untime;
 	}
 
-	while (ret && load(&count, relaxed) < 1000) {
-		ret &= swap_hooks(&hook) == 0;
+	// Block SIGALRM in this thread so the handler runs concurrently with
+	// sighook()/sigunhook()
+	sigset_t mask;
+	ret &= bfs_echeck(block_signal(SIGALRM, &mask) == 0);
+	if (!ret) {
+		goto untime;
+	}
+
+	// Rapidly register/unregister SIGALRM hooks
+	while (load(&count, relaxed) < 1000) {
+		struct sighook *next = sighook(SIGALRM, alrm_hook, NULL, SH_CONTINUE);
+		ret &= bfs_echeck(next, "sighook(SIGALRM)");
+		if (!ret) {
+			break;
+		}
+
+		sigunhook(hook);
+		hook = next;
 	}
 
- 	void *ptr;
-	thread_join(thread, &ptr);
-	ret &= bfs_check(ptr);
+	// Quit the background thread
+	mutex_lock(&mutex);
+	done = true;
+	mutex_unlock(&mutex);
+	cond_signal(&cond);
+	thread_join(thread, NULL);
 
+	// Restore the old signal mask
+	errno = pthread_sigmask(SIG_SETMASK, &mask, NULL);
+	ret &= bfs_echeck(errno == 0, "pthread_sigmask()");
 untime:
+	// Stop the timer
 	ival.it_value.tv_usec = 0;
 	ret &= bfs_echeck(setitimer(ITIMER_REAL, &ival, NULL) == 0);
 	if (!ret) {
@@ -91,6 +116,7 @@ untime:
 	}
 
 unhook:
+	// Unregister the SIGALRM hook
 	sigunhook(hook);
 done:
 	return ret;
-- 
cgit v1.2.3