Skip to content

Remove the custom HANDLE stream #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jake-Shadle merged 3 commits into from
Sep 24, 2022
Merged

Remove the custom HANDLE stream #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b2a9eb
Remove unnecessary unsafe blocks
gabrielesvelto Sep 23, 2022
0d54554
Remove needless lazy evaluation
gabrielesvelto Sep 23, 2022
dc6e8f6
Remove custom HANDLE stream
gabrielesvelto Sep 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ features = [
"Win32_System_Diagnostics_Debug",
"Win32_System_Kernel",
"Win32_System_Memory",
# VerifierEnumerateResource and friends
"Win32_System_ApplicationVerifier",
# GetCurrentThreadId & OpenProcess
"Win32_System_Threading",
]
Expand Down
122 changes: 9 additions & 113 deletions src/windows/minidump_writer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
use crate::windows::errors::Error;
use minidump_common::format::{BreakpadInfoValid, MINIDUMP_BREAKPAD_INFO, MINIDUMP_STREAM_TYPE};
use scroll::Pwrite;
use std::{ffi::c_void, os::windows::io::AsRawHandle};
use std::os::windows::io::AsRawHandle;
pub use windows_sys::Win32::Foundation::HANDLE;
use windows_sys::Win32::{
Foundation::{
CloseHandle, ERROR_SUCCESS, STATUS_INVALID_HANDLE, STATUS_NONCONTINUABLE_EXCEPTION,
},
System::{ApplicationVerifier as av, Diagnostics::Debug as md, Threading as threading},
Foundation::{CloseHandle, STATUS_NONCONTINUABLE_EXCEPTION},
System::{Diagnostics::Debug as md, Threading as threading},
};

