From 82f6320e92824c360de0a745447164d67602fa40 Mon Sep 17 00:00:00 2001 From: Waskim2 Date: Sat, 21 Mar 2026 17:01:07 +0300 Subject: [PATCH 1/3] Fix FD leak, use-after-free, and pipe cleanup in server - Close pipe FDs (pipefd[0], pipefd[1]) in free_stream_context, previously only the socket FD was closed causing 2 FDs to leak per stream - Fix use-after-free in callback_close: server_ctx was freed before accessing server_ctx->thread_ctx - Close pipe write-end on stream fin so io_copy thread exits cleanly instead of hanging indefinitely Co-Authored-By: Claude Opus 4.6 --- src/slipstream_server.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/slipstream_server.c b/src/slipstream_server.c index d4f4b07..4dabe7f 100644 --- a/src/slipstream_server.c +++ b/src/slipstream_server.c @@ -257,7 +257,18 @@ static void slipstream_server_free_stream_context(slipstream_server_ctx_t* serve server_ctx->first_stream = stream_ctx->next_stream; } - stream_ctx->fd = close(stream_ctx->fd); + if (stream_ctx->fd >= 0) { + close(stream_ctx->fd); + stream_ctx->fd = -1; + } + if (stream_ctx->pipefd[0] >= 0) { + close(stream_ctx->pipefd[0]); + stream_ctx->pipefd[0] = -1; + } + if (stream_ctx->pipefd[1] >= 0) { + close(stream_ctx->pipefd[1]); + stream_ctx->pipefd[1] = -1; + } free(stream_ctx); } @@ -564,9 +575,11 @@ int slipstream_server_callback(picoquic_cnx_t* cnx, } if (fin_or_event == picoquic_callback_stream_fin) { DBG_PRINTF("[stream_id=%d] fin", stream_ctx->stream_id); - /* Close the local_sock fd */ - close(stream_ctx->fd); - stream_ctx->fd = -1; + /* Close pipe write-end so io_copy thread sees EOF and exits */ + if (stream_ctx->pipefd[1] >= 0) { + close(stream_ctx->pipefd[1]); + stream_ctx->pipefd[1] = -1; + } picoquic_unlink_app_stream_ctx(cnx, stream_id); } break; @@ -591,12 +604,13 @@ int slipstream_server_callback(picoquic_cnx_t* cnx, case picoquic_callback_application_close: /* Received application close */ DBG_PRINTF("Connection closed.", NULL); if (server_ctx != NULL) { + picoquic_network_thread_ctx_t* thread_ctx = server_ctx->thread_ctx; slipstream_server_free_context(server_ctx); + /* Remove the application callback */ + picoquic_set_callback(cnx, NULL, NULL); + picoquic_close(cnx, 0); + picoquic_wake_up_network_thread(thread_ctx); } - /* Remove the application callback */ - picoquic_set_callback(cnx, NULL, NULL); - picoquic_close(cnx, 0); - picoquic_wake_up_network_thread(server_ctx->thread_ctx); break; case picoquic_callback_prepare_to_send: /* Active sending API */ From f813376e755af828aa1d0fb5919148df69bf1e30 Mon Sep 17 00:00:00 2001 From: Waskim2 Date: Sat, 21 Mar 2026 17:34:22 +0300 Subject: [PATCH 2/3] Fix poller thread leak: prevent duplicate pollers per stream slipstream_server_poller threads were created on every prepare_to_send callback with no data available, without checking if one was already active for the stream. This caused hundreds of threads to accumulate over time, exhausting resources and eventually crashing the server. Add polling_active flag to stream context to ensure only one poller thread exists per stream at a time. Co-Authored-By: Claude Opus 4.6 --- src/slipstream_server.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/slipstream_server.c b/src/slipstream_server.c index 4dabe7f..011e80d 100644 --- a/src/slipstream_server.c +++ b/src/slipstream_server.c @@ -195,6 +195,7 @@ typedef struct st_slipstream_server_stream_ctx_t { int pipefd[2]; uint64_t stream_id; volatile sig_atomic_t set_active; + volatile sig_atomic_t polling_active; } slipstream_server_stream_ctx_t; typedef struct st_slipstream_server_ctx_t { @@ -397,7 +398,7 @@ void* slipstream_server_poller(void* arg) { break; } - + args->stream_ctx->polling_active = 0; free(args); pthread_exit(NULL); } @@ -640,23 +641,28 @@ int slipstream_server_callback(picoquic_cnx_t* cnx, (void)picoquic_provide_stream_data_buffer(bytes, 0, 0, 0); DBG_PRINTF("[stream_id=%d] recv->quic_send: empty, disactivate", stream_ctx->stream_id); - slipstream_server_poller_args* args = malloc(sizeof(slipstream_server_poller_args)); - args->fd = stream_ctx->fd; - args->cnx = cnx; - args->server_ctx = server_ctx; - args->stream_ctx = stream_ctx; - - pthread_t thread; - if (pthread_create(&thread, NULL, slipstream_server_poller, args) != 0) { - perror("pthread_create() failed for thread1"); - free(args); - } + if (!stream_ctx->polling_active) { + stream_ctx->polling_active = 1; + + slipstream_server_poller_args* args = malloc(sizeof(slipstream_server_poller_args)); + args->fd = stream_ctx->fd; + args->cnx = cnx; + args->server_ctx = server_ctx; + args->stream_ctx = stream_ctx; + + pthread_t thread; + if (pthread_create(&thread, NULL, slipstream_server_poller, args) != 0) { + perror("pthread_create() failed for thread1"); + stream_ctx->polling_active = 0; + free(args); + } #ifdef __APPLE__ - pthread_setname_np("slipstream_server_poller"); + pthread_setname_np("slipstream_server_poller"); #else - pthread_setname_np(thread, "slipstream_server_poller"); + pthread_setname_np(thread, "slipstream_server_poller"); #endif - pthread_detach(thread); + pthread_detach(thread); + } } if (bytes_read == 0) { DBG_PRINTF("[stream_id=%d] recv: closed stream", stream_ctx->stream_id); From de52a5112d833127ccd86e51053597877b98193e Mon Sep 17 00:00:00 2001 From: Waskim2 Date: Sat, 21 Mar 2026 19:23:49 +0300 Subject: [PATCH 3/3] Fix stream context leak: clean up FDs on all exit paths Stream contexts were never freed in most cases because picoquic_reset_stream() triggered a callback with stream_ctx=NULL, so the reset handler couldn't clean up. Now free_stream_context is called directly before reset in all error/close paths in prepare_to_send and stream_data callbacks: - ioctl failure - recv returning 0 (connection closed) - recv returning -1 (error) - write EPIPE (pipe closed) - write error This was the main source of FD/thread accumulation causing the server to become unresponsive after extended use. Co-Authored-By: Claude Opus 4.6 --- src/slipstream_server.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/slipstream_server.c b/src/slipstream_server.c index 011e80d..4693213 100644 --- a/src/slipstream_server.c +++ b/src/slipstream_server.c @@ -426,12 +426,12 @@ void* slipstream_io_copy(void* arg) { addr_len = sizeof(struct sockaddr_in6); } else { perror("Invalid address family"); - return NULL; + goto cleanup; } if (connect(socket, (struct sockaddr*)&server_ctx->upstream_addr, addr_len) < 0) { perror("connect() failed"); - return NULL; + goto cleanup; } DBG_PRINTF("[%lu:%d] setup pipe done", stream_ctx->stream_id, stream_ctx->fd); @@ -452,7 +452,7 @@ void* slipstream_io_copy(void* arg) { DBG_PRINTF("[%lu:%d] read %d bytes", stream_ctx->stream_id, stream_ctx->fd, bytes_read); if (bytes_read < 0) { perror("recv failed"); - return NULL; + goto cleanup; } else if (bytes_read == 0) { // End of stream - source socket closed connection break; @@ -465,13 +465,17 @@ void* slipstream_io_copy(void* arg) { ssize_t bytes_written = send(socket, p, remaining, 0); if (bytes_written < 0) { perror("send failed"); - return NULL; + goto cleanup; } remaining -= bytes_written; p += bytes_written; } } +cleanup: + /* Don't close FDs here — free_stream_context owns them. + * Closing from this thread races with the main thread's ioctl/recv. */ + free(args); return NULL; } @@ -562,6 +566,8 @@ int slipstream_server_callback(picoquic_cnx_t* cnx, /* Connection closed */ DBG_PRINTF("[stream_id=%d] send: closed stream", stream_ctx->stream_id); + picoquic_unlink_app_stream_ctx(cnx, stream_id); + slipstream_server_free_stream_context(server_ctx, stream_ctx); (void)picoquic_reset_stream(cnx, stream_id, SLIPSTREAM_FILE_CANCEL_ERROR); return 0; } @@ -570,18 +576,16 @@ int slipstream_server_callback(picoquic_cnx_t* cnx, } DBG_PRINTF("[stream_id=%d] send: error: %s (%d)", stream_id, strerror(errno), errno); + picoquic_unlink_app_stream_ctx(cnx, stream_id); + slipstream_server_free_stream_context(server_ctx, stream_ctx); (void)picoquic_reset_stream(cnx, stream_id, SLIPSTREAM_INTERNAL_ERROR); return 0; } } if (fin_or_event == picoquic_callback_stream_fin) { DBG_PRINTF("[stream_id=%d] fin", stream_ctx->stream_id); - /* Close pipe write-end so io_copy thread sees EOF and exits */ - if (stream_ctx->pipefd[1] >= 0) { - close(stream_ctx->pipefd[1]); - stream_ctx->pipefd[1] = -1; - } picoquic_unlink_app_stream_ctx(cnx, stream_id); + slipstream_server_free_stream_context(server_ctx, stream_ctx); } break; case picoquic_callback_stop_sending: /* Should not happen, treated as reset */ @@ -624,7 +628,8 @@ int slipstream_server_callback(picoquic_cnx_t* cnx, // DBG_PRINTF("[stream_id=%d] recv->quic_send (available %d)", stream_id, length_available); if (ret < 0) { DBG_PRINTF("[stream_id=%d] ioctl error: %s (%d)", stream_ctx->stream_id, strerror(errno), errno); - /* TODO: why would it return an error? */ + picoquic_unlink_app_stream_ctx(cnx, stream_id); + slipstream_server_free_stream_context(server_ctx, stream_ctx); (void)picoquic_reset_stream(cnx, stream_id, SLIPSTREAM_INTERNAL_ERROR); break; } @@ -666,6 +671,8 @@ int slipstream_server_callback(picoquic_cnx_t* cnx, } if (bytes_read == 0) { DBG_PRINTF("[stream_id=%d] recv: closed stream", stream_ctx->stream_id); + picoquic_unlink_app_stream_ctx(cnx, stream_id); + slipstream_server_free_stream_context(server_ctx, stream_ctx); (void)picoquic_reset_stream(cnx, stream_id, SLIPSTREAM_FILE_CANCEL_ERROR); return 0; } @@ -687,12 +694,15 @@ int slipstream_server_callback(picoquic_cnx_t* cnx, // DBG_PRINTF("[%lu:%d] recv->quic_send recv done %d bytes into quic\n", stream_id, stream_ctx->fd, bytes_read); if (bytes_read == 0) { DBG_PRINTF("Closed connection on sock %d on recv", stream_ctx->fd); + picoquic_unlink_app_stream_ctx(cnx, stream_id); + slipstream_server_free_stream_context(server_ctx, stream_ctx); (void)picoquic_reset_stream(cnx, stream_id, SLIPSTREAM_FILE_CANCEL_ERROR); return 0; } if (bytes_read < 0) { DBG_PRINTF("recv: %s (%d)", strerror(errno), errno); - /* There should be bytes available, so a return value of 0 is an error */ + picoquic_unlink_app_stream_ctx(cnx, stream_id); + slipstream_server_free_stream_context(server_ctx, stream_ctx); (void)picoquic_reset_stream(cnx, stream_id, SLIPSTREAM_INTERNAL_ERROR); return 0; }