Skip to content

Commit

Permalink
fix: use pipe2 to signal tracee to proceed to exec
Browse files Browse the repository at this point in the history
Previously we raise SIGSTOP in tracee and then waitpid in tracer before seizing it, which yields a group stop(I think this is actually an UNDOCUMENTED behavior) after seizing. It hits some ptrace quirks.

For example, some child processes forked off after exec might caught this phantom ptrace group stop and hangs.

So this patch instead uses a pipe between tracer and root tracee to communicate when to proceed to exec.
  • Loading branch information
kxxt committed Feb 12, 2025
1 parent 1a57764 commit e281ba8
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 42 deletions.
28 changes: 2 additions & 26 deletions src/ptrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,27 +50,12 @@ impl RecursivePtraceEngine {
&mut self,
tracee: Pid,
mut options: nix::sys::ptrace::Options,
) -> Result<PtraceGroupStopGuard<'_>, Errno> {
) -> Result<(), Errno> {
if self.running {
return Err(Errno::EEXIST);
} else {
self.running = true;
}
// In this loop, the tracee is not traced yet.
loop {
let status = waitpid(tracee, Some(WaitPidFlag::WSTOPPED))?;
match status {
WaitStatus::Stopped(_, nix::sys::signal::SIGSTOP) => {
break;
}
WaitStatus::Stopped(_, _) => {
trace!("tracee stopped by other signal, continuing");
continue;
}
_ => unreachable!(), // WSTOPPED wait for children that have been stopped by delivery of a signal.
}
}
trace!("tracee stopped, setting options");
use nix::sys::ptrace::Options;
if self.seccomp {
options |= Options::PTRACE_O_TRACESECCOMP;
Expand All @@ -82,16 +67,7 @@ impl RecursivePtraceEngine {
| Options::PTRACE_O_TRACECLONE
| Options::PTRACE_O_TRACEVFORK,
)?;

// Then we will observe a group stop resulting of the very SIGSTOP

let status = waitpid::waitpid(self, Some(tracee), Some(WaitPidFlag::WSTOPPED))?;
trace!("waitpid event: {status:?}");
match status {
PtraceWaitPidEvent::Signaled { .. } | PtraceWaitPidEvent::Exited { .. } => Err(Errno::ESRCH),
PtraceWaitPidEvent::Ptrace(PtraceStopGuard::Group(guard)) => Ok(guard),
_ => unreachable!(),
}
Ok(())
}

/// Following the convention on ptrace(2), this function expects a child that initiates a `PTRACE_TRACEME`
Expand Down
4 changes: 2 additions & 2 deletions src/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,14 +586,14 @@ fn spawn_command_from_pty_fd(
_ = libc::sigprocmask(libc::SIG_SETMASK, &empty_set, std::ptr::null_mut());
}

pre_exec(&cmd.program).unwrap();

close_random_fds();

if let Some(mask) = configured_umask {
_ = unsafe { libc::umask(mask) };
}

pre_exec(&cmd.program).unwrap();

execv(
&CString::new(cmd.program.into_os_string().into_vec()).unwrap(),
&cmd.args,
Expand Down
46 changes: 32 additions & 14 deletions src/tracer.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use std::{
collections::{BTreeMap, HashMap},
ffi::CString,
io::{self, stdin},
fs::File,
io::{self, stdin, Read, Write},
ops::ControlFlow,
os::fd::AsRawFd,
process::exit,
os::fd::{AsRawFd, FromRawFd, OwnedFd},
sync::{atomic::AtomicU32, Arc, RwLock},
time::Duration,
};
Expand All @@ -17,9 +17,14 @@ use inspect::{read_arcstr, read_output_msg_array};
use nix::{
errno::Errno,
libc::{
self, dup2, pthread_self, pthread_setname_np, raise, AT_EMPTY_PATH, SIGSTOP, S_ISGID, S_ISUID,
self, c_int, dup2, pthread_self, pthread_setname_np, AT_EMPTY_PATH,
S_ISGID, S_ISUID,
},
sys::{
ptrace::AddressType,
stat::fstat,
wait::WaitPidFlag,
},
sys::{ptrace::AddressType, stat::fstat, wait::WaitPidFlag},
unistd::{
getpid, initgroups, setpgid, setresgid, setresuid, setsid, tcsetpgrp, Gid, Pid, Uid, User,
},
Expand Down Expand Up @@ -246,6 +251,14 @@ impl Tracer {
let use_pseudo_term = slave_pty.is_some();
let user = self.user.clone();

let mut fds: [c_int; 2] = [0; 2];
let ret = unsafe { libc::pipe2(fds.as_mut_ptr(), libc::O_CLOEXEC) };
if ret != 0 {
return Err(Errno::last().into());
}
let tracee_fd = unsafe { OwnedFd::from_raw_fd(fds[0]) };
let mut tracer_fd = unsafe { File::from_raw_fd(fds[1]) };
let tracee_raw_fd = tracee_fd.as_raw_fd();
let root_child = pty::spawn_command(slave_pty, cmd, move |program_path| {
#[cfg(feature = "seccomp-bpf")]
if seccomp_bpf == SeccompBpf::On {
Expand Down Expand Up @@ -296,21 +309,22 @@ impl Tracer {
setresuid(user.uid, euid, Uid::from_raw(u32::MAX))?;
}

if 0 != unsafe { raise(SIGSTOP) } {
error!("raise failed!");
exit(-1);
}
trace!("raise success!");
trace!("Waiting for tracer");
let mut tracee_fd = unsafe { File::from_raw_fd(tracee_raw_fd) };
let mut message = [0; 2];
tracee_fd.read_exact(&mut message)?;
trace!("tracee continue to exec");

Ok(())
})?;
filterable_event!(TraceeSpawn(root_child)).send_if_match(&self.msg_tx, self.filter)?;
drop(tracee_fd);
let ptrace_opts = {
use nix::sys::ptrace::Options;
Options::PTRACE_O_TRACEEXEC | Options::PTRACE_O_EXITKILL | Options::PTRACE_O_TRACESYSGOOD
};
let mut engine = RecursivePtraceEngine::new(self.seccomp_bpf());
let guard = engine.seize_children_recursive(root_child, ptrace_opts)?;
engine.seize_children_recursive(root_child, ptrace_opts)?;
let mut root_child_state = ProcessState::new(root_child, 0)?;
root_child_state.ppid = Some(getpid());
{
Expand All @@ -326,9 +340,13 @@ impl Tracer {
r => r?,
}
}
// restart child
trace!("resuming child");
guard.seccomp_aware_cont_syscall(true)?;

// Resume tracee
// Write a message of exactly two bytes to wake up tracee to proceed to exec
tracer_fd.write_all(b"go")?;
drop(tracer_fd);
trace!("Wrote message to tracee");

let mut collect_interval = tokio::time::interval(self.delay);
let mut pending_guards = HashMap::new();

Expand Down

0 comments on commit e281ba8

Please sign in to comment.