From 981750fbc5f1a94cc6f337ceba835bea7ad56107 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 12 Aug 2024 13:59:01 +0200 Subject: [PATCH] Respect MADV_DONTDUMP --- src/bin/test.rs | 57 +++++++++++++++++++++++++ src/linux/maps_reader.rs | 67 +++++++++++++++++++---------- src/linux/ptrace_dumper.rs | 5 ++- tests/linux_minidump_writer.rs | 1 + tests/ptrace_dumper.rs | 78 ++++++++++++++++++++++++++++++++++ 5 files changed, 183 insertions(+), 25 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 6713852c..f4250205 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -294,6 +294,62 @@ mod linux { } } + fn spawn_alloc_w_madvise_wait() -> Result<()> { + let page_size: usize = nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE) + .unwrap() + .unwrap() + .try_into() + .unwrap(); + let memory_size = std::num::NonZeroUsize::new(page_size * 5).unwrap(); + + let mapped_mem = unsafe { + mmap_anonymous( + None, + memory_size, + ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, + MapFlags::MAP_PRIVATE | MapFlags::MAP_ANON, + ) + .unwrap() + }; + + let offset = page_size as isize; + + // Mark 2 of the 5 pages MADV_DONTDUMP + // |1-dump| |2-dont| |3-dont| |4-dump| |5-dump| + let pages = unsafe { + [ + (mapped_mem, true), + (mapped_mem.byte_offset(offset), false), + (mapped_mem.byte_offset(offset * 2), false), + (mapped_mem.byte_offset(offset * 3), true), + (mapped_mem.byte_offset(offset * 4), true), + ] + }; + + for (page, dump) in pages { + if !dump { + unsafe { + nix::sys::mman::madvise( + page, + page_size, + nix::sys::mman::MmapAdvise::MADV_DONTDUMP, + ) + .expect("failed to call madvise"); + } + } + } + + println!("--start--"); + for (page, dump) in pages { + println!("{:p}{}", page.as_ptr(), if dump { "" } else { " dd" }); + } + println!("--end--"); + + loop { + std::thread::park(); + } + } + fn create_files_wait(num: usize) -> Result<()> { let mut file_array = Vec::::with_capacity(num); for id in 0..num { @@ -325,6 +381,7 @@ mod linux { "linux_gate_mapping_id" => test_linux_gate_mapping_id(), "spawn_mmap_wait" => spawn_mmap_wait(), "spawn_alloc_wait" => spawn_alloc_wait(), + "spawn_alloc_w_madvise_wait" => spawn_alloc_w_madvise_wait(), _ => Err("Len 1: Unknown test option".into()), }, 2 => match args[0].as_ref() { diff --git a/src/linux/maps_reader.rs b/src/linux/maps_reader.rs index e023a21a..b276613b 100644 --- a/src/linux/maps_reader.rs +++ b/src/linux/maps_reader.rs @@ -38,6 +38,7 @@ pub struct MappingInfo { pub offset: usize, // offset into the backed file. pub permissions: MMPermissions, // read, write and execute permissions. pub name: Option, + pub madvise_dontdump: bool, // pub elf_obj: Option, } @@ -95,6 +96,10 @@ impl MappingInfo { let start_address: usize = mm.address.0.try_into()?; let end_address: usize = mm.address.1.try_into()?; let mut offset: usize = mm.offset.try_into()?; + let madvise_dontdump = mm + .extension + .vm_flags + .contains(procfs_core::process::VmFlags::DD); let mut pathname: Option = match mm.pathname { MMapPath::Path(p) => Some(sanitize_path(p.into())), @@ -118,28 +123,33 @@ impl MappingInfo { } if let Some(prev_module) = infos.last_mut() { - if (start_address == prev_module.end_address()) - && pathname.is_some() - && (pathname == prev_module.name) - { - // Merge adjacent mappings into one module, assuming they're a single - // library mapped by the dynamic linker. - prev_module.system_mapping_info.end_address = end_address; - prev_module.size = end_address - prev_module.start_address; - prev_module.permissions |= mm.perms; - continue; - } else if (start_address == prev_module.end_address()) - && prev_module.is_executable() - && prev_module.name_is_path() - && ((offset == 0) || (offset == prev_module.end_address())) - && (mm.perms == MMPermissions::PRIVATE) - { - // Also merge mappings that result from address ranges that the - // linker reserved but which a loaded library did not use. These - // appear as an anonymous private mapping with no access flags set - // and which directly follow an executable mapping. - prev_module.size = end_address - prev_module.start_address; - continue; + // Note that we don't merge pages if they have different MADV_DONTMAP + // settings, though AFAICT linux will handle this already and + // merge adjacent mappings only if they have the same settings + if prev_module.madvise_dontdump == madvise_dontdump { + if (start_address == prev_module.end_address()) + && pathname.is_some() + && (pathname == prev_module.name) + { + // Merge adjacent mappings into one module, assuming they're a single + // library mapped by the dynamic linker. + prev_module.system_mapping_info.end_address = end_address; + prev_module.size = end_address - prev_module.start_address; + prev_module.permissions |= mm.perms; + continue; + } else if (start_address == prev_module.end_address()) + && prev_module.is_executable() + && prev_module.name_is_path() + && ((offset == 0) || (offset == prev_module.end_address())) + && (mm.perms == MMPermissions::PRIVATE) + { + // Also merge mappings that result from address ranges that the + // linker reserved but which a loaded library did not use. These + // appear as an anonymous private mapping with no access flags set + // and which directly follow an executable mapping. + prev_module.size = end_address - prev_module.start_address; + continue; + } } } @@ -170,7 +180,7 @@ impl MappingInfo { } } - infos.push(MappingInfo { + infos.push(Self { start_address, size: end_address - start_address, system_mapping_info: SystemMappingInfo { @@ -180,6 +190,7 @@ impl MappingInfo { offset, permissions: mm.perms, name: pathname, + madvise_dontdump, }); } Ok(infos) @@ -323,7 +334,11 @@ impl MappingInfo { false } + /// Whether the mapping is "interesting" and should be appended to the + /// minidump's mappings + #[inline] pub fn is_interesting(&self) -> bool { + !self.madvise_dontdump && // only want modules with filenames. self.name.is_some() && // Only want to include one mapping per shared lib. @@ -529,6 +544,7 @@ ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsysca | MMPermissions::EXECUTE | MMPermissions::PRIVATE, name: Some("/usr/bin/cat".into()), + madvise_dontdump: false, }; assert_eq!(mappings[0], cat_map); @@ -543,6 +559,7 @@ ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsysca offset: 0, permissions: MMPermissions::READ | MMPermissions::WRITE | MMPermissions::PRIVATE, name: Some("[heap]".into()), + madvise_dontdump: false, }; assert_eq!(mappings[1], heap_map); @@ -557,6 +574,7 @@ ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsysca offset: 0, permissions: MMPermissions::READ | MMPermissions::WRITE | MMPermissions::PRIVATE, name: None, + madvise_dontdump: false, }; assert_eq!(mappings[2], empty_map); @@ -576,6 +594,7 @@ ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsysca offset: 0, permissions: MMPermissions::READ | MMPermissions::EXECUTE | MMPermissions::PRIVATE, name: Some("linux-gate.so".into()), + madvise_dontdump: false, }; assert_eq!(mappings[21], gate_map); @@ -639,6 +658,7 @@ ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsysca | MMPermissions::EXECUTE | MMPermissions::PRIVATE, name: Some("/lib64/libc-2.32.so".into()), + madvise_dontdump: false, }; assert_eq!(mappings[6], gate_map); @@ -671,6 +691,7 @@ a4840000-a4873000 rw-p 09021000 08:12 393449 /data/app/org.mozilla.firefox-1 | MMPermissions::EXECUTE | MMPermissions::PRIVATE, name: Some("/data/app/org.mozilla.firefox-1/lib/x86/libxul.so".into()), + madvise_dontdump: false, }; assert_eq!(mappings[0], gate_map); diff --git a/src/linux/ptrace_dumper.rs b/src/linux/ptrace_dumper.rs index 66f36564..a2265b81 100644 --- a/src/linux/ptrace_dumper.rs +++ b/src/linux/ptrace_dumper.rs @@ -313,7 +313,7 @@ impl PtraceDumper { // guaranteed (see http://crosbug.com/25355); therefore, try to use the // actual entry point to find the mapping. let entry_point_loc = self.auxv.get_entry_address().unwrap_or_default(); - let filename = format!("/proc/{}/maps", self.pid); + let filename = format!("/proc/{}/smaps", self.pid); let errmap = |e| InitError::IOError(filename.clone(), e); let maps_path = path::PathBuf::from(&filename); let maps_file = std::fs::File::open(maps_path).map_err(errmap)?; @@ -516,7 +516,8 @@ impl PtraceDumper { Ok(()) } - // Find the mapping which the given memory address falls in. + /// Find the mapping which the given memory address falls in. + #[inline] pub fn find_mapping(&self, address: usize) -> Option<&MappingInfo> { self.mappings .iter() diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index f6a14e34..7f38b0c9 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -164,6 +164,7 @@ contextual_test! { start_address: mmap_addr, end_address: mmap_addr + memory_size, }, + madvise_dontdump: false, }; let identifier = vec flag by ignoring +/// regions allocated with that flag set +#[test] +fn respects_madvise_dontdump() { + let mut child = start_child_and_return(&["spawn_alloc_w_madvise_wait"]); + let pid = child.id() as i32; + + let mut f = BufReader::new(child.stdout.as_mut().expect("Can't open stdout")); + + let mut buf = String::new(); + let mut mappings = Vec::new(); + + loop { + buf.clear(); + f.read_line(&mut buf).expect("failed to read line"); + let line = buf.trim(); + + if line == "--start--" { + assert!(mappings.is_empty()); + } else if line == "--end--" { + break; + } else { + let (ptr, dd) = line.split_once(' ').unwrap_or((line, "")); + match usize::from_str_radix(ptr.trim_start_matches("0x"), 16) { + Ok(ptr) => { + mappings.push((ptr, dd == "dd")); + } + Err(err) => panic!("failed to parse pointer '{ptr}': {err}"), + } + } + } + + let mut dumper = PtraceDumper::new( + pid, + minidump_writer::minidump_writer::STOP_TIMEOUT, + Default::default(), + ) + .expect("Couldn't init dumper"); + + dumper.suspend_threads().expect("failed to suspend threads"); + + let page_size: usize = nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE) + .unwrap() + .unwrap() + .try_into() + .unwrap(); + + for (address, dont_dump) in mappings { + let addrs = [ + address, + address + 1, + address + (page_size >> 1), + address + page_size - 1, + ]; + + for address in addrs { + let Some(mapping) = dumper.find_mapping(address) else { + panic!("failed to locate mapping for {address:#0x}"); + }; + + assert_eq!(mapping.madvise_dontdump, dont_dump); + + if dont_dump { + assert!(!mapping.is_interesting()); + } + } + } + + // Reap child + dumper.resume_threads().expect("Failed to resume threads"); + child.kill().expect("Failed to kill process"); + + let waitres = child.wait().expect("Failed to wait for child"); + let status = waitres.signal().expect("Child did not die due to signal"); + assert_eq!(waitres.code(), None); + assert_eq!(status, Signal::SIGKILL as i32); +}