Remove some unsafe code; fix a soundness hole

Replace a number of lines of unsafe code with safe equivalents, some
using `zerocopy`. In one instance, fix a soundness hole (#720).

Closes #720

Author: joshlf

Cargo.lock

@@ -28,6 +28,7 @@ exclude = [
 [dependencies]
 cfg-if = "1.0"
 rustc-demangle = "0.1.24"
+zerocopy = { version = "0.8.26", features = ["derive"] }
 
 # Optionally enable the ability to serialize a `Backtrace`, controlled through
 # the `serialize-serde` feature below.

Cargo.toml

@@ -13,6 +13,8 @@ use super::super::{dbghelp, windows_sys::*};
 use core::ffi::c_void;
 use core::mem;
 
+use zerocopy::{FromBytes, FromZeros};
+
 #[derive(Clone, Copy)]
 pub enum StackFrame {
     New(STACKFRAME_EX),
@@ -92,6 +94,7 @@ impl Frame {
 }
 
 #[repr(C, align(16))] // required by `CONTEXT`, is a FIXME in windows metadata right now
+#[derive(FromBytes)]
 struct MyContext(CONTEXT);
 
 #[inline(always)]
@@ -100,7 +103,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) {
     let process = GetCurrentProcess();
     let thread = GetCurrentThread();
 
-    let mut context = mem::zeroed::<MyContext>();
+    let mut context = MyContext::new_zeroed();
     RtlCaptureContext(&mut context.0);
 
     // Ensure this process's symbols are initialized

src/backtrace/win32.rs

@@ -8,6 +8,7 @@
 
 use super::super::windows_sys::*;
 use core::ffi::c_void;
+use zerocopy::{FromBytes, FromZeros};
 
 #[derive(Clone, Copy)]
 pub struct Frame {
@@ -46,6 +47,7 @@ impl Frame {
     }
 }
 
+#[derive(FromBytes)]
 #[repr(C, align(16))] // required by `CONTEXT`, is a FIXME in windows metadata right now
 struct MyContext(CONTEXT);
 
@@ -80,8 +82,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) {
     use core::ptr;
 
     // Capture the initial context to start walking from.
-    // FIXME: shouldn't this have a Default impl?
-    let mut context = unsafe { core::mem::zeroed::<MyContext>() };
+    let mut context = MyContext::new_zeroed();
     unsafe { RtlCaptureContext(&mut context.0) };
 
     loop {

src/backtrace/win64.rs

@@ -1,7 +1,8 @@
 use core::fmt::{self, Write};
-use core::mem::{size_of, transmute};
+use core::mem::size_of;
 use core::slice::from_raw_parts;
 use libc::c_char;
+use zerocopy::{FromBytes, Immutable, IntoBytes, KnownLayout};
 
 unsafe extern "C" {
     // dl_iterate_phdr takes a callback that will receive a dl_phdr_info pointer
@@ -119,6 +120,7 @@ const NT_GNU_BUILD_ID: u32 = 3;
 // Elf_Nhdr represents an ELF note header in the endianness of the target.
 #[allow(non_camel_case_types)]
 #[repr(C)]
+#[derive(FromBytes, IntoBytes, KnownLayout, Immutable)]
 struct Elf_Nhdr {
     n_namesz: u32,
     n_descsz: u32,
@@ -181,15 +183,9 @@ fn take_bytes_align4<'a>(num: usize, bytes: &mut &'a [u8]) -> Option<&'a [u8]> {
 // architectures correctness). The values in the Elf_Nhdr fields might
 // be nonsense but this function ensures no such thing.
 fn take_nhdr<'a>(bytes: &mut &'a [u8]) -> Option<&'a Elf_Nhdr> {
-    if size_of::<Elf_Nhdr>() > bytes.len() {
-        return None;
-    }
-    // This is safe as long as there is enough space and we just confirmed that
-    // in the if statement above so this should not be unsafe.
-    let out = unsafe { transmute::<*const u8, &'a Elf_Nhdr>(bytes.as_ptr()) };
-    // Note that sice_of::<Elf_Nhdr>() is always 4-byte aligned.
+    let prefix = &bytes[..size_of::<Elf_Nhdr>()];
     *bytes = &bytes[size_of::<Elf_Nhdr>()..];
-    Some(out)
+    Elf_Nhdr::ref_from_bytes(prefix).ok()
 }
 
 impl<'a> Iterator for NoteIter<'a> {

src/print/fuchsia.rs

@@ -222,9 +222,20 @@ unsafe fn do_resolve(
     get_line_from_addr: impl FnOnce(&mut IMAGEHLP_LINEW64) -> BOOL,
     cb: &mut dyn FnMut(&super::Symbol),
 ) {
-    const SIZE: usize = 2 * MAX_SYM_NAME as usize + mem::size_of::<SYMBOL_INFOW>();
-    let mut data = Aligned8([0u8; SIZE]);
-    let info = unsafe { &mut *data.0.as_mut_ptr().cast::<SYMBOL_INFOW>() };
+    const TRAILER_SIZE: usize = 2 * MAX_SYM_NAME as usize;
+
+    #[repr(C)]
+    #[derive(zerocopy::IntoBytes)]
+    struct Data {
+        symbol_infow: SYMBOL_INFOW,
+        __max_sym_name: [u8; TRAILER_SIZE],
+    }
+
+    let mut data = Aligned8(Data {
+        symbol_infow: zerocopy::transmute!([0u8; size_of::<SYMBOL_INFOW>()]),
+        __max_sym_name: [0u8; TRAILER_SIZE],
+    });
+    let info = &mut data.0.symbol_infow;
     info.MaxNameLen = MAX_SYM_NAME as u32;
     // the struct size in C.  the value is different to
     // `size_of::<SYMBOL_INFOW>() - MAX_SYM_NAME + 1` (== 81)

src/symbolize/dbghelp.rs

@@ -38,8 +38,8 @@ pub(super) fn native_libraries() -> Vec<Library> {
     let mut libs = Vec::new();
 
     // Request the current link map from the runtime linker:
+    let mut map: *const LinkMap = <*const _>::null();
     let map = unsafe {
-        let mut map: *const LinkMap = mem::zeroed();
         if dlinfo(
             RTLD_SELF,
             RTLD_DI_LINKMAP,

src/symbolize/gimli/libs_illumos.rs

@@ -23,8 +23,7 @@ unsafe fn add_loaded_images(ret: &mut Vec<Library>) {
             return;
         }
 
-        // huge struct, probably should avoid manually initializing it even if we can
-        let mut me = MaybeUninit::<MODULEENTRY32W>::zeroed().assume_init();
+        let mut me = MODULEENTRY32W::new_zeroed();
         me.dwSize = mem::size_of_val(&me) as u32;
         if Module32FirstW(snap, &mut me) == TRUE {
             loop {

src/symbolize/gimli/libs_windows.rs

@@ -7,6 +7,9 @@
     dead_code,
     clippy::all
 )]
+
+use zerocopy::{FromBytes, Immutable, IntoBytes};
+
 windows_targets::link!("dbghelp.dll" "system" fn EnumerateLoadedModulesW64(hprocess : HANDLE, enumloadedmodulescallback : PENUMLOADED_MODULES_CALLBACKW64, usercontext : *const core::ffi::c_void) -> BOOL);
 windows_targets::link!("dbghelp.dll" "system" fn StackWalk64(machinetype : u32, hprocess : HANDLE, hthread : HANDLE, stackframe : *mut STACKFRAME64, contextrecord : *mut core::ffi::c_void, readmemoryroutine : PREAD_PROCESS_MEMORY_ROUTINE64, functiontableaccessroutine : PFUNCTION_TABLE_ACCESS_ROUTINE64, getmodulebaseroutine : PGET_MODULE_BASE_ROUTINE64, translateaddress : PTRANSLATE_ADDRESS_ROUTINE64) -> BOOL);
 windows_targets::link!("dbghelp.dll" "system" fn StackWalkEx(machinetype : u32, hprocess : HANDLE, hthread : HANDLE, stackframe : *mut STACKFRAME_EX, contextrecord : *mut core::ffi::c_void, readmemoryroutine : PREAD_PROCESS_MEMORY_ROUTINE64, functiontableaccessroutine : PFUNCTION_TABLE_ACCESS_ROUTINE64, getmodulebaseroutine : PGET_MODULE_BASE_ROUTINE64, translateaddress : PTRANSLATE_ADDRESS_ROUTINE64, flags : u32) -> BOOL);
@@ -58,7 +61,7 @@ pub struct ADDRESS64 {
 }
 pub type ADDRESS_MODE = i32;
 #[repr(C)]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, FromBytes, Immutable)]
 pub union ARM64_NT_NEON128 {
     pub Anonymous: ARM64_NT_NEON128_0,
     pub D: [f64; 2],
@@ -67,7 +70,7 @@ pub union ARM64_NT_NEON128 {
     pub B: [u8; 16],
 }
 #[repr(C)]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, FromBytes, Immutable)]
 pub struct ARM64_NT_NEON128_0 {
     pub Low: u64,
     pub High: i64,
@@ -76,7 +79,7 @@ pub const AddrModeFlat: ADDRESS_MODE = 3i32;
 pub type BOOL = i32;
 #[repr(C)]
 #[cfg(target_arch = "aarch64")]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, FromBytes)]
 pub struct CONTEXT {
     pub ContextFlags: CONTEXT_FLAGS,
     pub Cpsr: u32,
@@ -93,14 +96,14 @@ pub struct CONTEXT {
 }
 #[repr(C)]
 #[cfg(target_arch = "aarch64")]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, FromBytes, Immutable)]
 pub union CONTEXT_0 {
     pub Anonymous: CONTEXT_0_0,
     pub X: [u64; 31],
 }
 #[repr(C)]
 #[cfg(target_arch = "aarch64")]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, FromBytes, Immutable)]
 pub struct CONTEXT_0_0 {
     pub X0: u64,
     pub X1: u64,
@@ -217,7 +220,7 @@ pub struct CONTEXT_0_0 {
 }
 #[repr(C)]
 #[cfg(target_arch = "x86")]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, FromBytes)]
 pub struct CONTEXT {
     pub ContextFlags: CONTEXT_FLAGS,
     pub Dr0: u32,
@@ -292,7 +295,7 @@ pub struct FLOATING_SAVE_AREA {
 }
 #[repr(C)]
 #[cfg(target_arch = "x86")]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, FromBytes)]
 pub struct FLOATING_SAVE_AREA {
     pub ControlWord: u32,
     pub StatusWord: u32,
@@ -563,7 +566,7 @@ pub struct STACKFRAME_EX {
     pub InlineFrameContext: u32,
 }
 #[repr(C)]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, FromBytes, IntoBytes)]
 pub struct SYMBOL_INFOW {
     pub SizeOfStruct: u32,
     pub TypeIndex: u32,
@@ -572,6 +575,7 @@ pub struct SYMBOL_INFOW {
     pub Size: u32,
     pub ModBase: u64,
     pub Flags: SYMBOL_INFO_FLAGS,
+    __padding0: [u8; 4],
     pub Value: u64,
     pub Address: u64,
     pub Register: u32,
@@ -580,6 +584,7 @@ pub struct SYMBOL_INFOW {
     pub NameLen: u32,
     pub MaxNameLen: u32,
     pub Name: [u16; 1],
+    __padding1: [u8; 2],
 }
 pub type SYMBOL_INFO_FLAGS = u32;
 pub const SYMOPT_DEFERRED_LOADS: u32 = 4u32;