pub struct MinidumpWriter {
Expand All @@ -20,6 +18,7 @@ pub struct MinidumpWriter {
/// The id of the 'crashing' thread
tid: u32,
/// The exception code for the dump
#[allow(dead_code)]
exception_code: i32,
/// Whether we are dumping the current process or not
is_external_process: bool,
Expand Down Expand Up @@ -145,7 +144,7 @@ impl MinidumpWriter {
/// If [`crash_context::CrashContext::exception_pointers`] is specified, it
/// is the responsibility of the caller to ensure that the pointer is valid
/// for the duration of this function call.
pub unsafe fn dump_crash_context(
pub fn dump_crash_context(
crash_context: crash_context::CrashContext,
destination: &mut std::fs::File,
) -> Result<(), Error> {
Expand Down Expand Up @@ -174,7 +173,7 @@ impl MinidumpWriter {
let tid = crash_context.thread_id;
let exception_code = crash_context.exception_code;

let exc_info = (!crash_context.exception_pointers.is_null()).then(||
let exc_info = (!crash_context.exception_pointers.is_null()).then_some(
// https://docs.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_exception_information
md::MINIDUMP_EXCEPTION_INFORMATION {
ThreadId: crash_context.thread_id,
Expand All @@ -189,7 +188,8 @@ impl MinidumpWriter {
/// it can use eg `ReadProcessMemory` to get the contextual information from
/// the crash, rather than from the current process
ClientPointers: if is_external_process { 1 } else { 0 },
});
},
);

let mdw = Self {
exc_info,
Expand All @@ -207,7 +207,7 @@ impl MinidumpWriter {
fn dump(mut self, destination: &mut std::fs::File) -> Result<(), Error> {
let exc_info = self.exc_info.take();

let mut user_streams = Vec::with_capacity(2);
let mut user_streams = Vec::with_capacity(1);

let mut breakpad_info = self.fill_breakpad_stream();

Expand All @@ -220,26 +220,6 @@ impl MinidumpWriter {
});
}

let mut handle_stream_buffer = if self.exception_code == STATUS_INVALID_HANDLE {
self.fill_handle_stream()
} else {
None
};

// Note that we do this by ref, as the buffer inside the option needs
// to stay alive for as long as we're writing the minidump since
// the user stream has a pointer to it
if let Some(buf) = &mut handle_stream_buffer {
let handle_stream = md::MINIDUMP_USER_STREAM {
Type: MINIDUMP_STREAM_TYPE::HandleOperationListStream as u32,
BufferSize: buf.len() as u32,
// Still not getting over the mut pointers here
Buffer: buf.as_mut_ptr().cast(),
};

user_streams.push(handle_stream);
}

let user_stream_infos = md::MINIDUMP_USER_STREAM_INFORMATION {
UserStreamCount: user_streams.len() as u32,
UserStreamArray: user_streams.as_mut_ptr(),
Expand Down Expand Up @@ -305,90 +285,6 @@ impl MinidumpWriter {

Some(breakpad_info)
}

/// In the case of a `STATUS_INVALID_HANDLE` exception, this function
/// enumerates all of the handle operations that occurred within the crashing
/// process and fills out a minidump user stream with the ops pertaining to
/// the last invalid handle that is enumerated, which is, presumably
/// (hopefully?), the one that led to the exception
fn fill_handle_stream(&self) -> Option<Vec<u8>> {
// State object we pass to the enumeration
struct HandleState {
ops: Vec<av::AVRF_HANDLE_OPERATION>,
last_invalid: u64,
}

unsafe extern "system" fn enum_callback(
resource_description: *mut c_void,
enumeration_context: *mut c_void,
enumeration_level: *mut u32,
) -> u32 {
let description = &*resource_description.cast::<av::AVRF_HANDLE_OPERATION>();
let mut hs = &mut *enumeration_context.cast::<HandleState>();

// Remember the last invalid handle operation.
if description.OperationType == av::OperationDbBADREF as u32 {
hs.last_invalid = description.Handle;
}

// Record all handle operations.
hs.ops.push(*description);
*enumeration_level = av::HeapEnumerationEverything as u32;
ERROR_SUCCESS
}

let mut hs = HandleState {
ops: Vec::new(),
last_invalid: 0,
};

// https://docs.microsoft.com/en-us/windows/win32/api/avrfsdk/nf-avrfsdk-verifierenumerateresource
// SAFETY: syscall
if unsafe {
av::VerifierEnumerateResource(
self.crashing_process, // process to enumerate the handles for
0, // flags
av::AvrfResourceHandleTrace, // resource typea, we want to trace handles
Some(enum_callback), // enumeration callback
(&mut hs as *mut HandleState).cast(), // enumeration context
)
} == ERROR_SUCCESS
{
let mut stream_buf = Vec::new();

// https://docs.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_handle_operation_list
let mut md_list = md::MINIDUMP_HANDLE_OPERATION_LIST {
SizeOfHeader: std::mem::size_of::<md::MINIDUMP_HANDLE_OPERATION_LIST>() as u32,
SizeOfEntry: std::mem::size_of::<av::AVRF_HANDLE_OPERATION>() as u32,
NumberOfEntries: 0,
Reserved: 0,
};

stream_buf.resize(md_list.SizeOfHeader as usize, 0);

#[inline]
fn to_bytes<T: Sized>(v: &T) -> &[u8] {
// SAFETY: both AVRF_HANDLE_OPERATION and MINIDUMP_HANDLE_OPERATION_LIST
// are POD types
unsafe {
std::slice::from_raw_parts((v as *const T).cast(), std::mem::size_of::<T>())
}
}

for op in hs.ops.into_iter().filter(|op| op.Handle == hs.last_invalid) {
stream_buf.extend_from_slice(to_bytes(&op));
md_list.NumberOfEntries += 1;
}

stream_buf[..md_list.SizeOfHeader as usize].copy_from_slice(to_bytes(&md_list));

Some(stream_buf)
} else {
// We don't _particularly_ care if this fails, it's better if we had
// the info, but not critical
None
}
}
}

impl Drop for MinidumpWriter {
Expand Down
6 changes: 2 additions & 4 deletions tests/windows_minidump_writer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,8 @@ fn dump_external_process() {

// SAFETY: We keep the process we are dumping alive until the minidump is written
// and the test process keep the pointers it sent us alive until it is killed
unsafe {
MinidumpWriter::dump_crash_context(crash_context, tmpfile.as_file_mut())
.expect("failed to write minidump");
}
MinidumpWriter::dump_crash_context(crash_context, tmpfile.as_file_mut())
.expect("failed to write minidump");

child.kill().expect("failed to kill child");

Expand Down