From 5709598f66ada9dddbb0b9071de7ad95681cb2f8 Mon Sep 17 00:00:00 2001 From: Johannes Nixdorf Date: Sat, 3 Apr 2021 11:28:20 +0200 Subject: [PATCH 1/4] Deduplicate the cleanup in handle_connection() --- fiche.c | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/fiche.c b/fiche.c index 99d140d..54c0cda 100644 --- a/fiche.c +++ b/fiche.c @@ -534,6 +534,7 @@ static void dispatch_connection(int socket, Fiche_Settings *settings) { static void *handle_connection(void *args) { + char *slug = NULL; // Cast args to it's previous type struct fiche_connection *c = (struct fiche_connection *) args; @@ -569,14 +570,7 @@ static void *handle_connection(void *args) { 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 +583,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 +606,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 +618,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 +627,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,6 +649,7 @@ 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); From d428ccf1062aed8f91f4aa1fb327e9edb7cf7159 Mon Sep 17 00:00:00 2001 From: Johannes Nixdorf Date: Sat, 3 Apr 2021 11:31:05 +0200 Subject: [PATCH 2/4] 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. --- fiche.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/fiche.c b/fiche.c index 54c0cda..c4e2c87 100644 --- a/fiche.c +++ b/fiche.c @@ -535,6 +535,7 @@ 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; @@ -562,10 +563,15 @@ 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(); + + goto exit; + } - const int r = recv(c->socket, buffer, sizeof(buffer), MSG_WAITALL); + 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(); @@ -654,6 +660,7 @@ static void *handle_connection(void *args) { close(c->socket); // Perform cleanup of values used in this thread + free(buffer); free(slug); free(c); From a16592263a1d30e808bc8cf4bb3b43aeb083b30f Mon Sep 17 00:00:00 2001 From: Johannes Nixdorf Date: Sat, 3 Apr 2021 11:35:16 +0200 Subject: [PATCH 3/4] Remove the superfluous call to pthread_exit Returning from the start function of a thread is specified to implicitly call pthread_exit(). --- fiche.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fiche.c b/fiche.c index c4e2c87..370dbac 100644 --- a/fiche.c +++ b/fiche.c @@ -664,8 +664,6 @@ static void *handle_connection(void *args) { free(slug); free(c); - pthread_exit(NULL); - return NULL; } From 0771a63bcd1385b5aeee628aacda1c41169c4a42 Mon Sep 17 00:00:00 2001 From: Johannes Nixdorf Date: Sat, 3 Apr 2021 12:02:40 +0200 Subject: [PATCH 4/4] 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. --- fiche.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fiche.c b/fiche.c index 370dbac..de48b15 100644 --- a/fiche.c +++ b/fiche.c @@ -33,6 +33,7 @@ Use netcat to push text - example: #include #include +#include #include #include #include @@ -520,12 +521,18 @@ 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; } + pthread_attr_destroy(&attr); + // Detach thread if created succesfully // TODO: consider using pthread_tryjoin_np pthread_detach(id);