Skip to content

Commit f9ae50a

Browse files
Remove padding bytes risk in dbghelp with MaybeUninit
As reported in #720, there is a risk that the current code, by using &mut to a struct with padding fields, interacts in ways that cause padding bytes to be written to bytes that Rust originally thought were real and initialized. If this assumption persists forward in time far enough, this could possibly cause an issue due to compiler optimizations. This seems unlikely, but we can fix this by using MaybeUninit and then addressing the data using raw pointers only. That way, we do not have to depend on all the data being in initialized states even after calling SymFromAddrW. Except for the specific fields we read, of course.
1 parent dccdb4d commit f9ae50a

File tree

1 file changed

+13
-8
lines changed

1 file changed

+13
-8
lines changed

src/symbolize/dbghelp.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@
1919

2020
use super::super::{dbghelp, windows_sys::*};
2121
use super::{BytesOrWideString, ResolveWhat, SymbolName};
22+
use core::cmp;
2223
use core::ffi::c_void;
2324
use core::marker;
24-
use core::mem;
25+
use core::mem::{self, MaybeUninit};
2526
use core::ptr;
2627
use core::slice;
2728

@@ -217,19 +218,20 @@ unsafe fn resolve_with_inline(
217218
Some(())
218219
}
219220

221+
/// This function is only meant to be called with SymFromAddrW as the argument
220222
unsafe fn do_resolve(
221223
sym_from_addr: impl FnOnce(*mut SYMBOL_INFOW) -> BOOL,
222224
get_line_from_addr: impl FnOnce(&mut IMAGEHLP_LINEW64) -> BOOL,
223225
cb: &mut dyn FnMut(&super::Symbol),
224226
) {
225227
const SIZE: usize = 2 * MAX_SYM_NAME as usize + mem::size_of::<SYMBOL_INFOW>();
226-
let mut data = Aligned8([0u8; SIZE]);
227-
let info = unsafe { &mut *data.0.as_mut_ptr().cast::<SYMBOL_INFOW>() };
228-
info.MaxNameLen = MAX_SYM_NAME as u32;
228+
let mut data = MaybeUninit::<Aligned8<[u8; SIZE]>>::zeroed();
229+
let info = data.as_mut_ptr().cast::<SYMBOL_INFOW>();
230+
unsafe { (*info).MaxNameLen = MAX_SYM_NAME as u32 };
229231
// the struct size in C. the value is different to
230232
// `size_of::<SYMBOL_INFOW>() - MAX_SYM_NAME + 1` (== 81)
231233
// due to struct alignment.
232-
info.SizeOfStruct = 88;
234+
unsafe { (*info).SizeOfStruct = 88 };
233235

234236
if sym_from_addr(info) != TRUE {
235237
return;
@@ -238,8 +240,10 @@ unsafe fn do_resolve(
238240
// If the symbol name is greater than MaxNameLen, SymFromAddrW will
239241
// give a buffer of (MaxNameLen - 1) characters and set NameLen to
240242
// the real value.
241-
let name_len = ::core::cmp::min(info.NameLen as usize, info.MaxNameLen as usize - 1);
242-
let name_ptr = info.Name.as_ptr().cast::<u16>();
243+
// SAFETY: We assume NameLen has been initialized by SymFromAddrW, and we initialized MaxNameLen
244+
let name_len = unsafe { cmp::min((*info).NameLen as usize, (*info).MaxNameLen as usize - 1) };
245+
// SAFETY: We assume Name has been initialized by SymFromAddrW
246+
let name_ptr = (&raw const (*info).Name).cast::<u16>();
243247

244248
// Reencode the utf-16 symbol to utf-8 so we can use `SymbolName::new` like
245249
// all other platforms
@@ -296,7 +300,8 @@ unsafe fn do_resolve(
296300
cb(&super::Symbol {
297301
inner: Symbol {
298302
name,
299-
addr: info.Address as *mut _,
303+
// SAFETY: we assume Address has been initialized by SymFromAddrW
304+
addr: unsafe { (*info).Address } as *mut _,
300305
line: lineno,
301306
filename,
302307
_filename_cache: unsafe { cache(filename) },

0 commit comments

Comments
 (0)