Add follow-forks support (-f) to trace child processes#136
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds strace -f-style fork-following to Pinchy by having the userspace daemon discover and auto-register child PIDs when clients opt in via -f/--follow-forks (plumbed over D-Bus).
Changes:
- Extend the D-Bus
trace_pidAPI and CLI to supportfollow_forks(-f/--follow-forks). - Update the daemon dispatch loop to detect successful fork-family exits and register child PIDs for interested clients.
- Add an integration test + UML harness plumbing to pass extra client flags and validate fork-follow behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
uml-kernel/uml-test-runner.sh |
Adds support for passing extra Pinchy client flags from kernel cmdline into the UML test invocation. |
README.md |
Documents -f/--follow-forks behavior and its known “may miss earliest syscalls” limitation. |
pinchy/tests/integration.rs |
Adds a follow_forks integration test asserting the child’s syscall is captured with -f. |
pinchy/tests/common.rs |
Adds run_workload_with_args and passes extra client args through UML cmdline. |
pinchy/src/tracing.rs |
Implements fork-child detection, follow-forks client bookkeeping, and forced fork-family syscall subscription when needed. |
pinchy/src/server.rs |
Extends trace_pid D-Bus method signature to accept follow_forks. |
pinchy/src/client.rs |
Adds CLI flag -f/--follow-forks and passes it through to the client library calls. |
pinchy/src/bin/test-helper.rs |
Adds a fork_test workload used by integration tests. |
pinchy-client/src/lib.rs |
Extends client-library API (attach/trace_child) and D-Bus proxy to pass follow_forks. |
Comment on lines
+1041
to
+1042
| self.add_pid_to_filter(child)?; | ||
| self.start_pidfd_monitoring(child, pidfd).await?; |
Owner
Author
There was a problem hiding this comment.
Fixed and squashed: lupa call sites updated (good catch — lupa is outside default-members so CI never builds it), both rollback paths added, and printf replaces echo in the runner.
| } | ||
|
|
||
| pub async fn attach(pid: u32, syscalls: Vec<i64>) -> OwnedFd { | ||
| pub async fn attach(pid: u32, syscalls: Vec<i64>, follow_forks: bool) -> OwnedFd { |
Owner
Author
There was a problem hiding this comment.
Fixed and squashed: lupa call sites updated (good catch — lupa is outside default-members so CI never builds it), both rollback paths added, and printf replaces echo in the runner.
Comment on lines
+937
to
+942
| let is_new_pid = !self.clients_map.contains_key(&pid); | ||
| self.clients_map.entry(pid).or_default().push(client_info); | ||
|
|
||
| // Add PID to eBPF filter if this is the first client for this PID | ||
| if is_new_pid { | ||
| let mut pid_filter: aya::maps::HashMap<_, u32, u8> = self | ||
| .ebpf | ||
| .map_mut("PID_FILTER") | ||
| .ok_or_else(|| anyhow::anyhow!("PID_FILTER map not found"))? | ||
| .try_into() | ||
| .map_err(|e| anyhow::anyhow!("Map conversion failed: {e}"))?; | ||
| pid_filter | ||
| .insert(pid, 0, 0) | ||
| .map_err(|e| anyhow::anyhow!("Failed to insert PID: {e}"))?; // Start monitoring this PID if we got a pidfd | ||
| self.add_pid_to_filter(pid)?; |
Owner
Author
There was a problem hiding this comment.
Fixed and squashed: lupa call sites updated (good catch — lupa is outside default-members so CI never builds it), both rollback paths added, and printf replaces echo in the runner.
When a client requests follow-forks, the daemon always subscribes the fork family (clone, clone3, and fork/vfork on x86_64) the same way it already does for execve, watches their successful exits in the dispatch loop, and registers the returned child PID for every interested client: the child goes into the eBPF PID filter, gets its own pidfd lifecycle monitoring, and its events flow into the same output stream. clone/clone3 with CLONE_THREAD are skipped, since threads already share the traced thread group. Children are discovered from the parent's fork exit event, so a child's very first syscalls may be missed; this is documented in the README. Client bookkeeping now copes with one client spanning several PIDs: disconnect cleans up every PID left without clients, and the per-UID registration cap counts distinct clients rather than per-PID entries. The D-Bus trace_pid method gains a follow_forks argument, and the client grows the -f/--follow-forks flag. The integration harness gains a way to pass extra client flags through the UML runner. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fifth PR from the review pass (after #132-#135): follow child processes with
-f/--follow-forks, likestrace -f.How it works — the daemon, not eBPF, drives child discovery. When any client requests follow-forks, the daemon always subscribes the fork family (
clone,clone3, andfork/vforkon x86_64) the same way it already doesexecvefor privilege revalidation. The dispatch loop watches successful fork-family exits in traced processes; the positive return value is the child PID. For each interested client the child is registered like a regular traced PID: eBPF PID-filter entry, its own pidfd lifecycle monitoring, events into the same output stream.clone/clone3withCLONE_THREADare skipped — threads already trace via the thread-group check. Authorization carries over (a child inherits the parent's credentials at fork; a later setuid exec is caught by the existing execve revalidation, which applies to children too).Client bookkeeping for one-client-many-PIDs:
Known limitation (documented in README): children are discovered from the parent's fork exit event, so a child's very first syscalls may be missed. Closing that race would need a
sched_process_forkeBPF program with task_struct access; deferred.Plumbing: the D-Bus
trace_pidmethod gains afollow_forksargument; the client grows-f/--follow-forks. The integration harness gains a way to pass extra client flags through the UML runner (PINCHY_TEST_EXTRA_ARGS), and afork_testworkload whose child retries a recognizableopenatover a few hundred ms so the single-CPU UML scheduler can't starve the daemon's filter update before the child runs.🤖 Generated with Claude Code