Skip to content

Commit

Permalink
Make mmap usage safe/sound
Browse files Browse the repository at this point in the history
For those not aware - mmap is very difficult to use soundly, and relies on other programs in the file system not doing things like truncating the file beneath you. It's also not actually always faster. As such, it's often better to simply use an in-memory buffer.
  • Loading branch information
CraftSpider committed Sep 7, 2024
1 parent c521d9a commit e2a20df
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 189 deletions.
42 changes: 3 additions & 39 deletions src/symbolize/gimli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,15 @@
use self::gimli::read::EndianSlice;
use self::gimli::NativeEndian as Endian;
use self::mmap::Mmap;
use self::stash::Stash;
use super::BytesOrWideString;
use super::ResolveWhat;
use super::SymbolName;
use addr2line::gimli;
use core::convert::TryInto;
use core::mem;
use core::u32;
use libc::c_void;
use mystd::ffi::OsString;
use mystd::fs::File;
use mystd::path::Path;
use mystd::prelude::v1::*;

Expand All @@ -26,41 +23,14 @@ mod mystd {
#[cfg(not(backtrace_in_libstd))]
extern crate std as mystd;

cfg_if::cfg_if! {
if #[cfg(windows)] {
#[path = "gimli/mmap_windows.rs"]
mod mmap;
} else if #[cfg(target_vendor = "apple")] {
#[path = "gimli/mmap_unix.rs"]
mod mmap;
} else if #[cfg(any(
target_os = "android",
target_os = "freebsd",
target_os = "fuchsia",
target_os = "haiku",
target_os = "hurd",
target_os = "linux",
target_os = "openbsd",
target_os = "solaris",
target_os = "illumos",
target_os = "aix",
))] {
#[path = "gimli/mmap_unix.rs"]
mod mmap;
} else {
#[path = "gimli/mmap_fake.rs"]
mod mmap;
}
}

mod stash;

const MAPPINGS_CACHE_SIZE: usize = 4;

struct Mapping {
// 'static lifetime is a lie to hack around lack of support for self-referential structs.
cx: Context<'static>,
_map: Mmap,
_map: Vec<u8>,
stash: Stash,
}

