diff --git a/src/symbolize/gimli.rs b/src/symbolize/gimli.rs index b9f6f3d6..4be6634c 100644 --- a/src/symbolize/gimli.rs +++ b/src/symbolize/gimli.rs @@ -186,7 +186,9 @@ impl<'data> Context<'data> { fn mmap(path: &Path) -> Option { let file = File::open(path).ok()?; let len = file.metadata().ok()?.len().try_into().ok()?; - unsafe { Mmap::map(&file, len, 0) } + // SAFETY: All files we mmap are mmaped by the dynamic linker or the kernel already for the + // executable code of the process. Modifying them would cause crashes or UB anyways. + unsafe { Mmap::map(file, len, 0) } } cfg_if::cfg_if! { diff --git a/src/symbolize/gimli/elf.rs b/src/symbolize/gimli/elf.rs index b73f6aac..441a2295 100644 --- a/src/symbolize/gimli/elf.rs +++ b/src/symbolize/gimli/elf.rs @@ -69,9 +69,9 @@ impl Mapping { let len = file.metadata().ok()?.len(); // NOTE: we map the remainder of the entire archive instead of just the library so we don't have to determine its length - // NOTE: mmap will fail if `zip_offset` is not page-aligned + // SAFETY: See `super::mmap` function let map = unsafe { - super::mmap::Mmap::map(&file, usize::try_from(len - zip_offset).ok()?, zip_offset) + super::mmap::Mmap::map(file, usize::try_from(len - zip_offset).ok()?, zip_offset) }?; Mapping::mk(map, |map, stash| { diff --git a/src/symbolize/gimli/mmap_fake.rs b/src/symbolize/gimli/mmap_fake.rs index 71697fc3..5659c16f 100644 --- a/src/symbolize/gimli/mmap_fake.rs +++ b/src/symbolize/gimli/mmap_fake.rs @@ -8,7 +8,11 @@ pub struct Mmap { } impl Mmap { - pub unsafe fn map(mut file: &File, len: usize, offset: u64) -> Option { + /// A fake `mmap` implementation for platforms with no native file mapping support. + /// + /// # Safety + /// This function is always safe to call. + pub unsafe fn map(mut file: File, len: usize, offset: u64) -> Option { let mut mmap = Mmap { vec: Vec::with_capacity(len), }; diff --git a/src/symbolize/gimli/mmap_unix.rs b/src/symbolize/gimli/mmap_unix.rs index 0895ee5d..0c35cb46 100644 --- a/src/symbolize/gimli/mmap_unix.rs +++ b/src/symbolize/gimli/mmap_unix.rs @@ -15,7 +15,14 @@ pub struct Mmap { } impl Mmap { - pub unsafe fn map(file: &File, len: usize, offset: u64) -> Option { + /// Map a file into memory, returning `None` on failure. `offset` must be a multiple of the page + /// size, or mapping will fail[^1]. + /// + /// # Safety + /// - Mapped files must not be altered for the lifetime of the returned value. + /// + /// [^1]: https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/functions/mmap.html + pub unsafe fn map(file: File, len: usize, offset: u64) -> Option { let ptr = mmap64( ptr::null_mut(), len, diff --git a/src/symbolize/gimli/mmap_windows.rs b/src/symbolize/gimli/mmap_windows.rs index 1c8bc83c..fd5b1e74 100644 --- a/src/symbolize/gimli/mmap_windows.rs +++ b/src/symbolize/gimli/mmap_windows.rs @@ -16,8 +16,14 @@ pub struct Mmap { } impl Mmap { - pub unsafe fn map(file: &File, len: usize, offset: u64) -> Option { - let file = file.try_clone().ok()?; + /// Map a file into memory, returning `None` on failure. `offset` must be a multiple of + /// `dwAllocationGranularity` size, or mapping will fail[^1]. + /// + /// # Safety + /// - Mapped files must not be altered for the lifetime of the returned value.' + /// + /// [^1]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-mapviewoffile + pub unsafe fn map(file: File, len: usize, offset: u64) -> Option { let mapping = CreateFileMappingA( file.as_raw_handle(), ptr::null_mut(),