diff --git a/src/bin/test.rs b/src/bin/test.rs index 2681343c..7051ee16 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -80,7 +80,7 @@ mod linux { use minidump_writer::process_reader::ProcessReader; let ppid = getppid().as_raw(); - let dumper = fail_on_soft_error!( + let mut dumper = fail_on_soft_error!( soft_errors, MinidumpWriterConfig::new(ppid, ppid).build_for_testing(&mut soft_errors)? ); @@ -91,7 +91,7 @@ mod linux { let expected_stack = 0x11223344usize.to_ne_bytes(); let expected_heap = 0x55667788usize.to_ne_bytes(); - let validate = |reader: &mut ProcessReader| -> Result<()> { + let validate = |reader: &ProcessReader| -> Result<()> { let mut val = [0u8; std::mem::size_of::()]; let read = reader.read(stack_var, &mut val)?; assert_eq!(read, val.len()); @@ -106,33 +106,45 @@ mod linux { // virtual mem { - let mut mr = ProcessReader::for_virtual_mem(ppid); - validate(&mut mr) - .map_err(|err| format!("failed to validate memory for {mr:?}: {err}"))?; + dumper.process_inspector.force_pr_virtual_mem(); + validate(dumper.process_inspector.process_reader()) + .map_err(|err| format!("failed to validate memory: {err}"))?; } // file { - let mut mr = ProcessReader::for_file(ppid) + dumper + .process_inspector + .force_pr_file() .map_err(|err| format!("failed to open `/proc/{ppid}/mem`: {err}"))?; - validate(&mut mr) - .map_err(|err| format!("failed to validate memory for {mr:?}: {err}"))?; + validate(dumper.process_inspector.process_reader()) + .map_err(|err| format!("failed to validate memory: {err}"))?; } // ptrace { - let mut mr = ProcessReader::for_ptrace(ppid); - validate(&mut mr) - .map_err(|err| format!("failed to validate memory for {mr:?}: {err}"))?; + dumper.process_inspector.force_pr_ptrace(); + validate(dumper.process_inspector.process_reader()) + .map_err(|err| format!("failed to validate memory: {err}"))?; } - let stack_res = - MinidumpWriter::copy_from_process(ppid, stack_var, std::mem::size_of::())?; + dumper.process_inspector.force_pr_reset(); + + let stack_res = MinidumpWriter::copy_from_process( + &dumper.process_inspector, + ppid, + stack_var, + std::mem::size_of::(), + )?; test!(stack_res == expected_stack, "stack var not correct"); - let heap_res = - MinidumpWriter::copy_from_process(ppid, heap_var, std::mem::size_of::())?; + let heap_res = MinidumpWriter::copy_from_process( + &dumper.process_inspector, + ppid, + heap_var, + std::mem::size_of::(), + )?; test!(heap_res == expected_heap, "heap var not correct"); @@ -225,8 +237,10 @@ mod linux { if mapping.name == Some(LINUX_GATE_LIBRARY_NAME.into()) { found_linux_gate = true; - let module_reader::BuildId(id) = - MinidumpWriter::from_process_memory_for_mapping(&mapping, ppid)?; + let module_reader::BuildId(id) = MinidumpWriter::from_process_memory_for_mapping( + &dumper.process_inspector, + &mapping, + )?; test!(!id.is_empty(), "id-vec is empty"); test!(id.iter().any(|&x| x > 0), "all id elements are 0"); drop(dumper); diff --git a/src/linux/android.rs b/src/linux/android.rs index 799d3491..9a76f8c8 100644 --- a/src/linux/android.rs +++ b/src/linux/android.rs @@ -1,7 +1,7 @@ use { super::{ Pid, maps_reader::MappingInfo, minidump_writer::MinidumpWriter, - process_reader::CopyFromProcessError, + process_inspection::ProcessInspector, process_reader::CopyFromProcessError, }, goblin::elf, }; @@ -48,11 +48,16 @@ struct DynVaddresses { dyn_count: usize, } -fn has_android_packed_relocations(pid: Pid, load_bias: usize, vaddrs: DynVaddresses) -> Result<()> { +fn has_android_packed_relocations( + process_inspector: &ProcessInspector, + pid: Pid, + load_bias: usize, + vaddrs: DynVaddresses, +) -> Result<()> { let dyn_addr = load_bias + vaddrs.dyn_vaddr; for idx in 0..vaddrs.dyn_count { let addr = dyn_addr + SIZEOF_DYN * idx; - let dyn_data = MinidumpWriter::copy_from_process(pid, addr, SIZEOF_DYN)?; + let dyn_data = MinidumpWriter::copy_from_process(process_inspector, pid, addr, SIZEOF_DYN)?; // TODO: Couldn't find a nice way to use goblin for that, to avoid the unsafe-block let dyn_obj: Dyn; unsafe { @@ -66,14 +71,19 @@ fn has_android_packed_relocations(pid: Pid, load_bias: usize, vaddrs: DynVaddres Err(AndroidError::NoRelFound) } -fn get_effective_load_bias(pid: Pid, ehdr: &elf_header::Header, address: usize) -> usize { - let ph = parse_loaded_elf_program_headers(pid, ehdr, address); +fn get_effective_load_bias( + process_inspector: &ProcessInspector, + pid: Pid, + ehdr: &elf_header::Header, + address: usize, +) -> usize { + let ph = parse_loaded_elf_program_headers(process_inspector, pid, ehdr, address); // If |min_vaddr| is non-zero and we find Android packed relocation tags, // return the effective load bias. if ph.min_vaddr != 0 { let load_bias = address - ph.min_vaddr; - if has_android_packed_relocations(pid, load_bias, ph).is_ok() { + if has_android_packed_relocations(process_inspector, pid, load_bias, ph).is_ok() { return load_bias; } } @@ -83,6 +93,7 @@ fn get_effective_load_bias(pid: Pid, ehdr: &elf_header::Header, address: usize) } fn parse_loaded_elf_program_headers( + process_inspector: &ProcessInspector, pid: Pid, ehdr: &elf_header::Header, address: usize, @@ -93,6 +104,7 @@ fn parse_loaded_elf_program_headers( let mut dyn_count = 0; let phdr_opt = MinidumpWriter::copy_from_process( + process_inspector, pid, phdr_addr, elf_header::SIZEOF_EHDR * ehdr.e_phnum as usize, @@ -122,17 +134,25 @@ fn parse_loaded_elf_program_headers( } } -pub fn late_process_mappings(pid: Pid, mappings: &mut [MappingInfo]) -> Result<()> { +pub fn late_process_mappings( + process_inspector: &ProcessInspector, + pid: Pid, + mappings: &mut [MappingInfo], +) -> Result<()> { // Only consider exec mappings that indicate a file path was mapped, and // where the ELF header indicates a mapped shared library. for map in mappings .iter_mut() .filter(|m| m.is_executable() && m.name_is_path()) { - let ehdr_opt = - MinidumpWriter::copy_from_process(pid, map.start_address, elf_header::SIZEOF_EHDR) - .ok() - .and_then(|x| elf_header::Header::parse(&x).ok()); + let ehdr_opt = MinidumpWriter::copy_from_process( + process_inspector, + pid, + map.start_address, + elf_header::SIZEOF_EHDR, + ) + .ok() + .and_then(|x| elf_header::Header::parse(&x).ok()); if let Some(ehdr) = ehdr_opt { if ehdr.e_type == elf_header::ET_DYN { @@ -142,7 +162,8 @@ pub fn late_process_mappings(pid: Pid, mappings: &mut [MappingInfo]) -> Result<( // the library does not contain Android packed relocations, // GetEffectiveLoadBias() returns |start_addr| and the mapping entry // is not changed. - let load_bias = get_effective_load_bias(pid, &ehdr, map.start_address); + let load_bias = + get_effective_load_bias(process_inspector, pid, &ehdr, map.start_address); map.size += map.start_address - load_bias; map.start_address = load_bias; } diff --git a/src/linux/dso_debug.rs b/src/linux/dso_debug.rs index 47e530db..6e51eee8 100644 --- a/src/linux/dso_debug.rs +++ b/src/linux/dso_debug.rs @@ -1,7 +1,7 @@ use { super::{ - auxv::AuxvDumpInfo, minidump_writer::MinidumpWriter, process_reader::CopyFromProcessError, - serializers::*, + auxv::AuxvDumpInfo, minidump_writer::MinidumpWriter, process_inspection::ProcessInspector, + process_reader::CopyFromProcessError, serializers::*, }, crate::{ mem_writer::{ @@ -97,6 +97,7 @@ pub struct RDebug { } pub fn write_dso_debug_stream( + process_inspector: &ProcessInspector, buffer: &mut Buffer, blamed_thread: i32, auxv: &AuxvDumpInfo, @@ -108,7 +109,12 @@ pub fn write_dso_debug_stream( .get_program_header_address() .ok_or(SectionDsoDebugError::CouldNotFind("AT_PHDR in auxv"))? as usize; - let ph = MinidumpWriter::copy_from_process(blamed_thread, phdr, SIZEOF_PHDR * phnum_max)?; + let ph = MinidumpWriter::copy_from_process( + process_inspector, + blamed_thread, + phdr, + SIZEOF_PHDR * phnum_max, + )?; let program_headers; #[cfg(target_pointer_width = "64")] { @@ -155,6 +161,7 @@ pub fn write_dso_debug_stream( // dump it to a MD_LINUX_DSO_DEBUG stream. loop { let dyn_data = MinidumpWriter::copy_from_process( + process_inspector, blamed_thread, dyn_addr as usize + dynamic_length, dyn_size, @@ -182,8 +189,12 @@ pub fn write_dso_debug_stream( // See for a more detailed discussion of the how the dynamic // loader communicates with debuggers. - let debug_entry_data = - MinidumpWriter::copy_from_process(blamed_thread, r_debug, std::mem::size_of::())?; + let debug_entry_data = MinidumpWriter::copy_from_process( + process_inspector, + blamed_thread, + r_debug, + std::mem::size_of::(), + )?; // goblin::elf::Dyn doesn't have padding bytes let (head, body, _tail) = unsafe { debug_entry_data.align_to::() }; @@ -195,6 +206,7 @@ pub fn write_dso_debug_stream( let mut curr_map = debug_entry.r_map; while curr_map != 0 { let link_map_data = MinidumpWriter::copy_from_process( + process_inspector, blamed_thread, curr_map, std::mem::size_of::(), @@ -220,8 +232,12 @@ pub fn write_dso_debug_stream( for (idx, map) in dso_vec.iter().enumerate() { let mut filename = String::new(); if map.l_name > 0 { - let filename_data = - MinidumpWriter::copy_from_process(blamed_thread, map.l_name, 256)?; + let filename_data = MinidumpWriter::copy_from_process( + process_inspector, + blamed_thread, + map.l_name, + 256, + )?; // C - string is NULL-terminated if let Some(name) = filename_data.splitn(2, |x| *x == b'\0').next() { @@ -256,8 +272,12 @@ pub fn write_dso_debug_stream( }; dirent.location.data_size += dynamic_length as u32; - let dso_debug_data = - MinidumpWriter::copy_from_process(blamed_thread, dyn_addr as usize, dynamic_length)?; + let dso_debug_data = MinidumpWriter::copy_from_process( + process_inspector, + blamed_thread, + dyn_addr as usize, + dynamic_length, + )?; MemoryArrayWriter::write_bytes(buffer, &dso_debug_data); Ok(dirent) diff --git a/src/linux/minidump_writer/app_memory.rs b/src/linux/minidump_writer/app_memory.rs index 2d65b646..2d9eaa0a 100644 --- a/src/linux/minidump_writer/app_memory.rs +++ b/src/linux/minidump_writer/app_memory.rs @@ -13,8 +13,12 @@ impl MinidumpWriter { pub fn write_app_memory(&mut self, buffer: &mut DumpBuf) -> Result<(), SectionAppMemoryError> { let blamed_thread = self.blamed_thread; for app_memory in &self.app_memory { - let data_copy = - Self::copy_from_process(blamed_thread, app_memory.ptr, app_memory.length)?; + let data_copy = Self::copy_from_process( + &self.process_inspector, + blamed_thread, + app_memory.ptr, + app_memory.length, + )?; let section = MemoryArrayWriter::write_bytes(buffer, &data_copy); let desc = MDMemoryDescriptor { diff --git a/src/linux/minidump_writer/mod.rs b/src/linux/minidump_writer/mod.rs index 75a6d6f6..edfba4bf 100644 --- a/src/linux/minidump_writer/mod.rs +++ b/src/linux/minidump_writer/mod.rs @@ -7,8 +7,7 @@ use { dso_debug, dumper_cpu_info::CpuInfoError, maps_reader::{MappingInfo, MappingList, MapsReaderError}, - process_inspection::ProcessInspector, - process_reader::{CopyFromProcessError, ProcessReader}, + process_inspection::{ProcessInspector, process_reader::CopyFromProcessError}, serializers::*, thread_info::{ThreadInfo, ThreadInfoError}, }, @@ -104,7 +103,7 @@ pub struct MinidumpWriter { pub crash_context: Option, pub app_memory: AppMemoryList, pub memory_blocks: Vec, - process_inspector: ProcessInspector, + pub process_inspector: ProcessInspector, } #[derive(Debug, Clone)] @@ -138,7 +137,7 @@ impl MinidumpWriterConfig { crashing_thread_context: Default::default(), stop_timeout: STOP_TIMEOUT, direct_auxv_dump_info: Default::default(), - process_inspector: ProcessInspector::local(), + process_inspector: ProcessInspector::local(process_id), } } @@ -298,7 +297,7 @@ impl MinidumpWriter { #[cfg(target_os = "android")] { - late_process_mappings(self.process_id, &mut self.mappings)?; + late_process_mappings(&self.process_inspector, self.process_id, &mut self.mappings)?; } if self.skip_stacks_if_mapping_unreferenced { @@ -427,7 +426,12 @@ impl MinidumpWriter { file_entry!("auxv", LinuxAuxv, WriteEnvironmentFailed); file_entry!("maps", LinuxMaps, WriteMapsFailed); - let dirent = match dso_debug::write_dso_debug_stream(buffer, self.process_id, &self.auxv) { + let dirent = match dso_debug::write_dso_debug_stream( + &self.process_inspector, + buffer, + self.process_id, + &self.auxv, + ) { Ok(dirent) => dirent, Err(e) => { soft_errors.push(WriterError::WriteDSODebugStreamFailed(e)); @@ -500,6 +504,7 @@ impl MinidumpWriter { }; let stack_copy = match MinidumpWriter::copy_from_process( + &self.process_inspector, self.blamed_thread, valid_stack_pointer, stack_len, @@ -946,16 +951,16 @@ impl MinidumpWriter { ) -> Result { assert!(idx < self.mappings.len()); - Self::from_process_memory_for_mapping(&self.mappings[idx], self.process_id) + Self::from_process_memory_for_mapping(&self.process_inspector, &self.mappings[idx]) } pub fn from_process_memory_for_mapping( + process_inspector: &ProcessInspector, mapping: &MappingInfo, - pid: Pid, ) -> Result { - let reader = ProcessReader::new(pid); + let reader = process_inspector.process_reader(); Ok(T::read_from_module( - module_reader::ModuleMemory::from_process(&reader, mapping.start_address), + module_reader::ModuleMemory::from_process(reader, mapping.start_address), )?) } @@ -963,6 +968,7 @@ impl MinidumpWriter { /// allocated copy #[inline] pub fn copy_from_process( + process_inspector: &ProcessInspector, pid: Pid, src: usize, length: usize, @@ -978,7 +984,7 @@ impl MinidumpWriter { source: nix::errno::Errno::EINVAL, })?; - let mem = ProcessReader::new(pid); + let mem = process_inspector.process_reader(); mem.read_to_vec(src, length) } } diff --git a/src/linux/minidump_writer/thread_list_stream.rs b/src/linux/minidump_writer/thread_list_stream.rs index 0e1b5253..3b44df12 100644 --- a/src/linux/minidump_writer/thread_list_stream.rs +++ b/src/linux/minidump_writer/thread_list_stream.rs @@ -137,6 +137,7 @@ impl MinidumpWriter { (end_of_range - ip_memory_d.start_of_memory_range) as u32; let memory_copy = MinidumpWriter::copy_from_process( + &self.process_inspector, thread.thread_id as i32, ip_memory_d.start_of_memory_range as _, ip_memory_d.memory.data_size as usize, @@ -211,6 +212,7 @@ impl MinidumpWriter { }; let mut stack_bytes = MinidumpWriter::copy_from_process( + &self.process_inspector, thread.thread_id.try_into()?, valid_stack_ptr, stack_len, diff --git a/src/linux/mod.rs b/src/linux/mod.rs index 33c3672f..301dc754 100644 --- a/src/linux/mod.rs +++ b/src/linux/mod.rs @@ -1,14 +1,13 @@ // `WriterError` is large and clippy doesn't like that, but not a huge deal atm #![allow(clippy::result_large_err)] -pub use maps_reader::LINUX_GATE_LIBRARY_NAME; +pub use {maps_reader::LINUX_GATE_LIBRARY_NAME, process_inspection::process_reader}; pub mod app_memory; pub mod crash_context; pub mod maps_reader; pub mod minidump_writer; pub mod module_reader; -pub mod process_reader; pub mod thread_info; pub(crate) mod auxv; diff --git a/src/linux/process_inspection/mod.rs b/src/linux/process_inspection/mod.rs index f7d758e1..11cbc311 100644 --- a/src/linux/process_inspection/mod.rs +++ b/src/linux/process_inspection/mod.rs @@ -1,6 +1,8 @@ use { + super::{Pid, maps_reader, serializers}, core::{ffi::c_void, mem}, nix::errno::Errno, + process_reader::ProcessReader, regs::*, std::{ fs::File, @@ -9,6 +11,7 @@ use { }, }; +pub mod process_reader; pub mod regs; #[cfg(target_env = "gnu")] @@ -19,12 +22,20 @@ type PtraceRequestType = core::ffi::c_int; #[derive(Debug)] pub struct ProcessInspector { - _private: (), // Placeholder to force API usage for creation + pid: libc::pid_t, + process_reader: ProcessReader, } impl ProcessInspector { - pub fn local() -> Self { - ProcessInspector { _private: () } + pub fn local(pid: libc::pid_t) -> Self { + ProcessInspector { + pid, + process_reader: ProcessReader::new(pid), + } + } + + pub fn process_reader(&self) -> &ProcessReader { + &self.process_reader } pub fn read_file(&self, path: impl AsRef) -> io::Result { @@ -139,3 +150,20 @@ fn ptrace_getregset(regset_type: usize, pid: libc::pid_t) -> nix::Result { Ok(unsafe { output.assume_init() }) } + +#[doc(hidden)] +impl ProcessInspector { + pub fn force_pr_reset(&mut self) { + self.process_reader = ProcessReader::new(self.pid) + } + pub fn force_pr_virtual_mem(&mut self) { + self.process_reader = ProcessReader::for_virtual_mem(self.pid) + } + pub fn force_pr_file(&mut self) -> std::io::Result<()> { + self.process_reader = ProcessReader::for_file(self.pid)?; + Ok(()) + } + pub fn force_pr_ptrace(&mut self) { + self.process_reader = ProcessReader::for_ptrace(self.pid); + } +} diff --git a/src/linux/process_reader.rs b/src/linux/process_inspection/process_reader.rs similarity index 96% rename from src/linux/process_reader.rs rename to src/linux/process_inspection/process_reader.rs index 9bfc63ae..82cfb0d0 100644 --- a/src/linux/process_reader.rs +++ b/src/linux/process_inspection/process_reader.rs @@ -5,9 +5,7 @@ use { }; #[cfg(target_os = "android")] -use super::module_reader::SoName; - -pub type ProcessHandle = libc::pid_t; +use super::super::module_reader::SoName; #[derive(Debug)] enum Style { @@ -84,7 +82,7 @@ impl ProcessReader { /// Creates a [`Self`] for the specified process id, the method used will /// be probed for on the first access #[inline] - pub fn new(pid: ProcessHandle) -> Self { + pub(super) fn new(pid: libc::pid_t) -> Self { Self { pid: nix::unistd::Pid::from_raw(pid), style: OnceLock::default(), @@ -93,7 +91,7 @@ impl ProcessReader { #[inline] #[doc(hidden)] - pub fn for_virtual_mem(pid: i32) -> Self { + pub(super) fn for_virtual_mem(pid: libc::pid_t) -> Self { Self { pid: nix::unistd::Pid::from_raw(pid), style: OnceLock::from(Style::VirtualMem), @@ -102,7 +100,7 @@ impl ProcessReader { #[inline] #[doc(hidden)] - pub fn for_file(pid: i32) -> std::io::Result { + pub(super) fn for_file(pid: libc::pid_t) -> std::io::Result { let file = std::fs::File::open(format!("/proc/{pid}/mem"))?; Ok(Self { @@ -113,7 +111,7 @@ impl ProcessReader { #[inline] #[doc(hidden)] - pub fn for_ptrace(pid: i32) -> Self { + pub(super) fn for_ptrace(pid: libc::pid_t) -> Self { Self { pid: nix::unistd::Pid::from_raw(pid), style: OnceLock::from(Style::Ptrace),