Expand All @@ -74,7 +44,7 @@ impl Mapping {
/// Creates a `Mapping` by ensuring that the `data` specified is used to
/// create a `Context` and it can only borrow from that or the `Stash` of
/// decompressed sections or auxiliary data.
fn mk<F>(data: Mmap, mk: F) -> Option<Mapping>
fn mk<F>(data: Vec<u8>, mk: F) -> Option<Mapping>
where
F: for<'a> FnOnce(&'a [u8], &'a Stash) -> Option<Context<'a>>,
{
Expand All @@ -86,7 +56,7 @@ impl Mapping {

/// Creates a `Mapping` from `data`, or if the closure decides to, returns a
/// different mapping.
fn mk_or_other<F>(data: Mmap, mk: F) -> Option<Mapping>
fn mk_or_other<F>(data: Vec<u8>, mk: F) -> Option<Mapping>
where
F: for<'a> FnOnce(&'a [u8], &'a Stash) -> Option<Either<Mapping, Context<'a>>>,
{
Expand Down Expand Up @@ -184,12 +154,6 @@ impl<'data> Context<'data> {
}
}

fn mmap(path: &Path) -> Option<Mmap> {
let file = File::open(path).ok()?;
let len = file.metadata().ok()?.len().try_into().ok()?;
unsafe { Mmap::map(&file, len) }
}

cfg_if::cfg_if! {
if #[cfg(windows)] {
mod coff;
Expand Down
3 changes: 2 additions & 1 deletion src/symbolize/gimli/coff.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::mystd::fs;
use super::{gimli, Context, Endian, EndianSlice, Mapping, Path, Stash, Vec};
use alloc::sync::Arc;
use core::convert::TryFrom;
Expand All @@ -14,7 +15,7 @@ type Pe = object::pe::ImageNtHeaders64;

impl Mapping {
pub fn new(path: &Path) -> Option<Mapping> {
let map = super::mmap(path)?;
let map = fs::read(path).ok()?;
Mapping::mk(map, |data, stash| {
Context::new(stash, Object::parse(data)?, None, None)
})
Expand Down
10 changes: 5 additions & 5 deletions src/symbolize/gimli/elf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type Elf = object::elf::FileHeader64<NativeEndian>;

impl Mapping {
pub fn new(path: &Path) -> Option<Mapping> {
let map = super::mmap(path)?;
let map = fs::read(path).ok()?;
Mapping::mk_or_other(map, |map, stash| {
let object = Object::parse(&map)?;

Expand All @@ -45,7 +45,7 @@ impl Mapping {

/// Load debuginfo from an external debug file.
fn new_debug(original_path: &Path, path: PathBuf, crc: Option<u32>) -> Option<Mapping> {
let map = super::mmap(&path)?;
let map = fs::read(&path).ok()?;
Mapping::mk(map, |map, stash| {
let object = Object::parse(&map)?;

Expand All @@ -56,7 +56,7 @@ impl Mapping {
// Try to locate a supplementary object file.
let mut sup = None;
if let Some((path_sup, build_id_sup)) = object.gnu_debugaltlink_path(&path) {
if let Some(map_sup) = super::mmap(&path_sup) {
if let Ok(map_sup) = fs::read(&path_sup) {
let map_sup = stash.cache_mmap(map_sup);
if let Some(sup_) = Object::parse(map_sup) {
if sup_.build_id() == Some(build_id_sup) {
Expand Down Expand Up @@ -84,7 +84,7 @@ impl Mapping {
})
.unwrap_or_else(|| "dwp".into());
path_dwp.set_extension(dwp_extension);
if let Some(map_dwp) = super::mmap(&path_dwp) {
if let Ok(map_dwp) = fs::read(&path_dwp) {
let map_dwp = stash.cache_mmap(map_dwp);
if let Some(dwp_) = Object::parse(map_dwp) {
return Some(dwp_);
Expand Down Expand Up @@ -473,7 +473,7 @@ pub(super) fn handle_split_dwarf<'data>(

path.push(convert_path(load.path.as_ref()?).ok()?);

if let Some(map_dwo) = super::mmap(&path) {
if let Ok(map_dwo) = fs::read(&path) {
let map_dwo = stash.cache_mmap(map_dwo);
if let Some(dwo) = Object::parse(map_dwo) {
return gimli::Dwarf::load(|id| -> Result<_, ()> {
Expand Down
5 changes: 3 additions & 2 deletions src/symbolize/gimli/libs_windows.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::super::super::windows_sys::*;
use super::mystd::fs;
use super::mystd::os::windows::prelude::*;
use super::{coff, mmap, Library, LibrarySegment, OsString};
use super::{coff, Library, LibrarySegment, OsString};
use alloc::vec;
use alloc::vec::Vec;
use core::mem;
Expand Down Expand Up @@ -75,7 +76,7 @@ unsafe fn load_library(me: &MODULEENTRY32W) -> Option<Library> {
//
// For now it appears that unlike ELF/MachO we can make do with one
// segment per library, using `modBaseSize` as the whole size.
let mmap = mmap(name.as_ref())?;
let mmap = fs::read(&name).ok()?;
let image_base = coff::get_image_base(&mmap)?;
let base_addr = me.modBaseAddr as usize;
Some(Library {
Expand Down
7 changes: 4 additions & 3 deletions src/symbolize/gimli/macho.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::mystd::fs;
use super::{gimli, Box, Context, Endian, EndianSlice, Mapping, Path, Stash, Vec};
use alloc::sync::Arc;
use core::convert::TryInto;
Expand All @@ -20,7 +21,7 @@ impl Mapping {
pub fn new(path: &Path) -> Option<Mapping> {
// First up we need to load the unique UUID which is stored in the macho
// header of the file we're reading, specified at `path`.
let map = super::mmap(path)?;
let map = fs::read(path).ok()?;
let (macho, data) = find_header(&map)?;
let endian = macho.endian().ok()?;
let uuid = macho.uuid(endian, data, 0).ok()?;
Expand Down Expand Up @@ -74,7 +75,7 @@ impl Mapping {
// information.
for entry in dir.read_dir().ok()? {
let entry = entry.ok()?;
let map = super::mmap(&entry.path())?;
let map = fs::read(&entry.path()).ok()?;
let candidate = Mapping::mk(map, |data, stash| {
let (macho, data) = find_header(data)?;
let endian = macho.endian().ok()?;
Expand Down Expand Up @@ -285,7 +286,7 @@ fn object_mapping(file: &object::read::ObjectMapFile<'_>) -> Option<Mapping> {
use super::mystd::ffi::OsStr;
use super::mystd::os::unix::prelude::*;

let map = super::mmap(Path::new(OsStr::from_bytes(file.path())))?;
let map = fs::read(Path::new(OsStr::from_bytes(file.path()))).ok()?;
let member_name = file.member();
Mapping::mk(map, |data, stash| {
let data = match member_name {
Expand Down
25 changes: 0 additions & 25 deletions src/symbolize/gimli/mmap_fake.rs

This file was deleted.

49 changes: 0 additions & 49 deletions src/symbolize/gimli/mmap_unix.rs

This file was deleted.

59 changes: 0 additions & 59 deletions src/symbolize/gimli/mmap_windows.rs

This file was deleted.

7 changes: 3 additions & 4 deletions src/symbolize/gimli/stash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@
// only used on Linux right now, so allow dead code elsewhere
#![cfg_attr(not(target_os = "linux"), allow(dead_code))]

use super::Mmap;
use alloc::vec;
use alloc::vec::Vec;
use core::cell::UnsafeCell;

/// A simple arena allocator for byte buffers.
pub struct Stash {
buffers: UnsafeCell<Vec<Vec<u8>>>,
mmaps: UnsafeCell<Vec<Mmap>>,
mmaps: UnsafeCell<Vec<Vec<u8>>>,
}

impl Stash {
Expand All @@ -34,9 +33,9 @@ impl Stash {
&mut buffers[i]
}

/// Stores a `Mmap` for the lifetime of this `Stash`, returning a pointer
/// Stores a `Vec<u8>` for the lifetime of this `Stash`, returning a pointer
/// which is scoped to just this lifetime.
pub fn cache_mmap(&self, map: Mmap) -> &[u8] {
pub fn cache_mmap(&self, map: Vec<u8>) -> &[u8] {
// SAFETY: this is the only location for a mutable pointer to
// `mmaps`, and this structure isn't threadsafe to shared across
// threads either. We also never remove elements from `self.mmaps`,
Expand Down
5 changes: 3 additions & 2 deletions src/symbolize/gimli/xcoff.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::mystd::ffi::{OsStr, OsString};
use super::mystd::fs;
use super::mystd::os::unix::ffi::OsStrExt;
use super::mystd::str;
use super::{gimli, Context, Endian, EndianSlice, Mapping, Path, Stash, Vec};
Expand All @@ -18,7 +19,7 @@ type Xcoff = object::xcoff::FileHeader64;

impl Mapping {
pub fn new(path: &Path, member_name: &OsString) -> Option<Mapping> {
let map = super::mmap(path)?;
let map = fs::read(path).ok()?;
Mapping::mk(map, |data, stash| {
if member_name.is_empty() {
Context::new(stash, Object::parse(data)?, None, None)
Expand Down Expand Up @@ -80,7 +81,7 @@ pub fn parse_xcoff(data: &[u8]) -> Option<Image> {
}

pub fn parse_image(path: &Path, member_name: &OsString) -> Option<Image> {
let map = super::mmap(path)?;
let map = fs::read(path).ok()?;
let data = map.deref();
if member_name.is_empty() {
return parse_xcoff(data);
Expand Down

0 comments on commit e2a20df

Please sign in to comment.