From 58a8f127ad06283a559a8ced89764c72c0d97ebd Mon Sep 17 00:00:00 2001
From: Thomas Merkel <tm@core.io>
Date: Tue, 19 Oct 2021 09:47:03 +0200
Subject: [PATCH] musl thread fix (#1)

* Deduplicate the cleanup in handle_connection()

* Remove the VLA from handle_connection()

This fixes a segfault on musl libc with reasonable sized buffers, as
musl's default thread stack size is quite small (128k since 1.1.21).

A similar bug exists on glibc with large enough buffers (reproducable
with e.g. 16MB on my test system).

This commit fixes #108.

* Remove the superfluous call to pthread_exit

Returning from the start function of a thread is specified to
implicitly call pthread_exit().

* Request a reasonably small thread stack

This somewhat mitigates the problem that now the buffer is allocated in
addition to the already allocated thread stack.

Co-authored-by: Johannes Nixdorf <mixi@exherbo.org>
---
 fiche.c | 58 ++++++++++++++++++++++++---------------------------------
 1 file changed, 24 insertions(+), 34 deletions(-)

diff --git a/fiche.c b/fiche.c
index e21c7b8..130131c 100644
--- a/fiche.c
+++ b/fiche.c
@@ -33,6 +33,7 @@ Use netcat to push text - example:
 #include <stdlib.h>
 #include <string.h>
 
+#include <errno.h>
 #include <pwd.h>
 #include <time.h>
 #include <unistd.h>
@@ -520,13 +521,19 @@ static void dispatch_connection(int socket, Fiche_Settings *settings) {
 
     // Spawn a new thread to handle this connection
     pthread_t id;
+    pthread_attr_t attr;
 
-    if ( pthread_create(&id, NULL, &handle_connection, c) != 0 ) {
+    if ( (errno = pthread_attr_init(&attr)) ||
+         (errno = pthread_attr_setstacksize(&attr, 128*1024)) ||
+         (errno = pthread_create(&id, &attr, &handle_connection, c)) ) {
+        pthread_attr_destroy(&attr);
         print_error("Couldn't spawn a thread!");
         return;
     }
 
     // Detach thread if created successfully
+    pthread_attr_destroy(&attr);
+
     // TODO: consider using pthread_tryjoin_np
     pthread_detach(id);
 
@@ -534,6 +541,8 @@ static void dispatch_connection(int socket, Fiche_Settings *settings) {
 
 
 static void *handle_connection(void *args) {
+    char *slug = NULL;
+    uint8_t *buffer = NULL;
 
     // Cast args to it's previous type
     struct fiche_connection *c = (struct fiche_connection *) args;
@@ -561,22 +570,20 @@ static void *handle_connection(void *args) {
     }
 
     // Create a buffer
-    uint8_t buffer[c->settings->buffer_len];
-    memset(buffer, 0, c->settings->buffer_len);
+    buffer = calloc(c->settings->buffer_len, 1);
+    if (!buffer) {
+        print_error("Couldn't allocate the buffer!");
+        print_separator();
 
-    const int r = recv(c->socket, buffer, sizeof(buffer), MSG_WAITALL);
+        goto exit;
+    }
+
+    const int r = recv(c->socket, buffer, c->settings->buffer_len, MSG_WAITALL);
     if (r <= 0) {
         print_error("No data received from the client!");
         print_separator();
 
-        // Close the socket
-        close(c->socket);
-
-        // Cleanup
-        free(c);
-        pthread_exit(NULL);
-
-        return 0;
+        goto exit;
     }
 
     // - Check if request was performed with a known protocol
@@ -589,7 +596,6 @@ static void *handle_connection(void *args) {
     // TODO
 
     // Generate slug and use it to create an url
-    char *slug;
     uint8_t extra = 0;
 
     do {
@@ -613,12 +619,7 @@ static void *handle_connection(void *args) {
             print_error("Couldn't generate a valid slug!");
             print_separator();
 
-            // Cleanup
-            free(c);
-            free(slug);
-            close(c->socket);
-            pthread_exit(NULL);
-            return NULL;
+            goto exit;
         }
 
     }
@@ -630,12 +631,7 @@ static void *handle_connection(void *args) {
         print_error("Couldn't generate a slug!");
         print_separator();
 
-        close(c->socket);
-
-        // Cleanup
-        free(c);
-        pthread_exit(NULL);
-        return NULL;
+        goto exit;
     }
 
 
@@ -644,13 +640,7 @@ static void *handle_connection(void *args) {
         print_error("Couldn't save a file!");
         print_separator();
 
-        close(c->socket);
-
-        // Cleanup
-        free(c);
-        free(slug);
-        pthread_exit(NULL);
-        return NULL;
+        goto exit;
     }
 
     // Write a response to the user
@@ -672,15 +662,15 @@ static void *handle_connection(void *args) {
     // TODO: log unsuccessful and rejected connections
     log_entry(c->settings, ip, hostname, slug);
 
+exit:
     // Close the connection
     close(c->socket);
 
     // Perform cleanup of values used in this thread
+    free(buffer);
     free(slug);
     free(c);
 
-    pthread_exit(NULL);
-
     return NULL;
 }