From a2ae03c389090cd7f32e1b9d1268bb7850554444 Mon Sep 17 00:00:00 2001 From: Steve Karmeinsky Date: Thu, 8 Oct 2026 23:58:09 +0100 Subject: [PATCH 1/3] renderd: fix leaks and unchecked malloc results - request_queue.c: check malloc return before bzero; destroy mutex/cond and free queue on failure - renderd.c (slave_thread): check malloc return for req_slave and resp; free req_slave if resp alloc fails - renderd.c: check malloc for render_threads and slave_threads arrays - store_file.c: check malloc return before snprintf; check rename return value and unlink temp file on failure; check malloc(header_len) before use Co-Authored-By: Claude Sonnet 4.6 --- src/renderd.c | 25 +++++++++++++++++++++++++ src/request_queue.c | 9 +++++++++ src/store_file.c | 20 +++++++++++++++++++- 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/renderd.c b/src/renderd.c index cf331baa..73122317 100644 --- a/src/renderd.c +++ b/src/renderd.c @@ -589,7 +589,20 @@ void *slave_thread(void * arg) struct protocol * req_slave; req_slave = (struct protocol *)malloc(sizeof(struct protocol)); + + if (req_slave == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "slave_thread: failed to allocate memory for req_slave"); + return NULL; + } + resp = (struct protocol *)malloc(sizeof(struct protocol)); + + if (resp == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "slave_thread: failed to allocate memory for resp"); + free(req_slave); + return NULL; + } + bzero(req_slave, sizeof(struct protocol)); bzero(resp, sizeof(struct protocol)); @@ -875,6 +888,12 @@ int main(int argc, char **argv) render_threads = (pthread_t *) malloc(sizeof(pthread_t) * config->num_threads); + if (render_threads == NULL) { + g_logger(G_LOG_LEVEL_CRITICAL, "Failed to allocate memory for render threads"); + close(fd); + return 7; + } + for (i = 0; i < config->num_threads; i++) { if (pthread_create(&render_threads[i], NULL, render_thread, (void *)maps)) { g_logger(G_LOG_LEVEL_CRITICAL, "Could not spawn rendering thread"); @@ -888,6 +907,12 @@ int main(int argc, char **argv) k = 0; slave_threads = (pthread_t *) malloc(sizeof(pthread_t) * num_slave_threads); + if (slave_threads == NULL) { + g_logger(G_LOG_LEVEL_CRITICAL, "Failed to allocate memory for slave threads"); + close(fd); + return 7; + } + for (i = 1; i < MAX_SLAVES; i++) { for (j = 0; j < config_slaves[i].num_threads; j++) { if (pthread_create(&slave_threads[k++], NULL, slave_thread, (void *) &config_slaves[i])) { diff --git a/src/request_queue.c b/src/request_queue.c index 832d2e78..6bb89399 100644 --- a/src/request_queue.c +++ b/src/request_queue.c @@ -491,6 +491,15 @@ struct request_queue * request_queue_init() queue->renderHead.next = queue->renderHead.prev = &(queue->renderHead); queue->hashidxSize = HASHIDX_SIZE; queue->item_hashidx = (struct item_idx *) malloc(sizeof(struct item_idx) * queue->hashidxSize); + + if (queue->item_hashidx == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "Failed to allocate memory for request queue hash index"); + pthread_cond_destroy(&(queue->qCond)); + pthread_mutex_destroy(&(queue->qLock)); + free(queue); + return NULL; + } + bzero(queue->item_hashidx, sizeof(struct item_idx) * queue->hashidxSize); return queue; diff --git a/src/store_file.c b/src/store_file.c index 67d60314..398c016d 100644 --- a/src/store_file.c +++ b/src/store_file.c @@ -74,6 +74,11 @@ static int file_tile_read(struct storage_backend * store, const char *xmlconfig, struct meta_layout *m = (struct meta_layout *)malloc(header_len); size_t file_offset, tile_size; + if (m == NULL) { + snprintf(log_msg, PATH_MAX - 1, "Failed to allocate memory for metatile header\n"); + return -1; + } + meta_offset = xyzo_to_meta(path, sizeof(path), (char *)(store->storage_ctx), xmlconfig, options, x, y, z); fd = open(path, O_RDONLY); @@ -219,6 +224,12 @@ static int file_metatile_write(struct storage_backend * store, const char *xmlco g_logger(G_LOG_LEVEL_DEBUG, "Creating and writing a metatile to %s", meta_path); tmp = (char *)malloc(sizeof(char) * strlen(meta_path) + 24); + + if (tmp == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "file_metatile_write: failed to allocate memory for temp path"); + return -1; + } + snprintf(tmp, strlen(meta_path) + 24, "%s.%lu", meta_path, (unsigned long) pthread_self()); if (mkdirp(tmp)) { @@ -246,7 +257,14 @@ static int file_metatile_write(struct storage_backend * store, const char *xmlco } close(fd); - rename(tmp, meta_path); + + if (rename(tmp, meta_path) != 0) { + g_logger(G_LOG_LEVEL_WARNING, "Error renaming temp file %s to %s: %s", tmp, meta_path, strerror(errno)); + unlink(tmp); + free(tmp); + return -1; + } + free(tmp); return sz; From c6068135705658997ac6aa3881ddb46fdfe817b1 Mon Sep 17 00:00:00 2001 From: Steve Karmeinsky Date: Thu, 8 Oct 2026 23:58:09 +0100 Subject: [PATCH 2/3] Fix threading bugs: recv pointer arithmetic, signal handler safety renderd.c slave_thread recv loop: apply the same fix as upstream commit fddb0a9 applied to mod_tile.c -- the loop was overwriting ret_size with each recv() return value instead of accumulating it, and using struct pointer arithmetic (resp + ret_size) instead of byte arithmetic ((char*)resp + ret_size), so partial reads wrote into the wrong memory location. Also handle EINTR and separate n==0 (EOF) from n<0 (error). renderd.c request_exit: remove g_logger/strerror calls from this signal handler. Both functions are non-async-signal-safe (they use mutexes and malloc internally). Only write(2) -- which is async-signal-safe -- is needed here. Co-Authored-By: Claude Sonnet 4.6 --- src/renderd.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/renderd.c b/src/renderd.c index 73122317..cc3f50f2 100644 --- a/src/renderd.c +++ b/src/renderd.c @@ -170,14 +170,11 @@ enum protoCmd rx_request(struct protocol *req, int fd) void request_exit(void) { - // Any write to the exit pipe will trigger a graceful exit + // Any write to the exit pipe will trigger a graceful exit. + // This function is called from a signal handler, so only async-signal-safe + // functions (write(2)) may be used here — no g_logger, no strerror. char c = 0; - - g_logger(G_LOG_LEVEL_INFO, "Sending exit request"); - - if (write(exit_pipe_fd, &c, sizeof(c)) < 0) { - g_logger(G_LOG_LEVEL_ERROR, "Failed to write to the exit pipe: %s", strerror(errno)); - } + (void)write(exit_pipe_fd, &c, sizeof(c)); } void process_loop(int listen_fd) @@ -683,10 +680,27 @@ void *slave_thread(void * arg) retry = 10; while ((ret_size < sizeof(struct protocol)) && (retry > 0)) { - ret_size = recv(pfd, resp + ret_size, (sizeof(struct protocol) - - ret_size), 0); + ssize_t n = recv(pfd, (char *)resp + ret_size, + sizeof(struct protocol) - ret_size, 0); + + if (n < 0) { + if (errno == EINTR) { + continue; + } + + if (errno == EPIPE) { + close(pfd); + pfd = FD_INVALID; + ret_size = 0; + g_logger(G_LOG_LEVEL_ERROR, "Pipe to Renderd slave closed"); + break; + } + + retry--; + continue; + } - if ((errno == EPIPE) || ret_size == 0) { + if (n == 0) { close(pfd); pfd = FD_INVALID; ret_size = 0; @@ -694,7 +708,7 @@ void *slave_thread(void * arg) break; } - retry--; + ret_size += n; } if (ret_size < sizeof(struct protocol)) { From e2bce618c7c48d187aec10de3ee4421c80c1a084 Mon Sep 17 00:00:00 2001 From: Steve Karmeinsky Date: Thu, 8 Oct 2026 23:58:09 +0100 Subject: [PATCH 3/3] renderd: avoid -Wunused-result in request_exit() GCC ignores a (void) cast on functions declared warn_unused_result, so the result of write() in the signal handler is assigned to a variable and discarded instead. Nothing useful can be done on failure there. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV --- src/renderd.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/renderd.c b/src/renderd.c index cc3f50f2..2e2a2547 100644 --- a/src/renderd.c +++ b/src/renderd.c @@ -174,7 +174,10 @@ void request_exit(void) // This function is called from a signal handler, so only async-signal-safe // functions (write(2)) may be used here — no g_logger, no strerror. char c = 0; - (void)write(exit_pipe_fd, &c, sizeof(c)); + // Nothing useful can be done on failure here; assign to discard (GCC ignores a (void) cast + // for functions declared warn_unused_result) + ssize_t ignored = write(exit_pipe_fd, &c, sizeof(c)); + (void)ignored; } void process_loop(int listen_fd)