Respect MADV_DONTDUMP

src/bin/test.rs

@@ -247,6 +247,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::<tempfile::NamedTempFile>::with_capacity(num);
         for id in 0..num {
@@ -278,6 +334,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() {

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<OsString>,
+    pub madvise_dontdump: bool,
     // pub elf_obj: Option<elf::Elf>,
 }
 
@@ -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<OsString> = 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);

src/linux/ptrace_dumper.rs

@@ -310,7 +310,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)?;
@@ -513,7 +513,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()

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![

tests/ptrace_dumper.rs

@@ -331,3 +331,81 @@ fn test_sanitize_stack_copy() {
     assert_eq!(waitres.code(), None);
     assert_eq!(status, Signal::SIGKILL as i32);
 }
+
+/// Tests that the ptrace dumper respect the [`MADV_DONTDUMP`](https://linux.die.net/man/2/madvise) 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);
+}