diff --git a/Cargo.lock b/Cargo.lock index a44cdb5271..e6ef165bba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4347,6 +4347,7 @@ version = "0.1.0" dependencies = [ "async-process", "crash-handler", + "libc", "log", "mach2 0.5.0", "minidumper", diff --git a/crates/crashes/Cargo.toml b/crates/crashes/Cargo.toml index f8b898112c..1b118097cb 100644 --- a/crates/crashes/Cargo.toml +++ b/crates/crashes/Cargo.toml @@ -16,6 +16,9 @@ serde_json.workspace = true system_specs.workspace = true zstd.workspace = true +[target.'cfg(target_os = "linux")'.dependencies] +libc.workspace = true + [target.'cfg(target_os = "macos")'.dependencies] mach2.workspace = true diff --git a/crates/crashes/src/crashes.rs b/crates/crashes/src/crashes.rs index 28217496ca..5d3fc954d3 100644 --- a/crates/crashes/src/crashes.rs +++ b/crates/crashes/src/crashes.rs @@ -138,6 +138,17 @@ where info!("crash signal handlers installed"); send_crash_server_message(&client, CrashServerMessage::Init(crash_init)); + #[cfg(all(target_os = "linux", target_env = "gnu"))] + if let Some(address) = abort_message_address() { + send_crash_server_message( + &client, + CrashServerMessage::AbortMessageLocation(AbortMessageLocation { + pid: process::id(), + address, + }), + ); + } + #[cfg(target_os = "linux")] handler.set_ptracer(Some(_crash_handler.id())); @@ -170,6 +181,7 @@ pub struct CrashServer { panic_info: Mutex>, active_gpu: Mutex>, user_info: Mutex>, + abort_message_location: Mutex>, has_connection: Arc, logs_dir: PathBuf, } @@ -179,11 +191,27 @@ pub struct CrashInfo { pub init: InitCrashHandler, pub panic: Option, pub minidump_error: Option, + /// The diagnostic the C runtime recorded before aborting the process, e.g. + /// glibc's "free(): invalid pointer". Only present when the crash was a + /// runtime-initiated abort rather than a signal like SIGSEGV or a panic. + #[serde(default)] + pub abort_message: Option, pub gpus: Vec, pub active_gpu: Option, pub user_info: Option, } +/// Where to find the C runtime's abort diagnostic in the crashed process's +/// memory. Sent by the client at startup so that after a crash the server can +/// recover the message with `process_vm_readv`; the crashed process itself +/// can't safely do this work, since its heap may be corrupt and its allocator +/// locks may be held by the crashed thread. +#[derive(Debug, Deserialize, Serialize, Clone, Copy)] +pub struct AbortMessageLocation { + pub pid: u32, + pub address: u64, +} + #[derive(Debug, Deserialize, Serialize, Clone)] pub struct InitCrashHandler { pub session_id: String, @@ -233,6 +261,110 @@ enum CrashServerMessage { Panic(CrashPanic), GPUInfo(GpuSpecs), UserInfo(UserInfo), + AbortMessageLocation(AbortMessageLocation), +} + +/// glibc records the diagnostic it prints just before aborting (malloc integrity +/// failures like "free(): invalid pointer", assertion failures, stack-smashing +/// reports) in the private global `__abort_msg`, specifically so it can be +/// recovered post-mortem. Resolve its address here, in a safe context at startup. +/// The symbol is only exported at the GLIBC_PRIVATE version, which plain `dlsym` +/// won't resolve, and it has no stability guarantee, so a null result (e.g. musl, +/// or a future glibc removing it) just disables this diagnostic. +#[cfg(all(target_os = "linux", target_env = "gnu"))] +fn abort_message_address() -> Option { + let ptr = unsafe { + libc::dlvsym( + libc::RTLD_DEFAULT, + c"__abort_msg".as_ptr(), + c"GLIBC_PRIVATE".as_ptr(), + ) + }; + std::ptr::NonNull::new(ptr).map(|ptr| ptr.as_ptr() as u64) +} + +/// Read the crashed process's abort diagnostic. `__abort_msg` points to a +/// `struct abort_msg_s { unsigned int size; char msg[]; }` that glibc allocates +/// with mmap so that it stays intact even when the heap is corrupt. `size` is +/// the total byte size of that mapping (header included, rounded up to whole +/// pages), not the message length; the message itself is NUL-terminated. +#[cfg(target_os = "linux")] +fn read_abort_message(location: AbortMessageLocation) -> Option { + let pointer_bytes = read_process_memory(location.pid, location.address, size_of::())?; + let message_address = usize::from_ne_bytes(pointer_bytes.try_into().ok()?) as u64; + if message_address == 0 { + return None; + } + let size_bytes = read_process_memory(location.pid, message_address, size_of::())?; + let size = u32::from_ne_bytes(size_bytes.try_into().ok()?); + let message_bytes = read_process_memory( + location.pid, + message_address + size_of::() as u64, + abort_message_read_len(size)?, + )?; + parse_abort_message(&message_bytes) +} + +/// How many message bytes to read given the `size` field of glibc's +/// `abort_msg_s`. `size` holds the total size of the mmap'd allocation, so a +/// value that isn't a whole number of pages means the layout has changed and +/// we shouldn't trust it. Reading is capped at (one page minus the header), +/// which both bounds the work and ensures the read never extends past the end +/// of the mapping. +#[cfg(any(target_os = "linux", test))] +fn abort_message_read_len(size: u32) -> Option { + // Every Linux page size (4 KiB, 16 KiB, 64 KiB, ...) is a multiple of 4 KiB. + const PAGE_MULTIPLE: usize = 4096; + const MAX_READ: usize = 4096; + + let size = size as usize; + if size == 0 || !size.is_multiple_of(PAGE_MULTIPLE) { + log::warn!("__abort_msg size field {size} is not page-rounded; layout may have changed"); + return None; + } + Some(size.min(MAX_READ) - size_of::()) +} + +/// The message is NUL-terminated inside a zero-filled mapping, so truncate at +/// the first NUL; `trim` alone would keep the padding, since NUL is not +/// whitespace. +#[cfg(any(target_os = "linux", test))] +fn parse_abort_message(bytes: &[u8]) -> Option { + let len = bytes + .iter() + .position(|&byte| byte == 0) + .unwrap_or(bytes.len()); + let message = String::from_utf8_lossy(&bytes[..len]).trim().to_string(); + (!message.is_empty()).then_some(message) +} + +#[cfg(target_os = "linux")] +fn read_process_memory(pid: u32, address: u64, len: usize) -> Option> { + let mut buffer = vec![0u8; len]; + let local = libc::iovec { + iov_base: buffer.as_mut_ptr().cast(), + iov_len: len, + }; + let remote = libc::iovec { + iov_base: address as *mut libc::c_void, + iov_len: len, + }; + let bytes_read = + unsafe { libc::process_vm_readv(pid as libc::pid_t, &local, 1, &remote, 1, 0) }; + if bytes_read < 0 { + log::warn!( + "process_vm_readv of {len} bytes at {address:#x} in pid {pid} failed: {}", + io::Error::last_os_error() + ); + return None; + } + if bytes_read as usize != len { + log::warn!( + "process_vm_readv short read at {address:#x} in pid {pid}: {bytes_read} of {len} bytes" + ); + return None; + } + Some(buffer) } impl minidumper::ServerHandler for CrashServer { @@ -281,6 +413,14 @@ impl minidumper::ServerHandler for CrashServer { } }; + // The crashed process is still alive at this point: it stays parked in + // its signal handler until the server acknowledges the dump request, + // which happens after this callback returns. + #[cfg(target_os = "linux")] + let abort_message = (*self.abort_message_location.lock()).and_then(read_abort_message); + #[cfg(not(target_os = "linux"))] + let abort_message = None; + let crash_info = CrashInfo { init: self .initialization_params @@ -289,6 +429,7 @@ impl minidumper::ServerHandler for CrashServer { .expect("not initialized"), panic: self.panic_info.lock().clone(), minidump_error, + abort_message, active_gpu: self.active_gpu.lock().clone(), gpus, user_info: self.user_info.lock().clone(), @@ -320,6 +461,9 @@ impl minidumper::ServerHandler for CrashServer { CrashServerMessage::UserInfo(user_info) => { self.user_info.lock().replace(user_info); } + CrashServerMessage::AbortMessageLocation(location) => { + self.abort_message_location.lock().replace(location); + } } } @@ -523,6 +667,7 @@ pub fn crash_server(socket: &Path, logs_dir: PathBuf) { initialization_params: Mutex::default(), panic_info: Mutex::default(), user_info: Mutex::default(), + abort_message_location: Mutex::default(), has_connection, active_gpu: Mutex::default(), logs_dir, @@ -532,3 +677,99 @@ pub fn crash_server(socket: &Path, logs_dir: PathBuf) { ) .expect("failed to run server"); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn abort_message_read_len_requires_page_rounded_total() { + assert_eq!(abort_message_read_len(0), None); + // A message length rather than a mapping total means the glibc layout + // has changed out from under us. + assert_eq!(abort_message_read_len(23), None); + assert_eq!(abort_message_read_len(4097), None); + // The read must stay within the mapping: one page minus the header. + assert_eq!(abort_message_read_len(4096), Some(4092)); + // Larger totals (long messages, larger page sizes) are clamped. + assert_eq!(abort_message_read_len(8192), Some(4092)); + assert_eq!(abort_message_read_len(65536), Some(4092)); + } + + #[test] + fn parse_abort_message_truncates_at_nul() { + let mut buffer = b"free(): invalid pointer\n\0".to_vec(); + buffer.resize(4092, 0); + assert_eq!( + parse_abort_message(&buffer), + Some("free(): invalid pointer".to_string()) + ); + } + + #[test] + fn parse_abort_message_handles_missing_nul() { + assert_eq!( + parse_abort_message(b"double free or corruption (out)"), + Some("double free or corruption (out)".to_string()) + ); + } + + #[test] + fn parse_abort_message_rejects_empty() { + assert_eq!(parse_abort_message(&[]), None); + assert_eq!(parse_abort_message(&[0; 16]), None); + assert_eq!(parse_abort_message(b"\n \0garbage after nul"), None); + } + + /// End-to-end check of `read_abort_message` against a synthetic + /// `abort_msg_s` in this very process (`process_vm_readv` may always read + /// one's own memory). The message page is followed by a `PROT_NONE` guard + /// page so the test fails if the read ever extends past the mapping glibc + /// would have allocated. + #[cfg(target_os = "linux")] + #[test] + fn read_abort_message_reads_glibc_layout_from_a_live_process() { + let page_size = unsafe { libc::sysconf(libc::_SC_PAGESIZE) } as usize; + unsafe { + let mapping = libc::mmap( + std::ptr::null_mut(), + 2 * page_size, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_ANON | libc::MAP_PRIVATE, + -1, + 0, + ); + assert_ne!(mapping, libc::MAP_FAILED); + assert_eq!( + libc::mprotect( + mapping.cast::().add(page_size).cast(), + page_size, + libc::PROT_NONE + ), + 0 + ); + + mapping.cast::().write(page_size as u32); + let message = b"free(): invalid pointer\n\0"; + std::ptr::copy_nonoverlapping( + message.as_ptr(), + mapping.cast::().add(size_of::()), + message.len(), + ); + + // Stands in for the `__abort_msg` global: a pointer variable whose + // address we hand to the reader. + let abort_msg: *mut libc::c_void = mapping; + let location = AbortMessageLocation { + pid: process::id(), + address: (&raw const abort_msg) as u64, + }; + assert_eq!( + read_abort_message(location), + Some("free(): invalid pointer".to_string()) + ); + + libc::munmap(mapping, 2 * page_size); + } + } +} diff --git a/crates/zed/src/reliability.rs b/crates/zed/src/reliability.rs index b4a06b2629..00917069d2 100644 --- a/crates/zed/src/reliability.rs +++ b/crates/zed/src/reliability.rs @@ -220,6 +220,21 @@ async fn upload_minidump( if let Some(minidump_error) = metadata.minidump_error.clone() { form = form.text("minidump_error", minidump_error); } + if let Some(abort_message) = metadata.abort_message.as_ref() { + // Sentry tag values are limited to 200 characters on a single line, so + // put a searchable prefix in the tag (which grouping rules also match + // on) and the full message in a context. + let tag: String = abort_message + .lines() + .next() + .unwrap_or_default() + .chars() + .take(200) + .collect(); + form = form + .text("sentry[tags][abort_message]", tag) + .text("sentry[contexts][abort][message]", abort_message.clone()); + } if let Some(is_staff) = &metadata .user_info