From 2d59eb2b7f1b1563765c2501500cdf56a620731a Mon Sep 17 00:00:00 2001 From: Gabriele Svelto Date: Wed, 25 Oct 2023 12:06:56 +0200 Subject: [PATCH 1/2] Add support for the `MINIDUMP_HANDLE_DATA_STREAM` to Linux This fixes issue #92 --- Cargo.toml | 8 +-- src/bin/test.rs | 24 +++++++ src/linux/errors.rs | 12 ++++ src/linux/minidump_writer.rs | 7 +- src/linux/sections.rs | 1 + src/linux/sections/handle_data_stream.rs | 84 ++++++++++++++++++++++++ src/minidump_format.rs | 4 +- tests/common/mod.rs | 5 ++ tests/linux_minidump_writer.rs | 40 +++++++++++ 9 files changed, 179 insertions(+), 6 deletions(-) create mode 100644 src/linux/sections/handle_data_stream.rs diff --git a/Cargo.toml b/Cargo.toml index 581b27d8..e8d6fe34 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ byteorder = "1.4" cfg-if = "1.0" crash-context = "0.6" memoffset = "0.9" -minidump-common = "0.18" +minidump-common = "0.19" scroll = "0.11" tempfile = "3.8" thiserror = "1.0" @@ -45,14 +45,14 @@ mach2 = "0.4" [dev-dependencies] # Minidump-processor is async so we need an executor futures = { version = "0.3", features = ["executor"] } -minidump = "0.18" +minidump = "0.19" memmap2 = "0.5" [target.'cfg(target_os = "macos")'.dev-dependencies] # We dump symbols for the `test` executable so that we can validate that minidumps # created by this crate can be processed by minidump-processor dump_syms = { version = "2.2", default-features = false } -minidump-processor = { version = "0.18", default-features = false } -minidump-unwind = { version = "0.18", features = ["debuginfo"] } +minidump-processor = { version = "0.19", default-features = false } +minidump-unwind = { version = "0.19", features = ["debuginfo"] } similar-asserts = "1.5" uuid = "1.4" diff --git a/src/bin/test.rs b/src/bin/test.rs index 4a3a9bb2..85b6fa6a 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -247,6 +247,26 @@ mod linux { } } + fn create_files_wait(num: usize) -> Result<()> { + let mut file_array = Vec::::with_capacity(num); + for id in 0..num { + let file = tempfile::Builder::new() + .prefix("test_file") + .suffix::(id.to_string().as_ref()) + .tempfile() + .unwrap(); + file_array.push(file); + println!("1"); + } + println!("1"); + loop { + std::thread::park(); + // This shouldn't be executed, but we put it here to ensure that + // all the files within the array are kept open. + println!("{}", file_array.len()); + } + } + pub(super) fn real_main(args: Vec) -> Result<()> { match args.len() { 1 => match args[0].as_ref() { @@ -268,6 +288,10 @@ mod linux { let num_of_threads: usize = args[1].parse().unwrap(); spawn_name_wait(num_of_threads) } + "create_files_wait" => { + let num_of_files: usize = args[1].parse().unwrap(); + create_files_wait(num_of_files) + } _ => Err(format!("Len 2: Unknown test option: {}", args[0]).into()), }, 3 => { diff --git a/src/linux/errors.rs b/src/linux/errors.rs index f5183ee9..b666fefa 100644 --- a/src/linux/errors.rs +++ b/src/linux/errors.rs @@ -142,6 +142,16 @@ pub enum SectionExceptionStreamError { MemoryWriterError(#[from] MemoryWriterError), } +#[derive(Debug, Error)] +pub enum SectionHandleDataStreamError { + #[error("Failed to access file")] + IOError(#[from] std::io::Error), + #[error("Failed to write to memory")] + MemoryWriterError(#[from] MemoryWriterError), + #[error("Failed integer conversion")] + TryFromIntError(#[from] std::num::TryFromIntError), +} + #[derive(Debug, Error)] pub enum SectionMappingsError { #[error("Failed to write to memory")] @@ -218,6 +228,8 @@ pub enum WriterError { SectionAppMemoryError(#[from] SectionAppMemoryError), #[error("Failed when writing section ExceptionStream")] SectionExceptionStreamError(#[from] SectionExceptionStreamError), + #[error("Failed when writing section HandleDataStream")] + SectionHandleDataStreamError(#[from] SectionHandleDataStreamError), #[error("Failed when writing section MappingsError")] SectionMappingsError(#[from] SectionMappingsError), #[error("Failed when writing section MemList")] diff --git a/src/linux/minidump_writer.rs b/src/linux/minidump_writer.rs index 72e030ba..7be6cef9 100644 --- a/src/linux/minidump_writer.rs +++ b/src/linux/minidump_writer.rs @@ -189,7 +189,7 @@ impl MinidumpWriter { ) -> Result<()> { // A minidump file contains a number of tagged streams. This is the number // of streams which we write. - let num_writers = 15u32; + let num_writers = 16u32; let mut header_section = MemoryWriter::::alloc(buffer)?; @@ -327,6 +327,11 @@ impl MinidumpWriter { // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; + // This section is optional, so we ignore errors when writing it + if let Ok(dirent) = handle_data_stream::write(self, buffer) { + let _ = dir_section.write_to_file(buffer, Some(dirent)); + } + // If you add more directory entries, don't forget to update num_writers, above. Ok(()) } diff --git a/src/linux/sections.rs b/src/linux/sections.rs index 3f7d3004..88d19f51 100644 --- a/src/linux/sections.rs +++ b/src/linux/sections.rs @@ -1,5 +1,6 @@ pub mod app_memory; pub mod exception_stream; +pub mod handle_data_stream; pub mod mappings; pub mod memory_info_list_stream; pub mod memory_list_stream; diff --git a/src/linux/sections/handle_data_stream.rs b/src/linux/sections/handle_data_stream.rs new file mode 100644 index 00000000..b41c542d --- /dev/null +++ b/src/linux/sections/handle_data_stream.rs @@ -0,0 +1,84 @@ +use std::{ + ffi::{CString, OsString}, + fs::{self, DirEntry}, + mem::{self}, + os::unix::prelude::OsStrExt, + path::{Path, PathBuf}, +}; + +use crate::mem_writer::MemoryWriter; + +use super::*; + +fn file_stat(path: &Path) -> Option { + let c_path = CString::new(path.as_os_str().as_bytes()).ok()?; + let mut stat = unsafe { std::mem::zeroed::() }; + let result = unsafe { libc::stat(c_path.as_ptr(), &mut stat) }; + + if result == 0 { + Some(stat) + } else { + None + } +} + +fn direntry_to_descriptor(buffer: &mut DumpBuf, entry: &DirEntry) -> Option { + let handle = filename_to_fd(&entry.file_name())?; + let realpath = fs::read_link(entry.path()).ok()?; + let path_rva = write_string_to_location(buffer, realpath.to_string_lossy().as_ref()).ok()?; + let stat = file_stat(&entry.path())?; + + // TODO: We store the contents of `st_mode` into the `attributes` field, but + // we could also store a human-readable string of the file type inside + // `type_name_rva`. We might move this missing information (and + // more) inside a custom `MINIDUMP_HANDLE_OBJECT_INFORMATION_TYPE` blob. + // That would make this conversion loss-less. + Some(MDRawHandleDescriptor { + handle, + type_name_rva: 0, + object_name_rva: path_rva.rva, + attributes: stat.st_mode, + granted_access: 0, + handle_count: 0, + pointer_count: 0, + }) +} + +fn filename_to_fd(filename: &OsString) -> Option { + let filename = filename.to_string_lossy(); + filename.parse::().ok() +} + +pub fn write( + config: &mut MinidumpWriter, + buffer: &mut DumpBuf, +) -> Result { + let proc_fd_path = PathBuf::from(format!("/proc/{}/fd", config.process_id)); + let proc_fd_iter = fs::read_dir(proc_fd_path)?; + let descriptors: Vec<_> = proc_fd_iter + .filter_map(|entry| entry.ok()) + .filter_map(|entry| direntry_to_descriptor(buffer, &entry)) + .collect(); + let number_of_descriptors = descriptors.len() as u32; + + let stream_header = MemoryWriter::::alloc_with_val( + buffer, + MDRawHandleDataStream { + size_of_header: mem::size_of::() as u32, + size_of_descriptor: mem::size_of::() as u32, + number_of_descriptors, + reserved: 0, + }, + )?; + + let mut dirent = MDRawDirectory { + stream_type: MDStreamType::HandleDataStream as u32, + location: stream_header.location(), + }; + + let descriptor_list = + MemoryArrayWriter::::alloc_from_iter(buffer, descriptors)?; + + dirent.location.data_size += descriptor_list.location().data_size; + Ok(dirent) +} diff --git a/src/minidump_format.rs b/src/minidump_format.rs index 9ced2b65..668ac332 100644 --- a/src/minidump_format.rs +++ b/src/minidump_format.rs @@ -2,7 +2,9 @@ pub use minidump_common::format::{ self, ArmElfHwCaps as MDCPUInformationARMElfHwCaps, PlatformId, ProcessorArchitecture as MDCPUArchitecture, GUID, MINIDUMP_DIRECTORY as MDRawDirectory, MINIDUMP_EXCEPTION as MDException, MINIDUMP_EXCEPTION_STREAM as MDRawExceptionStream, - MINIDUMP_HEADER as MDRawHeader, MINIDUMP_LOCATION_DESCRIPTOR as MDLocationDescriptor, + MINIDUMP_HANDLE_DATA_STREAM as MDRawHandleDataStream, + MINIDUMP_HANDLE_DESCRIPTOR as MDRawHandleDescriptor, MINIDUMP_HEADER as MDRawHeader, + MINIDUMP_LOCATION_DESCRIPTOR as MDLocationDescriptor, MINIDUMP_MEMORY_DESCRIPTOR as MDMemoryDescriptor, MINIDUMP_MEMORY_INFO as MDMemoryInfo, MINIDUMP_MEMORY_INFO_LIST as MDMemoryInfoList, MINIDUMP_MODULE as MDRawModule, MINIDUMP_SIGNATURE as MD_HEADER_SIGNATURE, MINIDUMP_STREAM_TYPE as MDStreamType, diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 2c1ded5f..bbb6abf3 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -58,6 +58,11 @@ pub fn start_child_and_wait_for_named_threads(num: usize) -> Child { start_child_and_wait_for_threads_helper("spawn_name_wait", num) } +#[allow(unused)] +pub fn start_child_and_wait_for_create_files(num: usize) -> Child { + start_child_and_wait_for_threads_helper("create_files_wait", num) +} + #[allow(unused)] pub fn wait_for_threads(child: &mut Child, num: usize) { let mut f = BufReader::new(child.stdout.as_mut().expect("Can't open stdout")); diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index f8eb7c61..8552003e 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -473,6 +473,46 @@ contextual_tests! { expected.insert(format!("thread_{}", id)); } assert_eq!(expected, names); + + } + + fn test_file_descriptors(context: Context) { + let num_of_files = 5; + let mut child = start_child_and_wait_for_create_files(num_of_files); + let pid = child.id() as i32; + + let mut tmpfile = tempfile::Builder::new() + .prefix("testfiles") + .tempfile() + .unwrap(); + + let mut tmp = context.minidump_writer(pid); + let _ = tmp.dump(&mut tmpfile).expect("Could not write minidump"); + child.kill().expect("Failed to kill process"); + + // Reap child + 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); + + // Read dump file and check its contents. There should be a truncated minidump available + let dump = Minidump::read_path(tmpfile.path()).expect("Failed to read minidump"); + let fds: MinidumpHandleDataStream = dump.get_stream().expect("Couldn't find MinidumpHandleDataStream"); + // We check that we create num_of_files plus stdin, stdout and stderr + for i in 0..2 { + let descriptor = fds.handles.get(i).expect("Descriptor should be present"); + let fd = *descriptor.raw.handle().expect("Handle should be populated"); + assert_eq!(fd, i as u64); + } + + for i in 3..num_of_files { + let descriptor = fds.handles.get(i).expect("Descriptor should be present"); + let object_name = descriptor.object_name.as_ref().expect("The path should be populated"); + let file_name = object_name.split('/').last().expect("The filename should be present"); + assert!(file_name.starts_with("test_file")); + assert!(file_name.ends_with(&(i - 3).to_string())); + } } } From 8cbca269a71a2e40c060718c1876d9c1044b7158 Mon Sep 17 00:00:00 2001 From: Gabriele Svelto Date: Thu, 2 Nov 2023 17:00:10 +0100 Subject: [PATCH 2/2] Update dependencies to align with rust-minidump and symbolic --- Cargo.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e8d6fe34..cbe887c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,8 +21,8 @@ thiserror = "1.0" [target.'cfg(unix)'.dependencies] libc = "0.2" -goblin = "0.7" -memmap2 = "0.5" +goblin = "0.7.1" +memmap2 = "0.8" [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] nix = { version = "0.27", default-features = false, features = [ @@ -33,7 +33,7 @@ nix = { version = "0.27", default-features = false, features = [ ] } # Used for parsing procfs info. # default-features is disabled since it pulls in chrono -procfs-core = { version = "0.16.0-RC1", default-features = false } +procfs-core = { version = "0.16", default-features = false } [target.'cfg(target_os = "windows")'.dependencies] bitflags = "2.4" @@ -46,7 +46,7 @@ mach2 = "0.4" # Minidump-processor is async so we need an executor futures = { version = "0.3", features = ["executor"] } minidump = "0.19" -memmap2 = "0.5" +memmap2 = "0.8" [target.'cfg(target_os = "macos")'.dev-dependencies] # We dump symbols for the `test` executable so that we can validate that minidumps