Rewrite libpsx thread shutdown path to support musl.

Addresses:

   https://bugzilla.kernel.org/show_bug.cgi?id=208477

Removed the non-wrapping libpsx macro hacks. The API surface as such
becomes a little smaller and I now have confidence that wrapping
pthread_create using the linker options works with Go, gcc and musl
compilers. I feel it is stable enough to call good to delete the
workarounds.

Signed-off-by: Andrew G. Morgan <morgan@kernel.org>

8 files changed, 68 insertions, 85 deletions

diff --git a/Makefile b/Makefile
index 30e1166..e148633 100644
--- a/Makefile
+++ b/Makefile
@@ -60,8 +60,11 @@ endif
 
 distcheck:
 	./distcheck.sh
+	make CC=/usr/local/musl/bin/musl-gcc clean all test sudotest
+	make clean all test sudotest
+	make distclean
 
-morganrelease: distclean distcheck
+morganrelease: distcheck
 	@echo "sign the main library tag twice: older DSA key; and newer RSA (kernel.org) key"
 	git tag -u D41A6DF2 -s libcap-$(VERSION).$(MINOR) -m "This is libcap-$(VERSION).$(MINOR)"
 	git tag -u E2CCF3F4 -s libcap-korg-$(VERSION).$(MINOR) -m "This is libcap-$(VERSION).$(MINOR)"
diff --git a/doc/Makefile b/doc/Makefile
index f15ef94..e60f72d 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -17,7 +17,7 @@ MAN3S = cap_init.3 cap_free.3 cap_dup.3 \
 	cap_get_mode.3 cap_set_mode.3 cap_mode_name.3 \
 	cap_get_secbits.3 cap_set_secbits.3 \
 	cap_setuid.3 cap_setgroups.3 \
-	psx_register.3 psx_syscall.3 psx_syscall3.3 psx_syscall6.3 libpsx.3
+	psx_syscall.3 psx_syscall3.3 psx_syscall6.3 libpsx.3
 MAN8S = getcap.8 setcap.8 getpcaps.8
 
 MANS = $(MAN1S) $(MAN3S) $(MAN8S)
diff --git a/doc/libpsx.3 b/doc/libpsx.3
index 9516558..615fceb 100644
--- a/doc/libpsx.3
+++ b/doc/libpsx.3
@@ -40,10 +40,6 @@ the
 call with a psx thread registration function is used to allow
 .B libpsx
 to keep track of all pthreads. If that trick is not usable by your application, then the much more cumbersome and fragile
