Mark Mmap::map as safe

Author: oli-obk

compiler/rustc_codegen_llvm/src/back/lto.rs

@@ -113,7 +113,7 @@ fn prepare_lto(
                     .extend(exported_symbols[&cnum].iter().filter_map(symbol_filter));
             }
 
-            let archive_data = unsafe { Mmap::map(&path).expect("couldn't map rlib") };
+            let archive_data = Mmap::map(&path).expect("couldn't map rlib");
             let archive = ArchiveFile::parse(&*archive_data).expect("wanted an rlib");
             let obj_files = archive
                 .members()

compiler/rustc_codegen_ssa/src/back/archive.rs

@@ -137,10 +137,8 @@ pub trait ArchiveBuilderBuilder {
         outdir: &Path,
         bundled_lib_file_names: &FxIndexSet<Symbol>,
     ) -> Result<(), ExtractBundledLibsError<'a>> {
-        let archive_map = unsafe {
-            Mmap::map(rlib)
-                .map_err(|e| ExtractBundledLibsError::MmapFile { rlib, error: Box::new(e) })?
-        };
+        let archive_map = Mmap::map(rlib)
+            .map_err(|e| ExtractBundledLibsError::MmapFile { rlib, error: Box::new(e) })?;
         let archive = ArchiveFile::parse(&*archive_map)
             .map_err(|e| ExtractBundledLibsError::ParseArchive { rlib, error: Box::new(e) })?;
 
