diff --git a/Cargo.lock b/Cargo.lock index d27a5b3e..264e0a5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1410,9 +1410,9 @@ dependencies = [ [[package]] name = "minidump-common" -version = "0.26.0" +version = "0.26.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd1e7ee92185b2f4fa67c3e5c1743057d979e145f58b4391d5481b79f0d8067c" +checksum = "2e16d10087ae9e375bad7a40e8ef5504bc08e808ccc6019067ff9de42a84570f" dependencies = [ "bitflags 2.9.4", "debugid", diff --git a/Cargo.toml b/Cargo.toml index b37595af..86a2ab42 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ memmap2 = "0.9" [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] nix = { version = "0.29", default-features = false, features = [ + "fs", "mman", "process", "ptrace", @@ -67,3 +68,7 @@ dump_syms = { version = "2.2", default-features = false } minidump-unwind = { version = "0.26", features = ["debuginfo"] } similar-asserts = "1.6" uuid = "1.12" + +[features] +default = ["testing"] +testing = [] \ No newline at end of file diff --git a/src/bin/test.rs b/src/bin/test.rs index 19986a7e..cd5e4df4 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -11,7 +11,9 @@ mod linux { error_graph::ErrorList, minidump_writer::{ minidump_writer::{MinidumpWriter, MinidumpWriterConfig}, - module_reader, LINUX_GATE_LIBRARY_NAME, + module_reader, + process_inspection::{DirectInspector, ProcessInspector}, + LINUX_GATE_LIBRARY_NAME, }, nix::{ sys::mman::{mmap_anonymous, MapFlags, ProtFlags}, @@ -76,8 +78,6 @@ mod linux { } fn test_copy_from_process(stack_var: usize, heap_var: usize) -> Result<()> { - use minidump_writer::mem_reader::MemReader; - let ppid = getppid().as_raw(); let dumper = fail_on_soft_error!( soft_errors, @@ -90,13 +90,13 @@ mod linux { let expected_stack = 0x11223344usize.to_ne_bytes(); let expected_heap = 0x55667788usize.to_ne_bytes(); - let validate = |reader: &mut MemReader| -> Result<()> { + let validate = |reader: &mut dyn ProcessInspector| -> Result<()> { let mut val = [0u8; std::mem::size_of::()]; - let read = reader.read(stack_var, &mut val)?; + let read = reader.read_memory(stack_var, &mut val)?; assert_eq!(read, val.len()); test!(val == expected_stack, "stack var not correct"); - let read = reader.read(heap_var, &mut val)?; + let read = reader.read_memory(heap_var, &mut val)?; assert_eq!(read, val.len()); test!(val == expected_heap, "heap var not correct"); @@ -105,14 +105,14 @@ mod linux { // virtual mem { - let mut mr = MemReader::for_virtual_mem(ppid); + let mut mr = DirectInspector::for_virtual_mem(ppid); validate(&mut mr) .map_err(|err| format!("failed to validate memory for {mr:?}: {err}"))?; } // file { - let mut mr = MemReader::for_file(ppid) + let mut mr = DirectInspector::for_file(ppid) .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}"))?; @@ -120,18 +120,20 @@ mod linux { // ptrace { - let mut mr = MemReader::for_ptrace(ppid); + let mut mr = DirectInspector::for_ptrace(ppid); validate(&mut mr) .map_err(|err| format!("failed to validate memory for {mr:?}: {err}"))?; } + let process_inspector = DirectInspector::new(ppid); + let stack_res = - MinidumpWriter::copy_from_process(ppid, stack_var, std::mem::size_of::())?; + process_inspector.read_memory_to_vec(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::())?; + process_inspector.read_memory_to_vec(heap_var, std::mem::size_of::())?; test!(heap_res == expected_heap, "heap var not correct"); @@ -215,7 +217,7 @@ mod linux { fn test_linux_gate_mapping_id() -> Result<()> { 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)? ); @@ -224,8 +226,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.as_mut(), + &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 7eee7cb5..8e976678 100644 --- a/src/linux/android.rs +++ b/src/linux/android.rs @@ -1,7 +1,7 @@ use { super::{ - maps_reader::MappingInfo, mem_reader::CopyFromProcessError, - minidump_writer::MinidumpWriter, Pid, + maps_reader::MappingInfo, + process_inspection::{self, ProcessInspector}, }, goblin::elf, }; @@ -31,7 +31,7 @@ type Result = std::result::Result; #[derive(Debug, thiserror::Error, serde::Serialize)] pub enum AndroidError { #[error("Failed to copy memory from process")] - CopyFromProcessError(#[from] CopyFromProcessError), + CopyFromProcessError(#[from] process_inspection::Error), #[error("Failed slice conversion")] TryFromSliceError( #[from] @@ -48,11 +48,15 @@ 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: &mut dyn ProcessInspector, + 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 = process_inspector.read_memory_to_vec(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 +70,18 @@ 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: &mut dyn ProcessInspector, + ehdr: &elf_header::Header, + address: usize, +) -> usize { + let ph = parse_loaded_elf_program_headers(process_inspector, 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, load_bias, ph).is_ok() { return load_bias; } } @@ -83,7 +91,7 @@ fn get_effective_load_bias(pid: Pid, ehdr: &elf_header::Header, address: usize) } fn parse_loaded_elf_program_headers( - pid: Pid, + process_inspector: &mut dyn ProcessInspector, ehdr: &elf_header::Header, address: usize, ) -> DynVaddresses { @@ -92,11 +100,9 @@ fn parse_loaded_elf_program_headers( let mut dyn_vaddr = 0; let mut dyn_count = 0; - let phdr_opt = MinidumpWriter::copy_from_process( - pid, - phdr_addr, - elf_header::SIZEOF_EHDR * ehdr.e_phnum as usize, - ); + let phdr_opt = process_inspector + .read_memory_to_vec(phdr_addr, elf_header::SIZEOF_EHDR * ehdr.e_phnum as usize); + if let Ok(ph_data) = phdr_opt { // TODO: The original C code doesn't have error-handling here at all. // We silently ignore "not parsable" for now, but might bubble it up. @@ -122,17 +128,20 @@ fn parse_loaded_elf_program_headers( } } -pub fn late_process_mappings(pid: Pid, mappings: &mut [MappingInfo]) -> Result<()> { +pub fn late_process_mappings( + process_inspector: &mut dyn ProcessInspector, + 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 = process_inspector + .read_memory_to_vec(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 +151,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, &ehdr, map.start_address); map.size += map.start_address - load_bias; map.start_address = load_bias; } diff --git a/src/linux/auxv/mod.rs b/src/linux/auxv/mod.rs index 01654798..46f3798b 100644 --- a/src/linux/auxv/mod.rs +++ b/src/linux/auxv/mod.rs @@ -1,10 +1,12 @@ use { self::reader::ProcfsAuxvIter, super::Pid, - crate::serializers::*, + crate::{ + linux::process_inspection::{self, ProcessInspector}, + serializers::*, + }, error_graph::WriteErrorList, failspot::failspot, - std::{fs::File, io::BufReader}, thiserror::Error, }; @@ -84,17 +86,18 @@ pub struct AuxvDumpInfo { impl AuxvDumpInfo { pub fn try_filling_missing_info( &mut self, - pid: Pid, + process_inspector: &mut dyn ProcessInspector, mut soft_errors: impl WriteErrorList, ) -> Result<(), AuxvError> { if self.is_complete() { return Ok(()); } - let auxv_path = format!("/proc/{pid}/auxv"); - let auxv_file = File::open(&auxv_path).map_err(|e| AuxvError::OpenError(auxv_path, e))?; + let auxv_reader = process_inspector + .read_proc_path("auxv".as_ref()) + .map_err(AuxvError::ReadFile)?; - for pair_result in ProcfsAuxvIter::new(BufReader::new(auxv_file)) { + for pair_result in ProcfsAuxvIter::new(auxv_reader) { let AuxvPair { key, value } = match pair_result { Ok(pair) => pair, Err(e) => { @@ -147,6 +150,8 @@ pub enum AuxvError { #[serde(serialize_with = "serialize_io_error")] std::io::Error, ), + #[error("Failed to read /proc//auxv file")] + ReadFile(#[source] process_inspection::Error), #[error("No auxv entry found for PID {0}")] NoAuxvEntryFound(Pid), #[error("Invalid auxv format (should not hit EOF before AT_NULL)")] diff --git a/src/linux/auxv/reader.rs b/src/linux/auxv/reader.rs index e36ba93a..450bba55 100644 --- a/src/linux/auxv/reader.rs +++ b/src/linux/auxv/reader.rs @@ -8,22 +8,19 @@ use { super::{AuxvError, AuxvPair, AuxvType}, byteorder::{NativeEndian, ReadBytesExt}, - std::{ - fs::File, - io::{BufReader, Read}, - }, + std::io::Read, }; /// An iterator across auxv pairs from procfs. -pub struct ProcfsAuxvIter { +pub struct ProcfsAuxvIter { pair_size: usize, buf: Vec, - input: BufReader, + input: R, keep_going: bool, } -impl ProcfsAuxvIter { - pub fn new(input: BufReader) -> Self { +impl ProcfsAuxvIter { + pub fn new(input: R) -> Self { let pair_size = 2 * std::mem::size_of::(); let buf: Vec = Vec::with_capacity(pair_size); @@ -36,7 +33,7 @@ impl ProcfsAuxvIter { } } -impl Iterator for ProcfsAuxvIter { +impl Iterator for ProcfsAuxvIter { type Item = Result; fn next(&mut self) -> Option { if !self.keep_going { diff --git a/src/linux/dso_debug.rs b/src/linux/dso_debug.rs index 67d6e6f8..9e579d36 100644 --- a/src/linux/dso_debug.rs +++ b/src/linux/dso_debug.rs @@ -1,6 +1,7 @@ use { super::{ - auxv::AuxvDumpInfo, mem_reader::CopyFromProcessError, minidump_writer::MinidumpWriter, + auxv::AuxvDumpInfo, + process_inspection::{self, ProcessInspector}, serializers::*, }, crate::{ @@ -40,7 +41,7 @@ pub enum SectionDsoDebugError { #[error("Could not find: {0}")] CouldNotFind(&'static str), #[error("Failed to copy memory from process")] - CopyFromProcessError(#[from] CopyFromProcessError), + CopyFromProcessError(#[from] process_inspection::Error), #[error("Failed to copy memory from process")] FromUTF8Error( #[from] @@ -97,8 +98,8 @@ pub struct RDebug { } pub fn write_dso_debug_stream( + process_inspector: &mut dyn ProcessInspector, buffer: &mut Buffer, - blamed_thread: i32, auxv: &AuxvDumpInfo, ) -> Result { let phnum_max = @@ -108,7 +109,8 @@ 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 = process_inspector.read_memory_to_vec(phdr, SIZEOF_PHDR * phnum_max)?; + let program_headers; #[cfg(target_pointer_width = "64")] { @@ -154,11 +156,8 @@ pub fn write_dso_debug_stream( // DSOs loaded into the program. If this information is indeed available, // dump it to a MD_LINUX_DSO_DEBUG stream. loop { - let dyn_data = MinidumpWriter::copy_from_process( - blamed_thread, - dyn_addr as usize + dynamic_length, - dyn_size, - )?; + let dyn_data = + process_inspector.read_memory_to_vec(dyn_addr as usize + dynamic_length, dyn_size)?; dynamic_length += dyn_size; // goblin::elf::Dyn doesn't have padding bytes @@ -183,7 +182,7 @@ pub fn write_dso_debug_stream( // loader communicates with debuggers. let debug_entry_data = - MinidumpWriter::copy_from_process(blamed_thread, r_debug, std::mem::size_of::())?; + process_inspector.read_memory_to_vec(r_debug, std::mem::size_of::())?; // goblin::elf::Dyn doesn't have padding bytes let (head, body, _tail) = unsafe { debug_entry_data.align_to::() }; @@ -194,11 +193,8 @@ pub fn write_dso_debug_stream( let mut dso_vec = Vec::new(); let mut curr_map = debug_entry.r_map; while curr_map != 0 { - let link_map_data = MinidumpWriter::copy_from_process( - blamed_thread, - curr_map, - std::mem::size_of::(), - )?; + let link_map_data = + process_inspector.read_memory_to_vec(curr_map, std::mem::size_of::())?; // LinkMap is repr(C) and doesn't have padding bytes, so this should be safe let (head, body, _tail) = unsafe { link_map_data.align_to::() }; @@ -220,8 +216,7 @@ 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 = process_inspector.read_memory_to_vec(map.l_name, 256)?; // C - string is NULL-terminated if let Some(name) = filename_data.splitn(2, |x| *x == b'\0').next() { @@ -256,8 +251,7 @@ 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 = process_inspector.read_memory_to_vec(dyn_addr as usize, dynamic_length)?; MemoryArrayWriter::write_bytes(buffer, &dso_debug_data); Ok(dirent) diff --git a/src/linux/mem_reader.rs b/src/linux/mem_reader.rs deleted file mode 100644 index 4ca9ff34..00000000 --- a/src/linux/mem_reader.rs +++ /dev/null @@ -1,260 +0,0 @@ -//! Functionality for reading a remote process's memory - -use { - super::{minidump_writer::MinidumpWriter, serializers::*, Pid}, - std::sync::OnceLock, -}; - -#[derive(Debug)] -enum Style { - /// Uses [`process_vm_readv`](https://linux.die.net/man/2/process_vm_readv) - /// to read the memory. - /// - /// This is not available on old <3.2 (really, ancient) kernels, and requires - /// the same permissions as ptrace - VirtualMem, - /// Reads the memory from `/proc//mem` - /// - /// Available on basically all versions of Linux, but could fail if the process - /// has insufficient privileges, ie ptrace - File(std::fs::File), - /// Reads the memory with [ptrace (`PTRACE_PEEKDATA`)](https://man7.org/linux/man-pages/man2/ptrace.2.html) - /// - /// Reads data one word at a time, so slow, but fairly reliable, as long as - /// the process can be ptraced - Ptrace, - /// No methods succeeded, generally there isn't a case where failing a syscall - /// will work if called again - Unavailable { - vmem: nix::Error, - file: nix::Error, - ptrace: nix::Error, - }, -} - -#[derive(Debug, thiserror::Error, serde::Serialize)] -#[error("Copy from process {child} failed (source {src}, offset: {offset}, length: {length})")] -pub struct CopyFromProcessError { - pub child: Pid, - pub src: usize, - pub offset: usize, - pub length: usize, - #[serde(serialize_with = "serialize_nix_error")] - pub source: nix::Error, -} - -pub struct MemReader { - /// The pid of the child to read - pid: nix::unistd::Pid, - style: OnceLock