From c9fb638b515a7e5226b57335e65b449aefedb1d5 Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Tue, 6 Oct 2026 12:23:29 +0200 Subject: [PATCH 1/4] linux: read the stacks right up until the guard page In recent glibc (>= 2.42), guard pages use the MADV_GUARD_INSTALL mechanism which isn't reflected in procmaps, so some stacks are merged together in a single mapping, and reading that entire mapping fails due to some pages within it being guarded. Instead, we take another approach: we know where the stack starts, so we just read from there up until we can't anymore, which would be our guard page. Fixes #244 --- src/bin/test.rs | 44 +++++++++++++ src/linux/minidump_writer/mod.rs | 30 ++++++++- .../minidump_writer/thread_list_stream.rs | 3 +- tests/ptrace_dumper.rs | 66 +++++++++++++++++++ 4 files changed, 141 insertions(+), 2 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index e2ad1b54..2619ecd0 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -304,6 +304,49 @@ mod linux { } } + /// Maps 4 pages filled with 0xab, with an unreadable guard page in the + /// third one, and prints the mapping address, mapping length, top of + /// stack, stack size, guard page address and page size. + fn spawn_guarded_mmap_wait() -> Result<()> { + // Not in the libc crate yet. + const MADV_GUARD_INSTALL: libc::c_int = 102; + + let page_size = unsafe { libc::sysconf(libc::_SC_PAGESIZE) } as usize; + let len = 4 * page_size; + let mapping = unsafe { + libc::mmap( + ptr::null_mut(), + len, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; + test!(mapping != libc::MAP_FAILED, "mmap failed"); + unsafe { ptr::write_bytes(mapping.cast::(), 0xab, len) }; + + let guard = unsafe { mapping.add(2 * page_size) }; + let top_of_stack = unsafe { mapping.add(3 * page_size) }; + let stack_size = page_size; + // Kernels older than 6.13 don't support MADV_GUARD_INSTALL, unmap the + // page instead. A PROT_NONE page wouldn't do, as /proc//mem and + // ptrace can still read it. + let rc = unsafe { libc::madvise(guard, page_size, MADV_GUARD_INSTALL) }; + if rc != 0 { + let rc = unsafe { libc::munmap(guard, page_size) }; + test!(rc == 0, "munmap failed"); + } + + println!( + "{} {} {} {} {} {page_size}", + mapping as usize, len, top_of_stack as usize, stack_size, guard as usize, + ); + loop { + thread::park(); + } + } + fn spawn_mmap_wait() -> Result<()> { let page_size = unsafe { libc::sysconf(libc::_SC_PAGESIZE) }; assert!(page_size > 0); @@ -397,6 +440,7 @@ mod linux { "mappings_include_linux_gate" => test_mappings_include_linux_gate(), "linux_gate_mapping_id" => test_linux_gate_mapping_id(), "spawn_mmap_wait" => spawn_mmap_wait(), + "spawn_guarded_mmap_wait" => spawn_guarded_mmap_wait(), "spawn_alloc_wait" => spawn_alloc_wait(), _ => Err("Len 1: Unknown test option".into()), }, diff --git a/src/linux/minidump_writer/mod.rs b/src/linux/minidump_writer/mod.rs index 395f9697..f280ba32 100644 --- a/src/linux/minidump_writer/mod.rs +++ b/src/linux/minidump_writer/mod.rs @@ -532,8 +532,9 @@ impl MinidumpWriter { } }; - let stack_copy = match MinidumpWriter::copy_from_process( + let stack_copy = match MinidumpWriter::read_until_guard( self.process_inspector.as_ref(), + self.page_size, valid_stack_pointer, stack_len, ) { @@ -1068,6 +1069,33 @@ impl MinidumpWriter { let mem = process_inspector.process_reader(); mem.read_all_to_vec(src, length) } + + /// Copies up to `length` bytes from the target process, stopping at the + /// first page that can't be read, typically a guard page. Fails only if + /// nothing could be read at all. + pub fn read_until_guard( + process_inspector: &dyn ProcessInspector, + page_size: usize, + src: usize, + length: usize, + ) -> Result, CopyFromProcessError> { + let mem = process_inspector.process_reader(); + let mut output = vec![0u8; length]; + let mut offset = 0; + while offset < length { + let chunk = (length - offset).min(page_size - (src + offset) % page_size); + if let Err(e) = mem.read_exact(src + offset, &mut output[offset..offset + chunk]) { + if offset == 0 { + return Err(e); + } + break; + } + offset += chunk; + } + + output.truncate(offset); + Ok(output) + } } impl Drop for MinidumpWriter { diff --git a/src/linux/minidump_writer/thread_list_stream.rs b/src/linux/minidump_writer/thread_list_stream.rs index f0abad74..039146cc 100644 --- a/src/linux/minidump_writer/thread_list_stream.rs +++ b/src/linux/minidump_writer/thread_list_stream.rs @@ -258,8 +258,9 @@ impl MinidumpWriter { let stack_copy = failspot!(if ThreadStackCopy { Err(CopyFromProcessError::InvalidArgument) } else { - MinidumpWriter::copy_from_process( + MinidumpWriter::read_until_guard( self.process_inspector.as_ref(), + self.page_size, valid_stack_ptr, stack_len, ) diff --git a/tests/ptrace_dumper.rs b/tests/ptrace_dumper.rs index c8b527f9..6199e2b9 100644 --- a/tests/ptrace_dumper.rs +++ b/tests/ptrace_dumper.rs @@ -389,3 +389,69 @@ fn sanitizes_stack_copies() { assert_eq!(waitres.code(), None); assert_eq!(status, libc::SIGKILL); } + +// Ensures that reads stop right at a guard page, whichever way the memory is read +#[test] +fn reads_until_guard_page() { + use minidump_writer::{ProcessReaderKind, minidump_writer::MinidumpWriter}; + + let mut child = start_child_and_return(&["spawn_guarded_mmap_wait"]); + let pid = child.id() as i32; + + let mut line = String::new(); + BufReader::new(child.stdout.as_mut().expect("Can't open stdout")) + .read_line(&mut line) + .expect("Couldn't read address provided by child"); + let [mapping, map_len, .., guard, page_size]: [usize; 6] = line + .split_whitespace() + .map(|x| x.parse().unwrap()) + .collect::>() + .try_into() + .unwrap(); + + let mut dumper = assert_no_soft_errors!( + soft_errors, + remote_mw_config(pid, pid).build_for_testing(&mut soft_errors) + ) + .expect("Couldn't init dumper"); + + for kind in [ + ProcessReaderKind::VirtualMem, + ProcessReaderKind::File, + ProcessReaderKind::Ptrace, + ] { + let name = format!("{kind:?}"); + dumper + .process_inspector + .force_process_reader_kind(kind) + .unwrap(); + + // Start in the middle of a page, to check that the reads stop at the + // guard page rather than at some multiple of the page size. + let src = mapping + 100; + let data = MinidumpWriter::read_until_guard( + dumper.process_inspector.as_ref(), + page_size, + src, + map_len - 100, + ) + .unwrap_or_else(|e| panic!("{name}: read failed: {e}")); + assert_eq!(data.len(), guard - src, "{name}"); + assert!(data.iter().all(|&b| b == 0xab), "{name}"); + + assert!( + MinidumpWriter::read_until_guard( + dumper.process_inspector.as_ref(), + page_size, + guard, + page_size + ) + .is_err(), + "{name}: read starting at the guard page succeeded" + ); + } + + drop(dumper); + child.kill().expect("Failed to kill process"); + child.wait().expect("Failed to wait for child"); +} From 5121934b533fb9c6d3c76d524a650346a1b88450 Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Tue, 6 Oct 2026 12:23:43 +0200 Subject: [PATCH 2/4] linux: split get_stack_info into smaller functions More functions to be used later on, with only one small observable effect: when get_stack_info can't find the end of the guard area, it now errors out rather than return the last visited page, which would inevitably trigger a failure down the road. --- src/linux/minidump_writer/mod.rs | 55 +++++++++++++------------------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/src/linux/minidump_writer/mod.rs b/src/linux/minidump_writer/mod.rs index f280ba32..c249ca54 100644 --- a/src/linux/minidump_writer/mod.rs +++ b/src/linux/minidump_writer/mod.rs @@ -878,46 +878,37 @@ impl MinidumpWriter { pub fn get_stack_info(&self, int_stack_pointer: usize) -> Result<(usize, usize), WriterError> { // Round the stack pointer to the nearest page, this will cause us to // capture data below the stack pointer which might still be relevant. - let mut stack_pointer = int_stack_pointer & !(self.page_size - 1); - let mut mapping = self.find_mapping(stack_pointer); + let (stack_pointer, mapping) = self + .skip_guard_pages(int_stack_pointer & !(self.page_size - 1)) + .ok_or(WriterError::NoStackPointerMapping)?; + Ok((stack_pointer, mapping.end_address() - stack_pointer)) + } + /// Returns the first page at or above `address` that may be part of a + /// stack, and the mapping containing it. + fn skip_guard_pages(&self, mut address: usize) -> Option<(usize, &MappingInfo)> { // The guard page has been 1 MiB in size since kernel 4.12, older // kernels used a 4 KiB one instead. Note the saturating add, as 32-bit // processes can have a stack pointer within 1MiB of usize::MAX - let guard_page_max_addr = stack_pointer.saturating_add(1024 * 1024); - - // If we found no mapping, or the mapping we found has no permissions - // then we might have hit a guard page, try looking for a mapping in - // addresses past the stack pointer. Stack grows towards lower addresses - // on the platforms we care about so the stack should appear after the - // guard page. - while !Self::may_be_stack(mapping) && (stack_pointer <= guard_page_max_addr) { - stack_pointer += self.page_size; - mapping = self.find_mapping(stack_pointer); - } + let guard_page_max_addr = address.saturating_add(1024 * 1024); - mapping - .map(|mapping| { - let valid_stack_pointer = if mapping.contains_address(stack_pointer) { - stack_pointer - } else { - mapping.start_address - }; + while address <= guard_page_max_addr { + if let Some(mapping) = self.find_mapping(address) + && Self::may_be_stack(mapping) + { + return Some((address, mapping)); + } + // Look upwards since the stack grows downwards on supported platforms + address = address.checked_add(self.page_size)?; + } - let stack_len = mapping.size - (valid_stack_pointer - mapping.start_address); - (valid_stack_pointer, stack_len) - }) - .ok_or(WriterError::NoStackPointerMapping) + None } - fn may_be_stack(mapping: Option<&MappingInfo>) -> bool { - if let Some(mapping) = mapping { - return mapping - .permissions - .intersects(MMPermissions::READ | MMPermissions::WRITE); - } - - false + fn may_be_stack(mapping: &MappingInfo) -> bool { + mapping + .permissions + .intersects(MMPermissions::READ | MMPermissions::WRITE) } pub fn sanitize_stack_copy( From b92e2b7ded96f29644a65357488f69db712dc79b Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Tue, 6 Oct 2026 18:47:59 +0200 Subject: [PATCH 3/4] linux: skip guard pages installed with MADV_GUARD_INSTALL As it turns out, just because a page permissions tell you it's readable doesn't actually mean it is. You can madvise() the kernel to segfault on that page via MADV_GUARD_INSTALL, so we actually try and read stuff to be sure that we're safe. --- src/bin/test.rs | 24 +++++++++---- src/linux/minidump_writer/mod.rs | 3 ++ tests/linux_minidump_writer.rs | 61 ++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 2619ecd0..92128c54 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -304,10 +304,17 @@ mod linux { } } + /// If MADV_GUARD_INSTALL fails, backup method to use to install + /// test guard pages. + enum BackupGuard { + Unmap, + MProtect, + } + /// Maps 4 pages filled with 0xab, with an unreadable guard page in the /// third one, and prints the mapping address, mapping length, top of /// stack, stack size, guard page address and page size. - fn spawn_guarded_mmap_wait() -> Result<()> { + fn spawn_guarded_mmap_wait(backup_guard: BackupGuard) -> Result<()> { // Not in the libc crate yet. const MADV_GUARD_INSTALL: libc::c_int = 102; @@ -329,13 +336,15 @@ mod linux { let guard = unsafe { mapping.add(2 * page_size) }; let top_of_stack = unsafe { mapping.add(3 * page_size) }; let stack_size = page_size; - // Kernels older than 6.13 don't support MADV_GUARD_INSTALL, unmap the - // page instead. A PROT_NONE page wouldn't do, as /proc//mem and - // ptrace can still read it. let rc = unsafe { libc::madvise(guard, page_size, MADV_GUARD_INSTALL) }; if rc != 0 { - let rc = unsafe { libc::munmap(guard, page_size) }; - test!(rc == 0, "munmap failed"); + let rc = match backup_guard { + BackupGuard::Unmap => unsafe { libc::munmap(guard, page_size) }, + BackupGuard::MProtect => unsafe { + libc::mprotect(guard, page_size, libc::PROT_NONE) + }, + }; + test!(rc == 0, "couldn't install the guard page"); } println!( @@ -440,7 +449,8 @@ mod linux { "mappings_include_linux_gate" => test_mappings_include_linux_gate(), "linux_gate_mapping_id" => test_linux_gate_mapping_id(), "spawn_mmap_wait" => spawn_mmap_wait(), - "spawn_guarded_mmap_wait" => spawn_guarded_mmap_wait(), + "spawn_guarded_mmap_wait" => spawn_guarded_mmap_wait(BackupGuard::Unmap), + "spawn_guarded_stack_wait" => spawn_guarded_mmap_wait(BackupGuard::MProtect), "spawn_alloc_wait" => spawn_alloc_wait(), _ => Err("Len 1: Unknown test option".into()), }, diff --git a/src/linux/minidump_writer/mod.rs b/src/linux/minidump_writer/mod.rs index c249ca54..d7982a58 100644 --- a/src/linux/minidump_writer/mod.rs +++ b/src/linux/minidump_writer/mod.rs @@ -891,10 +891,13 @@ impl MinidumpWriter { // kernels used a 4 KiB one instead. Note the saturating add, as 32-bit // processes can have a stack pointer within 1MiB of usize::MAX let guard_page_max_addr = address.saturating_add(1024 * 1024); + let mem = self.process_inspector.process_reader(); while address <= guard_page_max_addr { if let Some(mapping) = self.find_mapping(address) && Self::may_be_stack(mapping) + // Actually read a byte. Permissions sometimes lie. + && matches!(mem.read(address, &mut [0u8]), Ok(1)) { return Some((address, mapping)); } diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index e9f7af98..ae4e35b3 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -559,6 +559,67 @@ contextual_test! { } } +/// The stack pointer of a thread that overflowed its stack points into its +/// guard page. With glibc 2.42 and later, guard pages are installed with +/// `MADV_GUARD_INSTALL` and are part of the stack mapping, but can't be read. +/// On kernels without it, the child falls back to a separate `PROT_NONE` +/// mapping like older libcs. +/// +/// The writer doesn't check that the stack pointer is within the thread's +/// actual stack, so we point the main thread's at a guarded mapping. +#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] +#[test] +fn stack_pointer_in_guard_page() { + let mut child = start_child_and_return(&["spawn_guarded_stack_wait"]); + let pid = child.id() as i32; + + let mut line = String::new(); + BufReader::new(child.stdout.as_mut().unwrap()) + .read_line(&mut line) + .expect("Couldn't read mapping address from child"); + let [mapping, _, top_of_stack, stack_size, guard, page_size]: [usize; 6] = line + .split_whitespace() + .map(|x| x.parse().unwrap()) + .collect::>() + .try_into() + .unwrap(); + + let mut crash_context = get_dummy_crash_context(pid); + let sp = guard + page_size / 2; + #[cfg(target_arch = "x86_64")] + { + crash_context.inner.context.uc_mcontext.gregs[libc::REG_RSP as usize] = sp as _; + } + #[cfg(target_arch = "aarch64")] + { + crash_context.inner.context.uc_mcontext.sp = sp as _; + } + + let mut tmpfile = tempfile::Builder::new() + .prefix("stack_pointer_in_guard_page") + .tempfile() + .unwrap(); + let mut config = remote_mw_config(pid, pid); + config.set_crash_context(crash_context); + config + .write(&mut tmpfile) + .expect("Could not write minidump"); + child.kill().expect("Failed to kill process"); + child.wait().expect("Failed to wait for child"); + + let dump = Minidump::read_path(tmpfile.path()).expect("Failed to read minidump"); + let thread_list: MinidumpThreadList = dump.get_stream().expect("No thread list"); + let thread = thread_list + .get_thread(pid as u32) + .expect("Crashing thread not in thread list"); + let stack = &thread.raw.stack; + assert_eq!(stack.start_of_memory_range, top_of_stack as u64); + assert!(stack.memory.data_size as usize >= stack_size); + let rva = stack.memory.rva as usize; + let bytes = std::fs::read(tmpfile.path()).unwrap(); + assert!(bytes[rva..rva + stack_size].iter().all(|&b| b == 0xab)); +} + #[test] fn minidump_size_limit() { let num_of_threads = 40; From 37824b54ca100e96160fd3df5660476a8ea1deca Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Tue, 6 Oct 2026 12:08:58 +0200 Subject: [PATCH 4/4] ci: run Linux tests on all supported Ubuntu LTS releases This should allow us a decent coverage of the various glibc quirks over time. That would have caught the madvise thing. --- .github/workflows/ci.yml | 21 ++++++++++++++------- tests/linux_minidump_writer.rs | 2 +- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 156ffcd8..4c1757a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,16 +28,23 @@ jobs: test: name: Test - runs-on: ${{ matrix.job.os }} + runs-on: ${{ matrix.os || format('ubuntu-{0}{1}', matrix.ubuntu, matrix.job.arm && '-arm' || '') }} strategy: matrix: + # Supported Ubuntu LTS releases + ubuntu: ['22.04', '24.04', '26.04'] job: - - { os: ubuntu-24.04, target: x86_64-unknown-linux-gnu, release: true } - - { os: ubuntu-24.04, target: x86_64-unknown-linux-musl } - - { os: ubuntu-24.04-arm, target: aarch64-unknown-linux-gnu } - - { os: windows-2025, target: x86_64-pc-windows-msvc } - - { os: macos-15-intel, target: x86_64-apple-darwin } - - { os: macos-15, target: aarch64-apple-darwin } + - { target: x86_64-unknown-linux-gnu, release: true } + - { target: aarch64-unknown-linux-gnu, arm: true } + # These don't match any of the Ubuntu combinations, so each is added + # as a single extra job. + include: + # musl comes with the Rust toolchain, not the OS, and the kernel + # versions are covered by the glibc jobs. + - { ubuntu: '24.04', job: { target: x86_64-unknown-linux-musl } } + - { os: windows-2025, job: { target: x86_64-pc-windows-msvc } } + - { os: macos-15-intel, job: { target: x86_64-apple-darwin } } + - { os: macos-15, job: { target: aarch64-apple-darwin } } steps: - uses: actions/checkout@v4 - name: Install Rust diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index ae4e35b3..143ab3f4 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -577,7 +577,7 @@ fn stack_pointer_in_guard_page() { BufReader::new(child.stdout.as_mut().unwrap()) .read_line(&mut line) .expect("Couldn't read mapping address from child"); - let [mapping, _, top_of_stack, stack_size, guard, page_size]: [usize; 6] = line + let [.., top_of_stack, stack_size, guard, page_size]: [usize; 6] = line .split_whitespace() .map(|x| x.parse().unwrap()) .collect::>()