diff --git a/src/lib.rs b/src/lib.rs index 9337b4cb..a1866205 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,7 @@ cfg_if::cfg_if! { StackPointerMapping, ThreadStackCopy, CrashingThreadIpCopy, + EnumerateMappingsFromProc, } } diff --git a/src/linux/minidump_writer/errors.rs b/src/linux/minidump_writer/errors.rs index 8c2784ec..4618d4b8 100644 --- a/src/linux/minidump_writer/errors.rs +++ b/src/linux/minidump_writer/errors.rs @@ -12,7 +12,7 @@ use { thread_list_stream::SectionThreadListError, thread_names_stream::SectionThreadNamesError, }, - module_list::ModuleResolveError, + module_list::{FromRendezvousError, ModuleResolveError}, module_reader::ModuleReaderError, process_inspection, serializers::*, @@ -185,6 +185,16 @@ pub enum InitError { SuspendNoThreadsLeft(usize), #[error("Crash thread does not reference principal mapping")] PrincipalMappingNotReferenced, + #[error("no program header table address in auxiliary vector")] + MissingProgramHeaderTableAddress, + #[error("no program header count in auxiliary vector")] + MissingProgramHeaderCount, + #[error("failed to obtain the module list from the debugger rendez-vous")] + ModuleListFromDebuggerRendezvousFailed(#[source] FromRendezvousError), + #[error("Errors occurred while walking the debugger rendez-vous")] + DebuggerRendezvousErrors(#[source] ErrorList), + #[error("no module could be derived from the process mappings")] + NoModulesInProcessMappings, } #[derive(Debug, thiserror::Error, serde::Serialize)] diff --git a/src/linux/minidump_writer/mod.rs b/src/linux/minidump_writer/mod.rs index 3c77aad8..395f9697 100644 --- a/src/linux/minidump_writer/mod.rs +++ b/src/linux/minidump_writer/mod.rs @@ -6,10 +6,10 @@ use { app_memory::AppMemoryList, auxv::AuxvDumpInfo, crash_context_ext::CrashContextExt, - dso_debug, + dso_debug::{elf_addr_to_usize, write_dso_debug_stream}, dumper_cpu_info::CpuInfoError, maps_reader::{MappingInfo, MappingList}, - module_list, + module_list::{self, ModuleCandidate}, process_inspection::{self, ProcessInspector, process_reader::CopyFromProcessError}, remote_process_inspection, serializers::*, @@ -67,6 +67,16 @@ pub const AT_SYSINFO_EHDR: u32 = 33; #[cfg(target_pointer_width = "64")] pub const AT_SYSINFO_EHDR: u64 = 33; +/// Where the module list written to the `ModuleListStream` is sourced from. +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +pub enum ModuleListSource { + #[default] + /// Derive the module list from `/proc//maps`. + ProcMaps, + /// Use the DT_DEBUG debugger rendezvous protocol to inspect ELF objects + DebuggerRendezvous, +} + #[derive(Debug)] pub struct MinidumpWriterConfig { process_id: Pid, @@ -84,6 +94,7 @@ pub struct MinidumpWriterConfig { stop_timeout: Duration, direct_auxv_dump_info: Option, process_inspector: Box, + module_list_source: ModuleListSource, } #[derive(Debug)] @@ -94,6 +105,7 @@ pub struct MinidumpWriter { pub auxv: AuxvDumpInfo, pub mappings: Vec, pub modules: Vec, + module_list_source: ModuleListSource, pub page_size: usize, pub sanitize_stack: bool, pub minidump_size_limit: Option, @@ -142,6 +154,7 @@ impl MinidumpWriterConfig { stop_timeout: STOP_TIMEOUT, direct_auxv_dump_info: Default::default(), process_inspector: process_inspection::local(process_id), + module_list_source: Default::default(), } } @@ -216,6 +229,12 @@ impl MinidumpWriterConfig { self } + /// Chooses where the `ModuleListStream` is sourced from. + pub fn set_module_list_source(&mut self, module_list_source: ModuleListSource) -> &mut Self { + self.module_list_source = module_list_source; + self + } + /// Generates a minidump and writes to the destination provided. Returns the in-memory /// version of the minidump as well. pub fn write(self, destination: &mut (impl Write + Seek)) -> Result, WriterError> { @@ -250,6 +269,7 @@ impl MinidumpWriterConfig { auxv, mappings: Default::default(), modules: Default::default(), + module_list_source: self.module_list_source, page_size: Default::default(), sanitize_stack: self.sanitize_stack, minidump_size_limit: self.minidump_size_limit, @@ -456,17 +476,14 @@ impl MinidumpWriter { file_entry!("auxv", LinuxAuxv, WriteEnvironmentFailed); file_entry!("maps", LinuxMaps, WriteMapsFailed); - let dirent = match dso_debug::write_dso_debug_stream( - self.process_inspector.as_ref(), - buffer, - &self.auxv, - ) { - Ok(dirent) => dirent, - Err(e) => { - soft_errors.push(WriterError::WriteDSODebugStreamFailed(e)); - Default::default() - } - }; + let dirent = + match write_dso_debug_stream(self.process_inspector.as_ref(), buffer, &self.auxv) { + Ok(dirent) => dirent, + Err(e) => { + soft_errors.push(WriterError::WriteDSODebugStreamFailed(e)); + Default::default() + } + }; dir_section.write_to_file(buffer, Some(dirent))?; file_entry!("limits", MozLinuxLimits, WriteLimitsFailed); @@ -720,6 +737,12 @@ impl MinidumpWriter { } fn enumerate_mappings(&mut self) -> Result<(), InitError> { + if failspot!(EnumerateMappingsFromProc) { + return Err(InitError::AggregateMappingsFailed( + std::io::Error::other("fake I/O error reading maps file").into(), + )); + } + // linux_gate_loc is the beginning of the kernel's mapping of // linux-gate.so in the process. It doesn't actually show up in the // maps list as a filename, but it can be found using the AT_SYSINFO_EHDR @@ -745,8 +768,18 @@ impl MinidumpWriter { &mut self, mut soft_errors: impl WriteErrorList, ) -> Result<(), InitError> { - let mut candidates = - module_list::from_mappings(self.process_inspector.as_ref(), &self.mappings); + use ModuleListSource::*; + let (primary, fallback) = match self.module_list_source { + ProcMaps => (ProcMaps, DebuggerRendezvous), + DebuggerRendezvous => (DebuggerRendezvous, ProcMaps), + }; + let mut candidates = self + .try_module_source(primary, &mut soft_errors) + .or_else(|e| { + soft_errors.push(e); + self.try_module_source(fallback, &mut soft_errors) + })?; + candidates.append(&mut module_list::from_user_mappings( &self.user_mapping_list, )); @@ -762,6 +795,61 @@ impl MinidumpWriter { Ok(()) } + fn try_module_source( + &self, + source: ModuleListSource, + mut soft_errors: impl WriteErrorList, + ) -> Result, InitError> { + use ModuleListSource::*; + match source { + ProcMaps => { + let candidates = + module_list::from_mappings(self.process_inspector.as_ref(), &self.mappings); + + // No detected module is probably because the initial mapping parsing failed. + // Error out so that we try the fallback. + if candidates.is_empty() { + return Err(InitError::NoModulesInProcessMappings); + } + + Ok(candidates) + } + DebuggerRendezvous => self.modules_via_debugger_rendezvous( + soft_errors.subwriter(InitError::DebuggerRendezvousErrors), + ), + } + } + + fn modules_via_debugger_rendezvous( + &self, + soft_errors: impl WriteErrorList, + ) -> Result, InitError> { + // `DT_DEBUG` only lives in the main executable's dynamic section, and + // AT_PHDR/AT_PHNUM are how we find that executable's program headers. + let Some(program_header_table_address) = self + .auxv + .get_program_header_address() + .map(elf_addr_to_usize) + else { + return Err(InitError::MissingProgramHeaderTableAddress); + }; + + let Some(program_header_count) = + self.auxv.get_program_header_count().map(elf_addr_to_usize) + else { + return Err(InitError::MissingProgramHeaderCount); + }; + + module_list::from_debugger_rendezvous( + self.process_inspector.as_ref(), + self.process_id, + program_header_table_address, + program_header_count, + soft_errors, + ) + .map_err(InitError::ModuleListFromDebuggerRendezvousFailed) + } + /// Read thread info from /proc/$pid/status. /// Fill out the |tgid|, |ppid| and |pid| members of |info|. If unavailable, /// these members are set to -1. Returns true if all three members are diff --git a/src/linux/module_list.rs b/src/linux/module_list.rs index dfc501de..10511bda 100644 --- a/src/linux/module_list.rs +++ b/src/linux/module_list.rs @@ -584,7 +584,6 @@ mod tests { /// headers, hence the need for `process_inspector`, `program_header_table_address` /// and `program_header_count`, while `pid` is there in order to get a name for the /// main program's module, which isn't necessarily given in the DT_DEBUG data (glibc). -#[expect(unused)] pub(crate) fn from_debugger_rendezvous( process_inspector: &dyn ProcessInspector, pid: Pid, diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 9fb5e30d..e2f0e933 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -234,6 +234,19 @@ where ); } +#[allow(unused)] +/// Negative equivalent to [`assert_minidump_contains_soft_error`]. +pub fn assert_minidump_lacks_soft_error<'a, T>(dump: &minidump::Minidump<'a, T>, variant: &str) +where + T: std::ops::Deref + 'a, +{ + let errors = read_minidump_soft_errors_or_panic(dump); + assert!( + !soft_errors_contain(&errors, variant), + "soft error list contains unexpected error `{variant}`\nError_list: {errors:#?}" + ); +} + fn soft_errors_contain(value: &serde_json::Value, variant: &str) -> bool { match value { serde_json::Value::String(s) => s == variant, diff --git a/tests/linux_module_list.rs b/tests/linux_module_list.rs new file mode 100644 index 00000000..68970a96 --- /dev/null +++ b/tests/linux_module_list.rs @@ -0,0 +1,175 @@ +#![cfg(any(target_os = "linux", target_os = "android"))] + +use { + common::*, + minidump::*, + minidump_writer::minidump_writer::{MinidumpWriterConfig, ModuleListSource}, +}; + +mod common; + +/// The module sources this target can actually exercise. +const AVAILABLE_SOURCES: &[ModuleListSource] = &[ + ModuleListSource::ProcMaps, + #[cfg(not(target_feature = "crt-static"))] + ModuleListSource::DebuggerRendezvous, +]; + +// Spawn a child, extract its modules using the given method, and return +// the module names in order. +fn module_names(source: ModuleListSource) -> Vec { + let modules = module_list(source); + modules + .iter() + .map(|module| module.name.clone()) + .collect::>() +} + +fn source_specific_error_str(source: ModuleListSource) -> &'static str { + match source { + ModuleListSource::ProcMaps => "NoModulesInProcessMappings", + ModuleListSource::DebuggerRendezvous => "ModuleListFromDebuggerRendezvousFailed", + } +} +/// Dumps a freshly spawned child and returns its whole `ModuleListStream`, so a +/// test can look entries up by address rather than by name. +fn module_list(source: ModuleListSource) -> MinidumpModuleList { + let mut child = start_child_and_wait_for_threads(1); + let pid = child.id() as i32; + + let mut tmpfile = tempfile::Builder::new().tempfile().unwrap(); + + let mut writer = MinidumpWriterConfig::new(pid, pid); + writer.set_module_list_source(source); + writer + .write(&mut tmpfile) + .expect("could not write minidump"); + + child.kill().expect("Failed to kill process"); + child.wait().expect("Failed to wait on killed process"); + + let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); + + // We don't want the fallback path! + assert_minidump_lacks_soft_error(&dump, source_specific_error_str(source)); + + dump.get_stream().expect("no module list") +} + +/// We want the test exec first, and the libc *somewhere* — unless it's statically +/// linked, of course. +fn assert_module_list_is_sane(modules: &[String]) { + assert!(!modules.is_empty(), "empty module list"); + assert!( + modules[0].contains("test"), + "first module should be the test helper executable, got {:?}", + modules[0] + ); + #[cfg(not(target_feature = "crt-static"))] + assert!( + modules.iter().any(|name| name.contains("libc")), + "no libc in the module list: {modules:?}" + ); +} + +#[cfg(not(target_feature = "crt-static"))] +#[test] +fn module_list_from_debugger_rendezvous() { + let modules = module_names(ModuleListSource::DebuggerRendezvous); + assert_module_list_is_sane(&modules); +} + +#[test] +fn module_list_from_proc_maps() { + let modules = module_names(ModuleListSource::ProcMaps); + assert_module_list_is_sane(&modules); +} + +/// Both sources should agree on which files are loaded. +#[cfg(not(target_feature = "crt-static"))] +#[test] +fn both_module_list_sources_agree_on_files() { + use std::path::Path; + + // There are *some* variations... + const LINKER_ALIASES: &[&str] = &["ld-android.so", "linker64", "linker"]; + + let file_names = |modules: Vec| { + let mut names = modules + .into_iter() + .filter(|name| name.starts_with('/')) + .map(|name| { + Path::new(&name) + .file_name() + .unwrap() + .to_string_lossy() + .into_owned() + }) + .collect::>(); + names.sort(); + names.dedup(); + names + }; + + let from_rendezvous = file_names(module_names(ModuleListSource::DebuggerRendezvous)); + let from_proc_maps = file_names(module_names(ModuleListSource::ProcMaps)); + + for name in &from_proc_maps { + let found = from_rendezvous.contains(name) + || (LINKER_ALIASES.contains(&name.as_str()) + && from_rendezvous + .iter() + .any(|other| LINKER_ALIASES.contains(&other.as_str()))); + assert!( + found, + "{name} is in the /proc//maps module list but not in the rendez-vous one\n\ + rendez-vous: {from_rendezvous:?}\n\ + proc maps: {from_proc_maps:?}" + ); + } +} + +/// Each ELF object should appear exactly once. Only tested for +/// the rendezvous as it is a known issue of the procmaps method. +#[cfg(not(target_feature = "crt-static"))] +#[test] +fn module_list_has_no_duplicates() { + let modules = module_names(ModuleListSource::DebuggerRendezvous); + let mut sorted = modules.clone(); + sorted.sort(); + sorted.dedup(); + assert_eq!(sorted.len(), modules.len()); +} + +#[test] +fn module_list_includes_the_vdso() { + for &source in AVAILABLE_SOURCES { + let modules = module_names(source); + assert!( + modules + .iter() + // vDSO has different names depending on the libc. + .any(|name| name.contains("vdso") || name.contains("linux-gate")), + "no vDSO in the {source:?} module list: {modules:?}" + ); + } +} + +#[test] +fn no_module_overlaps_another() { + for &source in AVAILABLE_SOURCES { + let modules = module_list(source); + + for module in modules.iter() { + let found = modules.module_at_address(module.raw.base_of_image); + assert_eq!( + found.map(|found| found.raw.base_of_image), + Some(module.raw.base_of_image), + "`{}` at {:#x} is in the {source:?} module list but resolves to {:?}", + module.name, + module.raw.base_of_image, + found.map(|found| &found.name), + ); + } + } +} diff --git a/tests/linux_module_list_failspot.rs b/tests/linux_module_list_failspot.rs new file mode 100644 index 00000000..a96c2ce8 --- /dev/null +++ b/tests/linux_module_list_failspot.rs @@ -0,0 +1,51 @@ +// Pointless to test fallback if one of the two sources is *always* broken, +// as is the rendezvous path on statically linked binaries. +#![cfg(all( + any(target_os = "linux", target_os = "android"), + not(target_feature = "crt-static") +))] + +//! Lives in its own test binary because failspots are process-global: enabling +//! one here would otherwise leak into the tests running concurrently in +//! `linux_module_list.rs`. + +use { + common::*, + minidump::*, + minidump_writer::{ + FailSpotName, + minidump_writer::{MinidumpWriterConfig, ModuleListSource}, + }, +}; + +mod common; + +#[test] +fn module_list_procmaps_fallback_to_rendezvous() { + let mut failspot_client = FailSpotName::testing_client(); + failspot_client.set_enabled(FailSpotName::EnumerateMappingsFromProc, true); + + let mut child = start_child_and_wait_for_threads(1); + let pid = child.id() as i32; + + let mut tmpfile = tempfile::Builder::new().tempfile().unwrap(); + + let mut writer = MinidumpWriterConfig::new(pid, pid); + writer.set_module_list_source(ModuleListSource::ProcMaps); + writer.write(&mut tmpfile).unwrap(); + + child.kill().expect("Failed to kill process"); + child.wait().expect("Failed to wait on killed process"); + + let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); + let modules: MinidumpModuleList = dump.get_stream().expect("no module list"); + let names = modules + .iter() + .map(|module| module.name.clone()) + .collect::>(); + + assert!(!names.is_empty(), "empty module list"); + assert!(names.iter().any(|name| name.contains("libc"))); + assert_minidump_contains_soft_error(&dump, "AggregateMappingsFailed"); + assert_minidump_contains_soft_error(&dump, "NoModulesInProcessMappings"); +}