From cbb17327aad64621c8150736aa2fe3a34764ea96 Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Tue, 16 Jun 2020 13:00:27 +0200 Subject: [PATCH 1/3] ptrace: Block on read, not on write --- src/ptrace.rs | 6 ++++ src/scheme/proc.rs | 68 ++++++++++++++++++++++++---------------------- syscall | 2 +- 3 files changed, 43 insertions(+), 33 deletions(-) diff --git a/src/ptrace.rs b/src/ptrace.rs index b55b3f6..7029d84 100644 --- a/src/ptrace.rs +++ b/src/ptrace.rs @@ -73,6 +73,12 @@ impl SessionData { }); } + /// Returns true if the breakpoint is reached, or if there isn't a + /// breakpoint + pub fn is_reached(&self) -> bool { + self.breakpoint.as_ref().map(|b| b.reached).unwrap_or(false) + } + /// Used for getting the flags in fevent pub fn session_fevent_flags(&self) -> EventFlags { let mut flags = EventFlags::empty(); diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index 37f640d..f80f166 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -365,27 +365,42 @@ impl Scheme for ProcScheme { Ok(len) }, Operation::Trace => { + let mut handles = self.handles.write(); + let handle = handles.get_mut(&id).ok_or(Error::new(EBADF))?; + let data = handle.data.trace_data().expect("operations can't change"); + + // Wait for event + if handle.info.flags & O_NONBLOCK != O_NONBLOCK { + ptrace::wait(handle.info.pid)?; + } + + // Read events let slice = unsafe { slice::from_raw_parts_mut( buf.as_mut_ptr() as *mut PtraceEvent, buf.len() / mem::size_of::() ) }; - let read = ptrace::Session::with_session(info.pid, |session| { - Ok(session.data.lock().recv_events(slice)) + let (read, reached) = ptrace::Session::with_session(info.pid, |session| { + let mut data = session.data.lock(); + Ok((data.recv_events(slice), data.is_reached())) })?; - // Won't context switch, don't worry about the locks - let mut handles = self.handles.write(); - let handle = handles.get_mut(&id).ok_or(Error::new(EBADF))?; - let data = handle.data.trace_data().expect("operations can't change"); - + // Save child processes in a list of processes to restart for event in &slice[..read] { if event.cause == PTRACE_EVENT_CLONE { data.clones.push(ContextId::from(event.a)); } } + // If there are no events, and breakpoint isn't reached, we + // must not have waited. + if read == 0 && !reached { + assert!(handle.info.flags & O_NONBLOCK == O_NONBLOCK, "wait woke up spuriously??"); + return Err(Error::new(EAGAIN)); + } + + // Return read events Ok(read * mem::size_of::()) } } @@ -470,15 +485,12 @@ impl Scheme for ProcScheme { let op = u64::from_ne_bytes(bytes); let op = PtraceFlags::from_bits(op).ok_or(Error::new(EINVAL))?; - let should_continue = !op.contains(PTRACE_FLAG_WAIT) || op.intersects(PTRACE_STOP_MASK); - // Set next breakpoint ptrace::Session::with_session(info.pid, |session| { - if op.intersects(PTRACE_STOP_MASK) { - session.data.lock().set_breakpoint(Some(op)); - } else if should_continue { - session.data.lock().set_breakpoint(None); - } + session.data.lock().set_breakpoint( + Some(op) + .filter(|op| op.intersects(PTRACE_STOP_MASK | PTRACE_EVENT_MASK)) + ); Ok(()) })?; @@ -497,25 +509,17 @@ impl Scheme for ProcScheme { })?; } - // Continue execution, if requested - if should_continue { - // disable the ptrace_stop flag, which is used in some cases - with_context_mut(info.pid, |context| { - context.ptrace_stop = false; - Ok(()) - })?; + // disable the ptrace_stop flag, which is used in some cases + with_context_mut(info.pid, |context| { + context.ptrace_stop = false; + Ok(()) + })?; - // and notify the tracee's WaitCondition, which is used in other cases - ptrace::Session::with_session(info.pid, |session| { - session.tracee.notify(); - Ok(()) - })?; - } - - // And await the tracee, if requested - if op.contains(PTRACE_FLAG_WAIT) || info.flags & O_NONBLOCK != O_NONBLOCK { - ptrace::wait(info.pid)?; - } + // and notify the tracee's WaitCondition, which is used in other cases + ptrace::Session::with_session(info.pid, |session| { + session.tracee.notify(); + Ok(()) + })?; Ok(mem::size_of::()) } diff --git a/syscall b/syscall index 6ba71e7..1c637e7 160000 --- a/syscall +++ b/syscall @@ -1 +1 @@ -Subproject commit 6ba71e7e065d37309b7d5c4fc86835dd350f61b7 +Subproject commit 1c637e72b2f3be8e8f942372e8414101e463df98 From 9c891384ea08755fe117333d3ecbbcd4cab64dbf Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Tue, 16 Jun 2020 13:42:04 +0200 Subject: [PATCH 2/3] Fix ptrace returning ENODEV when process exists --- src/ptrace.rs | 25 +++++++++++++++++-------- src/scheme/proc.rs | 3 +++ src/syscall/process.rs | 6 +++--- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/ptrace.rs b/src/ptrace.rs index 7029d84..90ca718 100644 --- a/src/ptrace.rs +++ b/src/ptrace.rs @@ -112,7 +112,11 @@ impl Session { F: FnOnce(&Session) -> Result, { let sessions = sessions(); - let session = sessions.get(&pid).ok_or(Error::new(ENODEV))?; + let session = sessions.get(&pid).ok_or_else(|| { + println!("session doesn't exist - returning ENODEV."); + println!("can this ever happen?"); + Error::new(ENODEV) + })?; callback(session) } @@ -160,6 +164,18 @@ pub fn close_session(pid: ContextId) { if let Some(session) = sessions_mut().remove(&pid) { session.tracer.notify(); session.tracee.notify(); + } +} + +/// Wake up the tracer to make sure it catches on that the tracee is dead. This +/// is different from `close_session` in that it doesn't actually close the +/// session, and instead waits for the file handle to be closed, where the +/// session will *actually* be closed. This is partly to ensure ENOSRCH is +/// returned rather than ENODEV (which occurs when there's no session - should +/// never really happen). +pub fn close_tracee(pid: ContextId) { + if let Some(session) = sessions().get(&pid) { + session.tracer.notify(); let data = session.data.lock(); proc_trigger_event(data.file_id, EVENT_READ); @@ -250,13 +266,6 @@ pub fn wait(pid: ContextId) -> Result<()> { } } - let contexts = context::contexts(); - let context = contexts.get(pid).ok_or(Error::new(ESRCH))?; - let context = context.read(); - if let Status::Exited(_) = context.status { - return Err(Error::new(ESRCH)); - } - Ok(()) } diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index f80f166..64346ee 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -374,6 +374,9 @@ impl Scheme for ProcScheme { ptrace::wait(handle.info.pid)?; } + // Check if context exists + with_context(handle.info.pid, |_| Ok(()))?; + // Read events let slice = unsafe { slice::from_raw_parts_mut( diff --git a/src/syscall/process.rs b/src/syscall/process.rs index cf1737b..18de7f2 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -1065,6 +1065,8 @@ pub fn fexec(fd: FileHandle, arg_ptrs: &[[usize; 2]], var_ptrs: &[[usize; 2]]) - } pub fn exit(status: usize) -> ! { + ptrace::breakpoint_callback(PTRACE_STOP_EXIT, Some(ptrace_event!(PTRACE_STOP_EXIT, status))); + { let context_lock = { let contexts = context::contexts(); @@ -1072,8 +1074,6 @@ pub fn exit(status: usize) -> ! { Arc::clone(&context_lock) }; - ptrace::breakpoint_callback(PTRACE_STOP_EXIT, Some(ptrace_event!(PTRACE_STOP_EXIT, status))); - let mut close_files = Vec::new(); let pid = { let mut context = context_lock.write(); @@ -1152,7 +1152,7 @@ pub fn exit(status: usize) -> ! { } // Alert any tracers waiting of this process - ptrace::close_session(pid); + ptrace::close_tracee(pid); if pid == ContextId::from(1) { println!("Main kernel thread exited with status {:X}", status); From edcc39929d13782c720c8f7b42c53ad21c07bc39 Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Tue, 16 Jun 2020 13:58:36 +0200 Subject: [PATCH 3/3] Fix unused import I added that assert, because I managed to get an error I couldn't reproduce --- src/ptrace.rs | 2 +- src/scheme/proc.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ptrace.rs b/src/ptrace.rs index 90ca718..d098d9e 100644 --- a/src/ptrace.rs +++ b/src/ptrace.rs @@ -13,7 +13,7 @@ use crate::{ } }, common::unique::Unique, - context::{self, signal, Context, ContextId, Status}, + context::{self, signal, Context, ContextId}, event, scheme::proc, sync::WaitCondition, diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index 64346ee..07f9d91 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -345,6 +345,7 @@ impl Scheme for ProcScheme { })?, RegsKind::Int => try_stop_context(info.pid, |context| match unsafe { ptrace::regs_for(&context) } { None => { + assert!(!context.running, "try_stop_context is broken, clearly"); println!("{}:{}: Couldn't read registers from stopped process", file!(), line!()); Err(Error::new(ENOTRECOVERABLE)) },