@@ -360,7 +358,7 @@ pub fn try_extract_macho_fat_archive(
     sess: &Session,
     archive_path: &Path,
 ) -> io::Result<Option<PathBuf>> {
-    let archive_map = unsafe { Mmap::map(&archive_path)? };
+    let archive_map = Mmap::map(&archive_path)?;
     let target_arch = match sess.target.arch.as_ref() {
         "aarch64" => object::Architecture::Aarch64,
         "x86_64" => object::Architecture::X86_64,
@@ -397,7 +395,7 @@ impl<'a> ArchiveBuilder for ArArchiveBuilder<'a> {
             return Ok(());
         }
 
-        let archive_map = unsafe { Mmap::map(&archive_path)? };
+        let archive_map = Mmap::map(&archive_path)?;
         let archive = ArchiveFile::parse(&*archive_map)
             .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
         let archive_index = self.src_archives.len();
@@ -469,12 +467,10 @@ impl<'a> ArArchiveBuilder<'a> {
 
                     Box::new(data) as Box<dyn AsRef<[u8]>>
                 }
-                ArchiveEntry::File(file) => unsafe {
-                    Box::new(
-                        Mmap::map(file)
-                            .map_err(|err| io_error_context("failed to map object file", err))?,
-                    ) as Box<dyn AsRef<[u8]>>
-                },
+                ArchiveEntry::File(file) => Box::new(
+                    Mmap::map(file)
+                        .map_err(|err| io_error_context("failed to map object file", err))?,
+                ) as Box<dyn AsRef<[u8]>>,
             };
 
             entries.push(NewArchiveMember {

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -578,7 +578,7 @@ fn link_dwarf_object(sess: &Session, cg_results: &CodegenResults, executable_out
         }
 
         fn read_input(&self, path: &Path) -> std::io::Result<&[u8]> {
-            let mmap = (unsafe { Mmap::map(&path) })?;
+            let mmap = Mmap::map(&path)?;
             Ok(self.alloc_mmap(mmap))
         }
     }

compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -43,7 +43,7 @@ fn load_metadata_with(
     path: &Path,
     f: impl for<'a> FnOnce(&'a [u8]) -> Result<&'a [u8], String>,
 ) -> Result<OwnedSlice, String> {
-    unsafe { Mmap::map(&path) }
+    Mmap::map(&path)
         .map_err(|e| format!("failed to mmap file '{}': {}", path.display(), e))
         .and_then(|mmap| try_slice_owned(mmap, |mmap| f(mmap)))
 }

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -2152,11 +2152,8 @@ pub(crate) fn submit_pre_lto_module_to_llvm<B: ExtraBackendMethods>(
     let filename = pre_lto_bitcode_filename(&module.name);
     let bc_path = in_incr_comp_dir_sess(tcx.sess, &filename);
 
-    let mmap = unsafe {
-        Mmap::map(&bc_path).unwrap_or_else(|e| {
-            panic!("failed to mmap bitcode file `{}`: {}", bc_path.display(), e)
-        })
-    };
+    let mmap = Mmap::map(&bc_path)
+        .unwrap_or_else(|e| panic!("failed to mmap bitcode file `{}`: {}", bc_path.display(), e));
     // Schedule the module to be loaded
     drop(tx_to_llvm_workers.send(Box::new(Message::AddImportOnlyModule::<B> {
         module_data: SerializedModule::FromUncompressedFile(mmap),

compiler/rustc_data_structures/src/memmap.rs

@@ -12,14 +12,32 @@ pub struct Mmap(Vec<u8>);
 
 #[cfg(not(any(miri, target_arch = "wasm32")))]
 impl Mmap {
-    /// # Safety
-    ///
     /// The given file must not be mutated (i.e., not written, not truncated, ...) until the mapping is closed.
     ///
-    /// However in practice most callers do not ensure this, so uses of this function are likely unsound.
+    /// This process must not modify nor remove the backing file while the memory map lives.
+    /// For the dep-graph and the work product index, it is as soon as the decoding is done.
+    /// For the query result cache, the memory map is dropped in save_dep_graph before calling
+    /// save_in and trying to remove the backing file.
+    ///
+    /// There is no way to prevent another process from modifying this file.
+    ///
+    /// This means in practice all uses of this function are theoretically unsound, but also
+    /// the way rustc uses `Mmap` (reading bytes, validating them afterwards *anyway* to detect
+    /// corrupted files) avoids the actual issues this could cause.
+    ///
+    /// Someone may truncate our file, but then we'll SIGBUS, which is not great, but at least
+    /// we won't succeed with corrupted data.
+    ///
+    /// To get a bit more hardening out of this we will set the file as readonly before opening it.
     #[inline]
-    pub unsafe fn map(path: impl AsRef<Path>) -> io::Result<Self> {
+    pub fn map(path: impl AsRef<Path>) -> io::Result<Self> {
+        let path = path.as_ref();
+        let mut perms = std::fs::metadata(path)?.permissions();
+        perms.set_readonly(true);
+        std::fs::set_permissions(path, perms)?;
+
         let file = File::open(path)?;
+
         // By default, memmap2 creates shared mappings, implying that we could see updates to the
         // file through the mapping. That would violate our precondition; so by requesting a
         // map_copy_read_only we do not lose anything.
@@ -34,7 +52,7 @@ impl Mmap {
 #[cfg(any(miri, target_arch = "wasm32"))]
 impl Mmap {
     #[inline]
-    pub unsafe fn map(path: impl AsRef<Path>) -> io::Result<Self> {
+    pub fn map(path: impl AsRef<Path>) -> io::Result<Self> {
         Ok(Mmap(std::fs::read(path)?))
     }
 }

compiler/rustc_incremental/src/persist/file_format.rs

@@ -95,13 +95,7 @@ pub(crate) fn read_file(
     is_nightly_build: bool,
     cfg_version: &'static str,
 ) -> io::Result<Option<(Mmap, usize)>> {
-    // SAFETY: This process must not modify nor remove the backing file while the memory map lives.
-    // For the dep-graph and the work product index, it is as soon as the decoding is done.
-    // For the query result cache, the memory map is dropped in save_dep_graph before calling
-    // save_in and trying to remove the backing file.
-    //
-    // There is no way to prevent another process from modifying this file.
-    let mmap = match unsafe { Mmap::map(path) } {
+    let mmap = match Mmap::map(path) {
         Ok(file) => file,
         Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
         Err(err) => return Err(err),