Repository navigation
renderd: fix partial-read, signal-handler and malloc-check bugs - #503
Conversation
f799a64 to
d5f8798
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #503 +/- ##
==========================================
+ Coverage 81.54% 81.84% +0.30%
==========================================
Files 24 24
Lines 5645 5375 -270
==========================================
- Hits 4603 4399 -204
+ Misses 1042 976 -66 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV
d5f8798 to
e2bce61
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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, |
There was a problem hiding this comment.
False positive: Flawfinder flags every recv() call. This one reads into the fixed-size struct protocol and is bounded by sizeof(struct protocol) - ret_size, so it can't overrun, and the loop it replaces already called recv() on the same socket.
| size_t file_offset, tile_size; | ||
|
|
||
| if (m == NULL) { | ||
| snprintf(log_msg, PATH_MAX - 1, "Failed to allocate memory for metatile header\n"); |
There was a problem hiding this comment.
False positive: the format string is a constant literal with no arguments, so nothing external can influence it, and snprintf() always NUL-terminates when the size is non-zero. It's the same snprintf(log_msg, PATH_MAX - 1, ...) pattern used for the other errors in this function.
|
Hi @hummeltech, just checking in when you have time. The rest of this series (#503, #504, #506, #508, #510, #511 and #512, plus the draft #513) is up to date with the current |
renderd memory and threading fixes (split out of #497).
mod_tile.c— the recv loop overwrote rather than accumulated the byte count and used struct pointer arithmetic, so partial reads wrote to the wrong place. Also handlesEINTRand EOF.request_exit()calledg_logger/strerror, which are not async-signal-safe; it now only callswrite(2).mallocresults and leaks on error paths inrenderd.c,request_queue.candstore_file.c.Rebased onto the current
master.Independent of the other PRs in this series; it applies to
masteron its own and merges cleanly with each of them in any order.🤖 Generated with Claude Code
https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV