fix: build error and two runtime crashes in the syscall tracer - #6
Open
ViniciusCestarii wants to merge 3 commits into
Open
fix: build error and two runtime crashes in the syscall tracer#6ViniciusCestarii wants to merge 3 commits into
ViniciusCestarii wants to merge 3 commits into
Conversation
…g #include <time.h>
…ssembly to prevent Message contained out-of-bounds struct pointer
…ullptr): No such process
There was a problem hiding this comment.
Pull request overview
This PR hardens the syscall-based Cap’n Proto RPC tracer by fixing a GCC build break and addressing two tracer crash paths observed during real-world tracing, improving correctness of captured byte streams and tracer robustness.
Changes:
- Fixes a build failure by including the proper header for
clock_gettime(CLOCK_MONOTONIC, ...). - Prevents Cap’n Proto stream corruption by reassembling only the bytes actually read/written (
rc) forread/writeand distributingrcacross iovecs forreadv/writev. - Avoids aborting the tracer when a tracee disappears between
waitpid()andPTRACE_SYSCALLby toleratingESRCH, and improveswaitpid()error handling.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/rpc_tracer.h | Updates syscall handler signatures to use int64_t rc for syscall return values. |
| src/rpc_tracer.cc | Uses actual transferred byte counts for reassembly; tolerates ESRCH on PTRACE_SYSCALL; adds waitpid() error handling. |
| src/rpc_message_recorder.cc | Adds <time.h> to fix missing CLOCK_MONOTONIC / clock_gettime declarations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+125
to
+128
| // Use rc, not count: a partial write transfers fewer bytes than requested. | ||
| const size_t valid_len = static_cast<size_t>(rc); | ||
| std::vector<char> buf(valid_len); | ||
| ReadProcessMemory(tid, addr, valid_len, buf.data()); |
Comment on lines
+208
to
+211
| // Use rc, not count: bytes beyond the actual read are stale and corrupt reassembly. | ||
| const size_t valid_len = static_cast<size_t>(rc); | ||
| std::vector<char> buf(valid_len); | ||
| ReadProcessMemory(tid, addr, valid_len, buf.data()); |
Comment on lines
163
to
165
| struct iovec* iov = reinterpret_cast<struct iovec*>(alloca(sizeof(struct iovec) * iov_count)); | ||
|
|
||
| ReadProcessMemory(tid, iov_addr, sizeof(struct iovec) * iov_count, reinterpret_cast<char*>(iov)); |
Comment on lines
245
to
247
| struct iovec* iov = reinterpret_cast<struct iovec*>(alloca(sizeof(struct iovec) * iov_count)); | ||
|
|
||
| ReadProcessMemory(tid, iov_addr, sizeof(struct iovec) * iov_count, reinterpret_cast<char*>(iov)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes one build failure and two runtime crashes surfaced while tracing this Cap'n Proto client. Each commit maps to one of the errors below.
1. Build error:
CLOCK_MONOTONICwas not declaredrpc_message_recorder.ccusedclock_gettime(CLOCK_MONOTONIC, ...)without including the header that declares them, breaking the build on GCC 16:error: ‘CLOCK_MONOTONIC’ was not declared in this scope
error: ‘clock_gettime’ was not declared in this scope
Fix: add
#include <time.h>.2. Crash: out-of-bounds struct pointer during reassembly
capnp/layout.c++:2204: failed: expected boundsCheck(...); Message contained out-of-bounds struct pointer.
The read/write leave handlers fed the requested byte count (
count/ `iov_lcorrupt the reassembled Cap'n Proto stream.Fix: use
rcas the valid length forread/write, and distributercacross iovecs forreadv/writevso no stale bytes leak.3. Crash:
ptrace(PTRACE_SYSCALL): No such processrpc_tracer.cc:351: failed: ptrace(PTRACE_SYSCALL, tid, nullptr, nullptr): No such process
A tracee can die between
waitpid()and thePTRACE_SYSCALLrestart, so the syscall fails withESRCHand the hardKJ_SYSCALLaborts the whole tracer.Fix: tolerate
ESRCH