diff --git a/src/agent/debugger/src/debug_event.rs b/src/agent/debugger/src/debug_event.rs index 7be2654ee..966540489 100644 --- a/src/agent/debugger/src/debug_event.rs +++ b/src/agent/debugger/src/debug_event.rs @@ -41,35 +41,39 @@ impl<'a> Display for DebugEventInfo<'a> { Exception(info) => { write!( formatter, - "event=Exception exception_code=0x{:08x} exception_address=0x{:08x} first_chance={}", + "Exception code=0x{:08x} address=0x{:08x} {}", info.ExceptionRecord.ExceptionCode, info.ExceptionRecord.ExceptionAddress as u64, - info.dwFirstChance != 0 + if info.dwFirstChance == 0 { + "second_chance" + } else { + "first_chance" + } )?; } - CreateThread(_info) => { - write!(formatter, "event=CreateThread")?; + CreateThread(info) => { + write!(formatter, "CreateThread handle={:x}", info.hThread as u64)?; } CreateProcess(info) => { let image_name = get_path_from_handle(info.hFile).unwrap_or_else(|_| "???".into()); write!( formatter, - "event=CreateProcess name={} base=0x{:016x}", + "CreateProcess name={} base=0x{:016x}", Path::new(&image_name).display(), info.lpBaseOfImage as u64, )?; } ExitThread(info) => { - write!(formatter, "event=ExitThread exit_code={}", info.dwExitCode)?; + write!(formatter, "ExitThread exit_code={}", info.dwExitCode)?; } ExitProcess(info) => { - write!(formatter, "event=ExitProcess exit_code={}", info.dwExitCode)?; + write!(formatter, "ExitProcess exit_code={}", info.dwExitCode)?; } LoadDll(info) => { let image_name = get_path_from_handle(info.hFile).unwrap_or_else(|_| "???".into()); write!( formatter, - "event=LoadDll name={} base=0x{:016x}", + "LoadDll name={} base=0x{:016x}", Path::new(&image_name).display(), info.lpBaseOfDll as u64, )?; @@ -77,26 +81,26 @@ impl<'a> Display for DebugEventInfo<'a> { UnloadDll(info) => { write!( formatter, - "event=UnloadDll base=0x{:016x}", + "UnloadDll base=0x{:016x}", info.lpBaseOfDll as u64, )?; } OutputDebugString(info) => { write!( formatter, - "event=OutputDebugString unicode={} address=0x{:016x} length={}", + "OutputDebugString unicode={} address=0x{:016x} length={}", info.fUnicode, info.lpDebugStringData as u64, info.nDebugStringLength, )?; } Rip(info) => { write!( formatter, - "event=Rip error=0x{:x} type={}", + "Rip error=0x{:x} type={}", info.dwError, info.dwType )?; } Unknown => { - write!(formatter, "event=Unknown")?; + write!(formatter, "Unknown debug event")?; } }; @@ -153,7 +157,7 @@ impl<'a> Display for DebugEvent<'a> { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { write!( formatter, - " pid={} tid={} {}", + "{:x}.{:x} {}", self.process_id, self.thread_id, self.info )?; Ok(()) diff --git a/src/agent/debugger/src/debugger.rs b/src/agent/debugger/src/debugger.rs index bdf5b6dc7..32728e1e9 100644 --- a/src/agent/debugger/src/debugger.rs +++ b/src/agent/debugger/src/debugger.rs @@ -46,7 +46,7 @@ const STATUS_WX86_BREAKPOINT: u32 = ::winapi::shared::ntstatus::STATUS_WX86_BREA /// Uniquely identify a breakpoint. #[derive(Copy, Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] -pub struct BreakpointId(pub u32); +pub struct BreakpointId(pub u64); #[derive(Copy, Clone)] pub(crate) enum StepState { @@ -68,6 +68,10 @@ pub(crate) struct ModuleBreakpoint { } impl ModuleBreakpoint { + pub fn new(rva: u64, kind: BreakpointType, id: BreakpointId) -> Self { + ModuleBreakpoint { rva, kind, id } + } + pub fn rva(&self) -> u64 { self.rva } @@ -226,7 +230,7 @@ pub struct Debugger { continue_args: Option, registered_breakpoints: HashMap>, symbolic_breakpoints: HashMap>, - breakpoint_count: u32, + breakpoint_count: u64, } impl Debugger { @@ -255,7 +259,7 @@ impl Debugger { let de = unsafe { de.assume_init() }; let de = DebugEvent::new(&de); if let DebugEventInfo::CreateProcess(info) = de.info() { - trace!("DebugEvent: {}", de); + trace!("{}", de); let mut target = Target::new(de.process_id(), de.thread_id(), info.hProcess, info.hThread); @@ -342,7 +346,7 @@ impl Debugger { .entry(module.into()) .or_insert_with(|| vec![]); - module_breakpoints.push(ModuleBreakpoint { rva, kind, id }); + module_breakpoints.push(ModuleBreakpoint::new(rva, kind, id)); id } @@ -369,7 +373,7 @@ impl Debugger { if unsafe { WaitForDebugEvent(de.as_mut_ptr(), timeout_ms) } == TRUE { let de = unsafe { de.assume_init() }; let de = DebugEvent::new(&de); - trace!("DebugEvent: {}", de); + trace!("{}", de); let continue_status = self.dispatch_event(&de, callbacks); self.continue_args = Some(ContinueDebugEventArguments { @@ -601,11 +605,11 @@ impl Debugger { &bp.sym, ) { Ok(sym) => { - rva_breakpoints.push(ModuleBreakpoint { - rva: sym.address() - base_address, - kind: bp.kind, - id: bp.id, - }); + rva_breakpoints.push(ModuleBreakpoint::new( + sym.address() - base_address, + bp.kind, + bp.id, + )); } Err(e) => { debug!( @@ -665,7 +669,7 @@ impl Debugger { } pub fn get_current_thread_id(&self) -> u64 { - self.target.current_thread_handle() as u64 + self.target.current_thread_id() as u64 } pub fn read_register_u64(&mut self, reg: iced_x86::Register) -> Result { diff --git a/src/agent/debugger/src/target.rs b/src/agent/debugger/src/target.rs index da6ef63a4..b2db14efb 100644 --- a/src/agent/debugger/src/target.rs +++ b/src/agent/debugger/src/target.rs @@ -9,10 +9,14 @@ use std::{ use anyhow::Result; use log::{error, trace}; -use win_util::{file, handle::Handle, process}; +use win_util::{file, handle::Handle, last_os_error, process}; use winapi::{ shared::minwindef::{DWORD, LPVOID}, - um::winnt::{HANDLE, IMAGE_FILE_MACHINE_AMD64, IMAGE_FILE_MACHINE_I386}, + um::{ + processthreadsapi::{ResumeThread, SuspendThread}, + winbase::Wow64SuspendThread, + winnt::{HANDLE, IMAGE_FILE_MACHINE_AMD64, IMAGE_FILE_MACHINE_I386}, + }, }; use crate::{ @@ -20,6 +24,59 @@ use crate::{ debugger::{Breakpoint, BreakpointId, BreakpointType, ModuleBreakpoint, StepState}, }; +struct ThreadInfo { + id: u32, + handle: HANDLE, + suspended: bool, + wow64: bool, +} + +impl ThreadInfo { + fn new(id: u32, handle: HANDLE, wow64: bool) -> Self { + ThreadInfo { + id, + handle, + wow64, + suspended: false, + } + } + + fn resume_thread(&mut self) -> Result<()> { + if !self.suspended { + return Ok(()); + } + + let suspend_count = unsafe { ResumeThread(self.handle) }; + if suspend_count == (-1i32 as DWORD) { + Err(last_os_error()) + } else { + self.suspended = false; + trace!("Resume {:x} - suspend_count: {}", self.id, suspend_count); + Ok(()) + } + } + + fn suspend_thread(&mut self) -> Result<()> { + if self.suspended { + return Ok(()); + } + + let suspend_count = if self.wow64 { + unsafe { Wow64SuspendThread(self.handle) } + } else { + unsafe { SuspendThread(self.handle) } + }; + + if suspend_count == (-1i32 as DWORD) { + Err(last_os_error()) + } else { + self.suspended = true; + trace!("Suspend {:x} - suspend_count: {}", self.id, suspend_count); + Ok(()) + } + } +} + #[derive(Clone)] pub struct Module { path: PathBuf, @@ -91,15 +148,17 @@ pub struct Target { process_id: DWORD, process_handle: HANDLE, current_thread_handle: HANDLE, + current_thread_id: DWORD, saw_initial_bp: bool, saw_initial_wow64_bp: bool, + wow64: bool, // Track if we need to call SymInitialize for the process and if we need to notify // dbghelp about loaded/unloaded dlls. sym_initialized: bool, exited: bool, - thread_handles: fnv::FnvHashMap, + thread_info: fnv::FnvHashMap, // We cache the current thread context for possible repeated queries and modifications. // We want to call GetThreadContext once, then call SetThreadContext (if necessary) before @@ -127,17 +186,20 @@ impl Target { thread_handle: HANDLE, ) -> Self { let mut thread_handles = fnv::FnvHashMap::default(); - thread_handles.insert(thread_id, thread_handle); + let wow64 = process::is_wow64_process(process_handle); + thread_handles.insert(thread_id, ThreadInfo::new(thread_id, thread_handle, wow64)); Self { process_id, current_thread_handle: thread_handle, + current_thread_id: thread_id, process_handle, saw_initial_bp: false, saw_initial_wow64_bp: false, + wow64, sym_initialized: false, exited: false, - thread_handles, + thread_info: thread_handles, current_context: None, context_is_modified: false, modules: fnv::FnvHashMap::default(), @@ -150,17 +212,24 @@ impl Target { self.current_thread_handle } + pub fn current_thread_id(&self) -> DWORD { + self.current_thread_id + } + pub fn create_new_thread(&mut self, thread_handle: HANDLE, thread_id: DWORD) { self.current_thread_handle = thread_handle; - self.thread_handles.insert(thread_id, thread_handle); + self.thread_info.insert( + thread_id, + ThreadInfo::new(thread_id, thread_handle, self.wow64), + ); } pub fn set_current_thread(&mut self, thread_id: DWORD) { - self.current_thread_handle = *self.thread_handles.get(&thread_id).unwrap(); + self.current_thread_handle = self.thread_info.get(&thread_id).unwrap().handle; } pub fn exit_thread(&mut self, thread_id: DWORD) { - self.thread_handles.remove(&thread_id); + self.thread_info.remove(&thread_id); } pub fn process_handle(&self) -> HANDLE { @@ -491,23 +560,50 @@ impl Target { } }; - let context = self.get_current_context_mut()?; - context.set_program_counter(pc); + let single_step = { + let context = self.get_current_context_mut()?; + context.set_program_counter(pc); - // We need to single step if we need to restore the breakpoint. - let single_step = match handle_breakpoint { - HandleBreakpoint::User(_, single_step) => single_step, + // We need to single step if we need to restore the breakpoint. + let single_step = match handle_breakpoint { + HandleBreakpoint::User(_, single_step) => single_step, - // Single step only when in a recursive call, which is inferred when the current - // stack pointer (from context) is less then at the target to step out from (rsp). - // Note this only works if the stack grows down. - HandleBreakpoint::StepOut(rsp) => rsp > context.stack_pointer(), + // Single step only when in a recursive call, which is inferred when the current + // stack pointer (from context) is less then at the target to step out from (rsp). + // Note this only works if the stack grows down. + HandleBreakpoint::StepOut(rsp) => rsp > context.stack_pointer(), + }; + + if single_step { + context.set_single_step(true); + self.single_step + .insert(self.current_thread_handle, StepState::Breakpoint { pc }); + } + + single_step }; + let current_thread_handle = self.current_thread_handle; + let context = self.get_current_context_mut()?; + context.set_thread_context(current_thread_handle)?; + self.context_is_modified = false; + if single_step { - context.set_single_step(true); - self.single_step - .insert(self.current_thread_handle, StepState::Breakpoint { pc }); + for thread_info in self.thread_info.values_mut() { + // Don't suspend any thread that we are single stepping. Typically + // this is only the current thread/breakpoint, but debug events can be hit + // on multiple threads at roughly the same time, so e.g. we may see a second + // breakpoint from the OS before our scheduled single step exception. + // + // We must resume at least 1 thread when we are single stepping, but we can + // safely resume all threads that are single stepping because we won't miss + // any breakpoints as they are executing a single instruction. + if self.single_step.contains_key(&thread_info.handle) { + thread_info.resume_thread()?; + } else { + thread_info.suspend_thread()?; + }; + } } Ok(match handle_breakpoint { @@ -517,14 +613,22 @@ impl Target { } pub(crate) fn handle_single_step(&mut self, step_state: StepState) -> Result<()> { + self.single_step.remove(&self.current_thread_handle); + match step_state { StepState::Breakpoint { pc } => { write_instruction_byte(self.process_handle, pc, 0xcc)?; + + // Resume all threads if we aren't waiting for any threads to single step. + if self.single_step.is_empty() { + for thread_info in self.thread_info.values_mut() { + thread_info.resume_thread()?; + } + } } _ => {} } - self.single_step.remove(&self.current_thread_handle); Ok(()) }