-.BR psx_register ( pthread_t " thread)"
-function is provided to register threads manually with the library. To
-successfully use this approach an explanation of how to code for it is
-provided at the top of the
 .B <sys/psx_syscall.h>
 header file.
 .PP
diff --git a/doc/psx_register.3 b/doc/psx_register.3
deleted file mode 100644
index 663420c..0000000
--- a/doc/psx_register.3
+++ /dev/null
@@ -1 +0,0 @@
-.so man3/libpsx.3
diff --git a/psx/include/sys/psx_syscall.h b/psx/include/sys/psx_syscall.h
index 20d03a8..c089a88 100644
--- a/psx/include/sys/psx_syscall.h
+++ b/psx/include/sys/psx_syscall.h
@@ -14,25 +14,6 @@
  * mechanism is limited to 9 specific set*() syscalls that do not
  * support the syscall6 API (needed for prctl functions and the ambient
  * capabilities set for example).
- *
- * This psx library API also includes explicit registration of threads
- * if implicit wrapping the pthread_create() function is problematic
- * for your application via the psx_pthread_create() function. To use
- * the library in that way, you should include this line in the file
- * containing your main() function:
- *
- * -----------
- * #include <sys/psx_syscall.h>
- *
- * int main(...) {
- *
- *   ....
- *
- * }
- * PSX_NO_LINKER_WRAPPING
- * -----------
- *
- * This will ensure that your binary can link.
  */
 
 #ifndef _SYS_PSX_SYSCALL_H
@@ -52,12 +33,6 @@ extern "C" {
 int __real_pthread_create(pthread_t *thread, const pthread_attr_t *attr,
 			  void *(*start_routine) (void *), void *arg);
 
-#define PSX_NO_LINKER_WRAPPING  int                                      \
-    __real_pthread_create(pthread_t *thread, const pthread_attr_t *attr, \
-			  void *(*start_routine) (void *), void *arg) {  \
-      return -1;                                                         \
-    }
-
 /*
  * psx_syscall performs the specified syscall on all psx registered
  * threads. The mecanism by which this occurs is much less efficient
@@ -85,14 +60,6 @@ long int psx_syscall6(long int syscall_nr,
 		      long int arg4, long int arg5, long int arg6);
 
 /*
- * psx_register registers the current pthread with the psx abstraction
- * of system calls. Typically, there is never any need to call this
- * explicitly because the way the library is linked it is implicitly
- * called when pthread_create() is called.
- */
-void psx_register(void);
-
-/*
  * psx_pthread_create() wraps the -lpthread pthread_create() function
  * call and registers the generated thread with the psx_syscall
  * infrastructure.
diff --git a/psx/psx.c b/psx/psx.c
index 92b43dc..cabd342 100644
--- a/psx/psx.c
+++ b/psx/psx.c
@@ -54,6 +54,7 @@ typedef struct registered_thread_s {
     pthread_t thread;
     pthread_mutex_t mu;
     int pending;
+    int gone;
 } registered_thread_t;
 
 static pthread_once_t psx_tracker_initialized = PTHREAD_ONCE_INIT;
@@ -63,7 +64,8 @@ typedef enum {
     _PSX_SETUP = 1,
     _PSX_SYSCALL = 2,
     _PSX_CREATE = 3,
-    _PSX_INFORK = 4
+    _PSX_INFORK = 4,
+    _PSX_EXITING = 5,
 } psx_tracker_state_t;
 
 /*
@@ -102,7 +104,7 @@ pthread_key_t psx_action_key;
  * the current thread with a TLS specific key pointing at the threads
  * specific tracker.
  */
-static void psx_do_registration(void) {
+static void *psx_do_registration(void) {
     registered_thread_t *node = calloc(1, sizeof(registered_thread_t));
     pthread_mutex_init(&node->mu, NULL);
     node->thread = pthread_self();
@@ -112,6 +114,7 @@ static void psx_do_registration(void) {
 	node->next->prev = node;
     }
     psx_tracker.root = node;
+    return node;
 }
 
 /*
@@ -314,17 +317,6 @@ static void psx_do_unregister(registered_thread_t *node) {
     free(node);
 }
 
-/*
- * psx_register can be used to explicitly register a thread once
- * created. In general, it shouldn't be needed. Further, it should
- * never be used to register the main thread.
- */
-void psx_register(void) {
-    psx_lock();
-    psx_do_registration();
-    psx_unlock();
-}
-
 typedef struct {
     void *(*fn)(void *);
     void *arg;
@@ -332,28 +324,54 @@ typedef struct {
 } psx_starter_t;
 
 /*
+ * _psx_exiting is used to cleanup the node for the thread on its exit
+ * path. This is needed for musl libc:
+ *
+ *    https://bugzilla.kernel.org/show_bug.cgi?id=208477
+ *
+ * and likely wise for glibc too:
+ *
+ *    https://sourceware.org/bugzilla/show_bug.cgi?id=12889
+ */
+static void _psx_exiting(void *node) {
+    psx_new_state(_PSX_IDLE, _PSX_EXITING);
+    registered_thread_t *ref = node;
+    pthread_mutex_lock(&ref->mu);
+    ref->gone = 1;
+    pthread_mutex_unlock(&ref->mu);
+    psx_new_state(_PSX_EXITING, _PSX_IDLE);
+}
+
+/*
  * _psx_start_fn is a trampolene for the intended start function, it
- * is called both blocked and locked, but releases the (b)lock before
- * calling starter->fn. Before releasing the (b)lock the TLS specific
- * attribute(s) are initialized for use by the interrupt handler under
+ * is called blocked (_PSX_CREATE), but releases the block before
+ * calling starter->fn. Before releasing the block, the TLS specific
+ * attributes are initialized for use by the interrupt handler under
  * the psx mutex, so it doesn't race with an interrupt received by
  * this thread and the interrupt handler does not need to poll for
  * that specific attribute to be present (which is problematic during
  * thread shutdown).
  */
 static void *_psx_start_fn(void *data) {
-    psx_do_registration();
+    void *node = psx_do_registration();
+
     psx_new_state(_PSX_CREATE, _PSX_IDLE);
 
     psx_starter_t *starter = data;
     pthread_sigmask(SIG_SETMASK, &starter->sigbits, NULL);
-
     void *(*fn)(void *) = starter->fn;
     void *arg = starter->arg;
+
     memset(data, 0, sizeof(*starter));
     free(data);
 
-    return fn(arg);
+    void *ret;
+
+    pthread_cleanup_push(_psx_exiting, node);
+    ret = fn(arg);
+    pthread_cleanup_pop(1);
+
+    return ret;
 }
 
 /*
@@ -502,8 +520,12 @@ long int __psx_syscall(long int syscall_nr, ...) {
 	}
 	pthread_mutex_lock(&ref->mu);
 	ref->pending = 1;
+	int gone = ref->gone;
+	if (!gone) {
+	    gone = pthread_kill(ref->thread, psx_tracker.psx_sig) != 0;
+	}
 	pthread_mutex_unlock(&ref->mu);
-	if (pthread_kill(ref->thread, psx_tracker.psx_sig) == 0) {
+	if (!gone) {
 	    continue;
 	}
 	/*
@@ -524,8 +546,12 @@ long int __psx_syscall(long int syscall_nr, ...) {
 
 	    pthread_mutex_lock(&ref->mu);
 	    int pending = ref->pending;
+	    int gone = ref->gone;
+	    if (pending && !gone) {
+		gone = (pthread_kill(ref->thread, 0) != 0);
+	    }
 	    pthread_mutex_unlock(&ref->mu);
-	    if (!pending || pthread_kill(ref->thread, 0) == 0) {
+	    if (!gone) {
 		waiting += pending;
 		continue;
 	    }
diff --git a/tests/Makefile b/tests/Makefile
index 3fedeca..bfedbc2 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -18,14 +18,10 @@ sudotest: test run_libcap_launch_test run_libcap_launch_test
 
 install: all
 
-run_psx_test: psx_test psx_test_wrap
+run_psx_test: psx_test
 	./psx_test
-	./psx_test_wrap
 
 psx_test: psx_test.c $(DEPS)
-	$(CC) $(CFLAGS) $(IPATH) -DNOWRAP $< -o $@ $(LIBPSXLIB)
-
-psx_test_wrap: psx_test.c $(DEPS)
 	$(CC) $(CFLAGS) $(IPATH) $< -o $@ $(LIBPSXLIB) -Wl,-wrap,pthread_create
 
 run_libcap_psx_test: libcap_psx_test
@@ -51,5 +47,5 @@ noop: noop.c
 	$(CC) $(CFLAGS) $< -o $@ --static
 
 clean:
-	rm -f psx_test psx_test_wrap libcap_psx_test libcap_launch_test *~
+	rm -f psx_test libcap_psx_test libcap_launch_test *~
 	rm -f libcap_launch_test libcap_psx_launch_test core noop
diff --git a/tests/psx_test.c b/tests/psx_test.c
index 241d535..7f16175 100644
--- a/tests/psx_test.c
+++ b/tests/psx_test.c
@@ -14,13 +14,21 @@
 #include <time.h>
 #include <unistd.h>
 
+typedef union tp {
+    long long unsigned raw;
+    pthread_t pt;
+} thread_ptr;
+
 static void say_hello_expecting(const char *title, int n, int kept) {
     int keeper = prctl(PR_GET_KEEPCAPS);
-    printf("hello [%d], %s<%d> %lx (keepcaps=%d vs. want=%d)\n",
-	   getpid(), title, n, pthread_self(), keeper, kept);
+    thread_ptr tp;
+    tp.pt = pthread_self();
+
+    printf("hello [%d], %s<%d> %llx (keepcaps=%d vs. want=%d)\n",
+	   getpid(), title, n, tp.raw, keeper, kept);
     if (keeper != kept) {
-	printf("--> FAILURE %s thread=%lx has wrong keepcaps: got=%d want=%d\n",
-	       title, pthread_self(), keeper, kept);
+	printf("--> FAILURE %s thread=%llx has wrong keepcaps: got=%d want=%d\n",
+	       title, tp.raw, keeper, kept);
 	exit(1);
     }
 }
@@ -69,6 +77,7 @@ int main(int argc, char **argv) {
     char * const stop_argv[3] = { argv[0], strdup("stop"), NULL };
 
     if (argc != 1) {
+	printf("child %d starting\n", getpid());
 	usleep(2000);
 	printf("child %d exiting\n", getpid());
 	exit(0);
@@ -111,16 +120,7 @@ int main(int argc, char **argv) {
 		}
 	    }
 	    launched++;
-	    if (i == 1) {
-		/* Confirm this works whether or not we are WRAPPING. */
-		psx_pthread_create(&tid[i], NULL, say_hello, NULL);
-	    } else if (i < 3) {
-#ifdef NOWRAP
-		psx_pthread_create(&tid[i], NULL, say_hello, NULL);
-#else
-		pthread_create(&tid[i], NULL, say_hello, NULL);
-#endif
-	    }
+	    pthread_create(&tid[i], NULL, say_hello, NULL);
 	    /* Confirm that the thread is started. */
 	    pthread_mutex_lock(&mu);
 	    while (started < launched) {
@@ -148,7 +148,3 @@ int main(int argc, char **argv) {
     printf("%s PASSED\n", argv[0]);
     exit(0);
 }
-
-#ifdef NOWRAP
-PSX_NO_LINKER_WRAPPING
-#endif