I think the following code is safe, but I would appreciate a) a review of it b) suggestions for how to do this without unsafe (or without transmute), and without additional dependencies.

use std::sync::Mutex;

/// Data pool we can borrow from for parsing ELF file sections. Handles that some
/// debug info sections can be compressed.
#[derive(Debug)]
pub struct DataPool {
    /// The underlying memory-mapped file, if the ELF section is uncompressed we
    /// can borrow directly from this.
    file: memmap2::Mmap,
    /// Sections of decompressed data
    ///
    /// # Safety invariant
    /// This is only appended to, which means the inner data lives as long
    /// as DataPool does, and the *inner* contents have stable addresses.
    compressed_sections: Mutex<Vec<Box<[u8]>>>,
}

impl DataPool {
    pub fn new(file: memmap2::Mmap) -> Self {
        Self {
            file,
            compressed_sections: Mutex::new(Vec::new()),
        }
    }

    /// Add a section of compressed data to the pool, returning a reference
    /// to the stored data.
    ///
    /// The returned reference is valid as long as the DataPool lives.
    pub fn add_decompressed_section<'this>(&'this self, data: Box<[u8]>) -> &'this [u8] {
        let mut guard = self.compressed_sections.lock().expect("Mutex poisoned");
        guard.push(data);
        let inner: &[u8] = guard.last().expect("We just pushed...");
        // SAFETY:
        // * By the invariant on `compressed_sections`, the address is stable, so we can
        //   transmute from a temporary life time to 'this.
        // * We return a read only reference to the data (and it has no interior
        //   mutability), so it is thread safe.
        unsafe { transmute_lifetime(inner) }
    }

    /// Get the underlying memory-mapped file data.
    pub fn file_data(&self) -> &[u8] {
        &self.file
    }
}

/// Helper to guarantee we only transmute lifetime of the outer reference, not
/// the type
///
/// # Safety
/// Same as for `std::mem::transmute`, but the type is fixed.
unsafe fn transmute_lifetime<'input, 'output, T: ?Sized>(input: &'input T) -> &'output T {
    unsafe { std::mem::transmute(input) }
}

In another module I'm then using this as:

use ouroboros::self_referencing;

#[self_referencing]
struct BinInfoOwning {
    data: data_pool::DataPool,
    #[borrows(data)]
    #[not_covariant]
    parsed: BinInfo<'this>,
}

PS. I'm aware of the safety requirements of mmap: that the underlying file shouldn't change. That is "handled" elsewhere (i.e. it is up to the user to not use the debugging tool on binaries they are changing at the same time). That is not the topic of discussion here.

12 posts - 4 participants

Read full topic