From 7036aa1fb2479d6ac63a83a8bd1693e65ba856fd Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Wed, 2 Sep 2026 15:06:59 -0400 Subject: [PATCH] Isolated Processes 15.5: Treat non-essential getreg errors as soft --- .../linux/process-backend/src/local/error.rs | 11 ++- crates/linux/process-backend/src/local/mod.rs | 56 ++++++++--- src/linux/minidump_writer/mod.rs | 12 ++- .../minidump_writer/thread_list_stream.rs | 12 ++- src/linux/thread_info/aarch64.rs | 26 +++-- src/linux/thread_info/arm.rs | 26 +++-- src/linux/thread_info/mod.rs | 10 +- src/linux/thread_info/x86.rs | 99 ++++++++++++------- tests/ptrace_dumper.rs | 4 +- 9 files changed, 183 insertions(+), 73 deletions(-) diff --git a/crates/linux/process-backend/src/local/error.rs b/crates/linux/process-backend/src/local/error.rs index 00b46634..2eceb220 100644 --- a/crates/linux/process-backend/src/local/error.rs +++ b/crates/linux/process-backend/src/local/error.rs @@ -34,10 +34,13 @@ pub enum Error { BufferTooSmall, #[error("not supported")] NotSupported, - #[error("failed to get registers: {0}")] - GetRegistersFailed(c_int), - #[error("unexpected size for register set. Expected: {0}, actual: {0}")] - UnexpectedRegisterSetSize(usize, usize), + #[error( + "failed to get registers. getregset_error: `{getregset_error}`, legacy_error: `{legacy_error}`" + )] + GetRegistersFailed { + getregset_error: super::PtraceGetRegSetError, + legacy_error: super::PtraceGetRegsLegacyError, + }, #[error("failed to map memory: {0}")] MMapfailed(c_int), #[error("the start position of a mapping is past its end position")] diff --git a/crates/linux/process-backend/src/local/mod.rs b/crates/linux/process-backend/src/local/mod.rs index 4b727b5c..922e7027 100644 --- a/crates/linux/process-backend/src/local/mod.rs +++ b/crates/linux/process-backend/src/local/mod.rs @@ -199,18 +199,28 @@ impl Backend { } fn get_regs(&self, tid: libc::pid_t) -> Result { - self.ptrace_getregset::(tid).or_else(|e| { - if T::LEGACY_REQUEST.is_some() { - self.ptrace_getregs_legacy::(tid) - } else { - Err(e) - } + let getregset_error = match self.ptrace_getregset::(tid) { + Ok(output) => return Ok(output), + Err(e) => e, + }; + + let legacy_error = match self.ptrace_getregs_legacy::(tid) { + Ok(output) => return Ok(output), + Err(e) => e, + }; + + Err(Error::GetRegistersFailed { + getregset_error, + legacy_error, }) } - fn ptrace_getregs_legacy(&self, tid: libc::pid_t) -> Result { + fn ptrace_getregs_legacy( + &self, + tid: libc::pid_t, + ) -> core::result::Result { let Some(request) = T::LEGACY_REQUEST else { - return Err(Error::NotSupported); + return Err(PtraceGetRegsLegacyError::NotSupported); }; let mut output = T::Output::default(); @@ -222,11 +232,14 @@ impl Backend { (&raw mut output).cast(), ) }) - .map_err(Error::GetRegistersFailed)?; + .map_err(PtraceGetRegsLegacyError::PtraceFailed)?; Ok(output) } - fn ptrace_getregset(&self, tid: libc::pid_t) -> Result { + fn ptrace_getregset( + &self, + tid: libc::pid_t, + ) -> core::result::Result { let output_size = size_of::(); assert!(T::KERNEL_SIZE <= output_size); @@ -244,10 +257,13 @@ impl Backend { (&raw mut io).cast(), ) }) - .map_err(Error::GetRegistersFailed)?; + .map_err(PtraceGetRegSetError::PtraceFailed)?; if T::KERNEL_SIZE != io.iov_len { - return Err(Error::UnexpectedRegisterSetSize(T::KERNEL_SIZE, io.iov_len)); + return Err(PtraceGetRegSetError::UnexpectedRegisterSetSize( + T::KERNEL_SIZE, + io.iov_len, + )); } Ok(output) @@ -489,3 +505,19 @@ unsafe fn ptrace( ) -> c_long { unsafe { libc::ptrace(request, pid, addr, data) } } + +#[derive(Debug, thiserror::Error, serde::Deserialize, serde::Serialize)] +pub enum PtraceGetRegSetError { + #[error("ptrace returned error code: {0}")] + PtraceFailed(c_int), + #[error("unexpected size for register set. Expected: {0}, actual: {0}")] + UnexpectedRegisterSetSize(usize, usize), +} + +#[derive(Debug, thiserror::Error, serde::Deserialize, serde::Serialize)] +pub enum PtraceGetRegsLegacyError { + #[error("ptrace returned error code: {0}")] + PtraceFailed(c_int), + #[error("legacy API not supported on this architecture")] + NotSupported, +} diff --git a/src/linux/minidump_writer/mod.rs b/src/linux/minidump_writer/mod.rs index 1330fb1f..396dfd78 100644 --- a/src/linux/minidump_writer/mod.rs +++ b/src/linux/minidump_writer/mod.rs @@ -752,12 +752,20 @@ impl MinidumpWriter { /// 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 /// available. - pub fn get_thread_info_by_index(&self, index: usize) -> Result { + pub fn get_thread_info_by_index( + &self, + index: usize, + soft_errors: impl WriteErrorList, + ) -> Result { if index > self.threads.len() { return Err(ThreadInfoError::IndexOutOfBounds(index, self.threads.len())); } - ThreadInfo::create(self.process_inspector.as_ref(), self.threads[index].tid) + ThreadInfo::create( + self.process_inspector.as_ref(), + self.threads[index].tid, + soft_errors, + ) } // Returns a valid stack pointer and the mapping that contains the stack. diff --git a/src/linux/minidump_writer/thread_list_stream.rs b/src/linux/minidump_writer/thread_list_stream.rs index 4bed4134..f0abad74 100644 --- a/src/linux/minidump_writer/thread_list_stream.rs +++ b/src/linux/minidump_writer/thread_list_stream.rs @@ -1,5 +1,8 @@ use { - super::*, crate::minidump_cpu::RawContextCPU, error_graph::WriteErrorList, failspot::failspot, + super::*, + crate::minidump_cpu::RawContextCPU, + error_graph::{ErrorList, WriteErrorList}, + failspot::failspot, std::cmp::min, }; @@ -41,6 +44,8 @@ pub enum SectionThreadListError { GetStackInfoFailed(#[source] Box), #[error("Failed to get thread info")] ThreadInfoError(#[from] ThreadInfoError), + #[error("Soft errors occurred getting thread info")] + ThreadInfoSoftErrors(#[source] ErrorList), #[error("Failed to write to memory buffer")] IOError( #[from] @@ -180,7 +185,10 @@ impl MinidumpWriter { self.crashing_thread_context = CrashingThreadContext::CrashContext(cpu_section.location()); } else { - let info = self.get_thread_info_by_index(idx)?; + let info = self.get_thread_info_by_index( + idx, + soft_errors.subwriter(SectionThreadListError::ThreadInfoSoftErrors), + )?; let max_stack_len = if self.minidump_size_limit.is_some() && idx >= LIMIT_BASE_THREAD_COUNT { extra_thread_stack_len diff --git a/src/linux/thread_info/aarch64.rs b/src/linux/thread_info/aarch64.rs index 670bd3ad..103b96c4 100644 --- a/src/linux/thread_info/aarch64.rs +++ b/src/linux/thread_info/aarch64.rs @@ -1,6 +1,7 @@ use { super::{Pid, ProcessInspector, ThreadInfoError, regs::*}, crate::minidump_cpu::{FP_REG_COUNT, GP_REG_COUNT, RawContextCPU}, + error_graph::WriteErrorList, }; #[derive(Debug)] @@ -9,7 +10,7 @@ pub struct ThreadInfoAarch64 { pub tgid: Pid, // thread group id pub ppid: Pid, // parent process pub regs: GenRegs, - pub fpregs: FpRegs, + pub fpregs: Option, } impl ThreadInfoAarch64 { @@ -29,22 +30,33 @@ impl ThreadInfoAarch64 { // field instead out.pc = self.regs.pc; - out.fpsr = self.fpregs.fpsr; - out.fpcr = self.fpregs.fpcr; - out.float_regs[..FP_REG_COUNT].copy_from_slice(&self.fpregs.vregs[..FP_REG_COUNT]); + let fpregs = self.fpregs.unwrap_or_default(); + out.fpsr = fpregs.fpsr; + out.fpcr = fpregs.fpcr; + out.float_regs[..FP_REG_COUNT].copy_from_slice(&fpregs.vregs[..FP_REG_COUNT]); } pub fn create( process_inspector: &dyn ProcessInspector, tid: Pid, + mut soft_errors: impl WriteErrorList, ) -> Result { let (ppid, tgid) = super::get_ppid_and_tgid(process_inspector, tid)?; + let regs = process_inspector .get_gen_regs(tid) - .map_err(ThreadInfoError::PtraceError)?; - let fpregs = process_inspector + .map_err(ThreadInfoError::GetGenRegsFailed)?; + + let fpregs = match process_inspector .get_fp_regs(tid) - .map_err(ThreadInfoError::PtraceError)?; + .map_err(ThreadInfoError::GetFpRegsFailed) + { + Ok(regs) => Some(regs), + Err(e) => { + soft_errors.push(e); + None + } + }; let stack_pointer = regs.sp as usize; diff --git a/src/linux/thread_info/arm.rs b/src/linux/thread_info/arm.rs index 6296e239..ec255d7a 100644 --- a/src/linux/thread_info/arm.rs +++ b/src/linux/thread_info/arm.rs @@ -1,6 +1,7 @@ use { super::{Pid, ProcessInspector, ThreadInfoError, regs::*}, crate::minidump_cpu::RawContextCPU, + error_graph::WriteErrorList, }; #[derive(Debug)] @@ -9,7 +10,7 @@ pub struct ThreadInfoArm { pub tgid: Pid, // thread group id pub ppid: Pid, // parent process pub regs: GenRegs, - pub fpregs: FpRegs, + pub fpregs: Option, } impl ThreadInfoArm { @@ -23,21 +24,34 @@ impl ThreadInfoArm { out.iregs.copy_from_slice(&self.regs.uregs[..16]); out.cpsr = self.regs.uregs[16]; - out.float_save.fpscr = self.fpregs.fpscr as u64; - out.float_save.regs = self.fpregs.fpregs; + + let fpregs = self.fpregs.unwrap_or_default(); + + out.float_save.fpscr = fpregs.fpscr as u64; + out.float_save.regs = fpregs.fpregs; } pub fn create( process_inspector: &dyn ProcessInspector, tid: Pid, + mut soft_errors: impl WriteErrorList, ) -> Result { let (ppid, tgid) = super::get_ppid_and_tgid(process_inspector, tid)?; + let regs = process_inspector .get_gen_regs(tid) - .map_err(ThreadInfoError::PtraceError)?; - let fpregs = process_inspector + .map_err(ThreadInfoError::GetGenRegsFailed)?; + + let fpregs = match process_inspector .get_fp_regs(tid) - .map_err(ThreadInfoError::PtraceError)?; + .map_err(ThreadInfoError::GetFpRegsFailed) + { + Ok(regs) => Some(regs), + Err(e) => { + soft_errors.push(e); + None + } + }; let stack_pointer = regs.uregs[13] as usize; diff --git a/src/linux/thread_info/mod.rs b/src/linux/thread_info/mod.rs index 81203d2c..a88b8c7e 100644 --- a/src/linux/thread_info/mod.rs +++ b/src/linux/thread_info/mod.rs @@ -32,8 +32,14 @@ pub enum ThreadInfoError { #[serde(skip)] std::num::ParseIntError, ), - #[error("ptrace error")] - PtraceError(#[source] process_inspection::Error), + #[error("failed to get general-purpose registers")] + GetGenRegsFailed(#[source] process_inspection::Error), + #[error("failed to get floating-point registers")] + GetFpRegsFailed(#[source] process_inspection::Error), + #[error("failed to get floating-point register extended info")] + GetFpxRegsFailed(#[source] process_inspection::Error), + #[error("failed to get debug registers")] + GetDebugRegsFailed(#[source] process_inspection::Error), #[error("Invalid line in /proc/{0}/status: {1}")] InvalidProcStatusFile(Pid, String), } diff --git a/src/linux/thread_info/x86.rs b/src/linux/thread_info/x86.rs index 78d90ecc..50512cb5 100644 --- a/src/linux/thread_info/x86.rs +++ b/src/linux/thread_info/x86.rs @@ -2,6 +2,7 @@ use { super::{Pid, ProcessInspector, ThreadInfoError, regs::*}, crate::{minidump_cpu::RawContextCPU, minidump_format::format}, core::mem, + error_graph::WriteErrorList, scroll::Pwrite, }; @@ -10,45 +11,66 @@ pub struct ThreadInfoX86 { pub tgid: Pid, // thread group id pub ppid: Pid, // parent process pub regs: GenRegs, - pub fpregs: FpRegs, + pub fpregs: Option, pub dregs: [RegType; NUM_DEBUG_REGISTERS], #[cfg(target_arch = "x86")] - pub fpxregs: FpxRegs, + pub fpxregs: Option, } impl ThreadInfoX86 { pub fn create( process_inspector: &dyn ProcessInspector, tid: Pid, + mut soft_errors: impl WriteErrorList, ) -> Result { let (ppid, tgid) = super::get_ppid_and_tgid(process_inspector, tid)?; + let regs = process_inspector .get_gen_regs(tid) - .map_err(ThreadInfoError::PtraceError)?; - let fpregs = process_inspector + .map_err(ThreadInfoError::GetGenRegsFailed)?; + + let fpregs = match process_inspector .get_fp_regs(tid) - .map_err(ThreadInfoError::PtraceError)?; + .map_err(ThreadInfoError::GetFpRegsFailed) + { + Ok(regs) => Some(regs), + Err(e) => { + soft_errors.push(e); + None + } + }; #[cfg(target_arch = "x86")] - let fpxregs = { - if cfg!(target_feature = "fxsr") { - process_inspector - .get_fpx_regs(tid) - .map_err(ThreadInfoError::PtraceError)? - } else { - unsafe { mem::zeroed() } + let fpxregs = if cfg!(target_feature = "fxsr") { + match process_inspector + .get_fpx_regs(tid) + .map_err(ThreadInfoError::GetFpxRegsFailed) + { + Ok(regs) => Some(regs), + Err(e) => { + soft_errors.push(e); + None + } } + } else { + None }; let mut dregs: [RegType; NUM_DEBUG_REGISTERS] = [0; NUM_DEBUG_REGISTERS]; let debug_offset = USER_STRUCT_DEBUGREG_OFFSET; for (idx, dreg) in dregs.iter_mut().enumerate() { - let chunk = process_inspector + match process_inspector .ptrace_peekuser(debug_offset + idx * mem::size_of::()) - .map_err(ThreadInfoError::PtraceError)?; - - *dreg = RegType::from_ne_bytes(chunk[0..mem::size_of::()].try_into().unwrap()); + .map_err(ThreadInfoError::GetDebugRegsFailed) + { + Ok(chunk) => { + *dreg = RegType::from_ne_bytes( + chunk[0..mem::size_of::()].try_into().unwrap(), + ) + } + Err(e) => soft_errors.push(e), + } } #[cfg(target_arch = "x86_64")] @@ -126,7 +148,7 @@ impl ThreadInfoX86 { out.rip = self.regs.rip; { - let fs = &self.fpregs; + let fs = self.fpregs.unwrap_or_default(); let mut float_save = crate::minidump_cpu::FloatStateCPU { control_word: fs.cwd, status_word: fs.swd, @@ -182,18 +204,20 @@ impl ThreadInfoX86 { out.esp = self.regs.esp; out.ss = self.regs.ss; - out.float_save.control_word = self.fpregs.cwd; - out.float_save.status_word = self.fpregs.swd; - out.float_save.tag_word = self.fpregs.twd; - out.float_save.error_offset = self.fpregs.fip; - out.float_save.error_selector = self.fpregs.fcs; - out.float_save.data_offset = self.fpregs.foo; - out.float_save.data_selector = self.fpregs.fos; + let fpregs = self.fpregs.unwrap_or_default(); + + out.float_save.control_word = fpregs.cwd; + out.float_save.status_word = fpregs.swd; + out.float_save.tag_word = fpregs.twd; + out.float_save.error_offset = fpregs.fip; + out.float_save.error_selector = fpregs.fcs; + out.float_save.data_offset = fpregs.foo; + out.float_save.data_selector = fpregs.fos; { let ra = &mut out.float_save.register_area; // 8 registers * 10 bytes per register. - for (idx, block) in self.fpregs.st_space.iter().enumerate() { + for (idx, block) in fpregs.st_space.iter().enumerate() { let offset = idx * std::mem::size_of::(); if offset >= ra.len() { break; @@ -216,26 +240,29 @@ impl ThreadInfoX86 { }; } + let fpregs = self.fpregs.unwrap_or_default(); + let fpxregs = self.fpxregs.unwrap_or_default(); + // This matches the Intel fpsave format. - write_er!(self.fpregs.cwd as u16); - write_er!(self.fpregs.swd as u16); - write_er!(self.fpregs.twd as u16); - write_er!(self.fpxregs.fop); - write_er!(self.fpxregs.fip); - write_er!(self.fpxregs.fcs); - write_er!(self.fpregs.foo); - write_er!(self.fpregs.fos); - write_er!(self.fpxregs.mxcsr); + write_er!(fpregs.cwd as u16); + write_er!(fpregs.swd as u16); + write_er!(fpregs.twd as u16); + write_er!(fpxregs.fop); + write_er!(fpxregs.fip); + write_er!(fpxregs.fcs); + write_er!(fpregs.foo); + write_er!(fpregs.fos); + write_er!(fpxregs.mxcsr); offset = 32; - for val in &self.fpxregs.st_space { + for val in &fpxregs.st_space { write_er!(val); } debug_assert_eq!(offset, 160); - for val in &self.fpxregs.xmm_space.0 { + for val in &fpxregs.xmm_space.0 { write_er!(val); } } diff --git a/tests/ptrace_dumper.rs b/tests/ptrace_dumper.rs index 299837fe..efad47f9 100644 --- a/tests/ptrace_dumper.rs +++ b/tests/ptrace_dumper.rs @@ -116,7 +116,7 @@ fn thread_list_from_parent() { for (idx, curr_thread) in dumper.threads.iter().enumerate() { println!("curr_thread: {curr_thread:?}"); let info = dumper - .get_thread_info_by_index(idx) + .get_thread_info_by_index(idx, error_graph::strategy::DontCare) .expect("Could not get thread info by index"); let (_valid_stack_ptr, stack_len) = dumper .get_stack_info(info.stack_pointer) @@ -293,7 +293,7 @@ fn sanitizes_stack_copies() { assert_eq!(dumper.threads.len(), num_of_threads); let thread_info = dumper - .get_thread_info_by_index(0) + .get_thread_info_by_index(0, error_graph::strategy::DontCare) .expect("Couldn't find thread_info"); let defaced;