diff --git a/Cargo.toml b/Cargo.toml index 141d852e..c7682a6a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" license = "MIT" [dependencies] +bitflags = "2.0" byteorder = "1.3.2" cfg-if = "1.0" crash-context = "0.6" @@ -31,9 +32,6 @@ nix = { version = "0.26", default-features = false, features = [ "user", ] } -[target.'cfg(target_os = "windows")'.dependencies] -bitflags = "2.0" - [target.'cfg(target_os = "macos")'.dependencies] # Binds some additional mac specifics not in libc mach2 = "0.4" diff --git a/src/linux/android.rs b/src/linux/android.rs index 0c8195cc..a1bba078 100644 --- a/src/linux/android.rs +++ b/src/linux/android.rs @@ -118,7 +118,7 @@ pub fn late_process_mappings(pid: Pid, mappings: &mut [MappingInfo]) -> Result<( // where the ELF header indicates a mapped shared library. for mut map in mappings .iter_mut() - .filter(|m| m.executable && m.name.as_ref().map_or(false, |n| n.starts_with("/"))) + .filter(|m| m.is_executable() && m.name.as_ref().map_or(false, |n| n.starts_with("/"))) { let ehdr_opt = PtraceDumper::copy_from_process( pid, diff --git a/src/linux/maps_reader.rs b/src/linux/maps_reader.rs index 8050387a..a8395e3c 100644 --- a/src/linux/maps_reader.rs +++ b/src/linux/maps_reader.rs @@ -18,6 +18,16 @@ pub struct SystemMappingInfo { pub end_address: usize, } +bitflags::bitflags! { + #[derive(Copy, Clone, Debug, Default, Eq, PartialEq)] + #[repr(transparent)] + pub struct MappingPermission: u8 { + const READABLE = 0b0001; + const WRITABLE = 0b0010; + const EXECUTABLE = 0b0100; + } +} + // One of these is produced for each mapping in the process (i.e. line in // /proc/$x/maps). #[derive(Debug, PartialEq, Eq, Clone)] @@ -34,8 +44,8 @@ pub struct MappingInfo { // address range. The following structure holds the original mapping // address range as reported by the operating system. pub system_mapping_info: SystemMappingInfo, - pub offset: usize, // offset into the backed file. - pub executable: bool, // true if the mapping has the execute bit set. + pub offset: usize, // offset into the backed file. + pub permissions: MappingPermission, // read, write and execute permissions. pub name: Option, // pub elf_obj: Option, } @@ -120,7 +130,11 @@ impl MappingInfo { let start_address = usize::from_str_radix(addresses.next().unwrap(), 16)?; let end_address = usize::from_str_radix(addresses.next().unwrap(), 16)?; - let executable = perms.contains('x'); + let mut permissions = MappingPermission::default(); + permissions.set(MappingPermission::EXECUTABLE, perms.contains('x')); + permissions.set(MappingPermission::READABLE, perms.contains('r')); + permissions.set(MappingPermission::WRITABLE, perms.contains('w')); + let permissions = permissions; // Only copy name if the name is a valid path name, or if // it's the VDSO image. @@ -140,7 +154,7 @@ impl MappingInfo { { module.system_mapping_info.end_address = end_address; module.size = end_address - module.start_address; - module.executable |= executable; + module.permissions |= permissions; return Ok(MappingInfoParsingResult::SkipLine); } } @@ -151,7 +165,7 @@ impl MappingInfo { // and which directly follow an executable mapping. let module_end_address = module.start_address + module.size; if (start_address == module_end_address) - && module.executable + && module.is_executable() && is_mapping_a_path(module.name.as_deref()) && (offset == 0 || offset == module_end_address) && perms == RESERVED_FLAGS @@ -173,7 +187,7 @@ impl MappingInfo { end_address, }, offset, - executable, + permissions, name, // elf_obj, }; @@ -315,7 +329,7 @@ impl MappingInfo { return Ok((file_path, file_name)); }; - if self.executable && self.offset != 0 { + if self.is_executable() && self.offset != 0 { // If an executable is mapped from a non-zero offset, this is likely because // the executable was loaded directly from inside an archive file (e.g., an // apk on Android). @@ -356,7 +370,7 @@ impl MappingInfo { self.name.is_some() && // Only want to include one mapping per shared lib. // Avoid filtering executable mappings. - (self.offset == 0 || self.executable) && + (self.offset == 0 || self.is_executable()) && // big enough to get a signature for. self.size >= 4096 } @@ -365,6 +379,18 @@ impl MappingInfo { self.system_mapping_info.start_address <= address && address < self.system_mapping_info.end_address } + + pub fn is_executable(&self) -> bool { + self.permissions.contains(MappingPermission::EXECUTABLE) + } + + pub fn is_readable(&self) -> bool { + self.permissions.contains(MappingPermission::READABLE) + } + + pub fn is_writable(&self) -> bool { + self.permissions.contains(MappingPermission::WRITABLE) + } } #[cfg(test)] @@ -453,7 +479,9 @@ mod tests { end_address: 0x559748406000, }, offset: 0, - executable: true, + permissions: MappingPermission::READABLE + | MappingPermission::WRITABLE + | MappingPermission::EXECUTABLE, name: Some("/usr/bin/cat".to_string()), }; @@ -467,7 +495,7 @@ mod tests { end_address: 0x559749b2f000, }, offset: 0, - executable: false, + permissions: MappingPermission::READABLE | MappingPermission::WRITABLE, name: Some("[heap]".to_string()), }; @@ -481,7 +509,7 @@ mod tests { end_address: 0x7efd968f5000, }, offset: 0, - executable: false, + permissions: MappingPermission::READABLE | MappingPermission::WRITABLE, name: None, }; @@ -500,7 +528,8 @@ mod tests { end_address: 0x7ffc6e0f9000, }, offset: 0, - executable: true, + permissions: MappingPermission::READABLE | MappingPermission::EXECUTABLE, + name: Some("linux-gate.so".to_string()), }; @@ -560,7 +589,9 @@ mod tests { end_address: 0x7efd96d8c000, // ..but this is not visible here }, offset: 0, - executable: true, + permissions: MappingPermission::READABLE + | MappingPermission::WRITABLE + | MappingPermission::EXECUTABLE, name: Some("/lib64/libc-2.32.so".to_string()), }; @@ -621,7 +652,7 @@ mod tests { end_address: 0x7f0b97b73000, }, offset: 0, - executable: true, + permissions: MappingPermission::READABLE | MappingPermission::EXECUTABLE, name: Some("libmozgtk.so".to_string()), }; diff --git a/src/linux/ptrace_dumper.rs b/src/linux/ptrace_dumper.rs index 2380afb0..bf581bb7 100644 --- a/src/linux/ptrace_dumper.rs +++ b/src/linux/ptrace_dumper.rs @@ -332,15 +332,31 @@ impl PtraceDumper { // NOTE: original code uses getpagesize(), which a) isn't there in Rust and // b) shouldn't be used, as its not portable (see man getpagesize) let page_size = nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE)? - .expect("page size apparently unlimited: doesn't make sense."); - let stack_pointer = int_stack_pointer & !(page_size as usize - 1); + .expect("page size apparently unlimited: doesn't make sense.") + as usize; + let stack_pointer = int_stack_pointer & !(page_size - 1); // The number of bytes of stack which we try to capture. let stack_to_capture = 32 * 1024; - let mapping = self + let mut mapping = self .find_mapping(stack_pointer) .ok_or(DumperError::NoStackPointerMapping)?; + + // If the mapping we found has no permissions then it's likely the + // guard page, try using the mapping right before it. The stack grows + // towards lower addresses on the platforms we care about so the stack + // should appear after the guard page. + if mapping.permissions.is_empty() { + // This has been true since Linux 4.12 + let max_guard_page_size = 1024 * 1024; + + let next_mapping_address = mapping.start_address + mapping.size; + if (next_mapping_address - stack_pointer) < max_guard_page_size { + mapping = self.find_mapping(next_mapping_address).unwrap_or(mapping); + } + } + let offset = stack_pointer - mapping.start_address; let distance_to_end = mapping.size - offset; let stack_len = std::cmp::min(distance_to_end, stack_to_capture); @@ -394,7 +410,7 @@ impl PtraceDumper { // bit, modulo the bitfield size, is not set then there does not // exist a mapping in mappings that would contain that pointer. for mapping in &self.mappings { - if !mapping.executable { + if !mapping.is_executable() { continue; } // For each mapping, work out the (unmodulo'ed) range of bits to @@ -441,7 +457,7 @@ impl PtraceDumper { let test = addr >> shift; if could_hit_mapping[(test >> 3) & array_mask] & (1 << (test & 7)) != 0 { if let Some(hit_mapping) = self.find_mapping_no_bias(addr) { - if hit_mapping.executable { + if hit_mapping.is_executable() { last_hit_mapping = Some(hit_mapping); continue; } diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index cd2ca737..cf24e0e2 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -7,7 +7,7 @@ use minidump_writer::{ app_memory::AppMemory, crash_context::CrashContext, errors::*, - maps_reader::{MappingEntry, MappingInfo, SystemMappingInfo}, + maps_reader::{MappingEntry, MappingInfo, MappingPermission, SystemMappingInfo}, minidump_writer::MinidumpWriter, ptrace_dumper::PtraceDumper, thread_info::Pid, @@ -132,7 +132,7 @@ fn test_write_and_read_dump_from_parent_helper(context: Context) { start_address: mmap_addr, size: memory_size, offset: 0, - executable: false, + permissions: MappingPermission::READABLE | MappingPermission::WRITABLE, name: Some("a fake mapping".to_string()), system_mapping_info: SystemMappingInfo { start_address: mmap_addr, diff --git a/tests/ptrace_dumper.rs b/tests/ptrace_dumper.rs index 02ba836a..463fdc25 100644 --- a/tests/ptrace_dumper.rs +++ b/tests/ptrace_dumper.rs @@ -259,7 +259,7 @@ fn test_sanitize_stack_copy() { let mapping_info = dumper .find_mapping_no_bias(instr_ptr) .expect("Failed to find mapping info"); - assert!(mapping_info.executable); + assert!(mapping_info.is_executable()); // Pointers to code shouldn't be sanitized. simulated_stack = vec![0u8; 2 * size_of::()];