mmap: drop file-offset checking, defer to mmap(2) instead

CHANGELOG.md

@@ -17,6 +17,9 @@
   `Bytes::write_to`, `Bytes::write_all_to`, `GuestMemory::as_slice`, `GuestMemory::as_slice_mut`, `GuestMemory::with_regions`, 
   `GuestMemory::with_regions_mut`, `GuestMemory::map_and_fold`, `VolatileSlice::as_ptr`, `VolatileRef::as_ptr`, and
   `VolatileArrayRef::as_ptr`.
+- \[[#320](https://github.com/rust-vmm/vm-memory/pull/320)\] Drop `check_file_offset` check when using  `MmapRegionBuilder`.
+  The `mmap(2)` syscall itself already validates that offset and length are valid, and trying to replicate this check
+  in userspace ended up being imperfect.
 
 ## \[v0.16.1\]
 

src/mmap/mod.rs

@@ -13,8 +13,6 @@
 //! This implementation is mmap-ing the memory of the guest into the current process.
 
 use std::borrow::Borrow;
-#[cfg(target_family = "unix")]
-use std::io::{Seek, SeekFrom};
 use std::ops::Deref;
 use std::result;
 use std::sync::atomic::Ordering;
@@ -72,34 +70,6 @@ pub enum Error {
     UnsortedMemoryRegions,
 }
 
-// TODO: use this for Windows as well after we redefine the Error type there.
-#[cfg(target_family = "unix")]
-/// Checks if a mapping of `size` bytes fits at the provided `file_offset`.
-///
-/// For a borrowed `FileOffset` and size, this function checks whether the mapping does not
-/// extend past EOF, and that adding the size to the file offset does not lead to overflow.
-pub fn check_file_offset(
-    file_offset: &FileOffset,
-    size: usize,
-) -> result::Result<(), MmapRegionError> {
-    let mut file = file_offset.file();
-    let start = file_offset.start();
-
-    if let Some(end) = start.checked_add(size as u64) {
-        let filesize = file
-            .seek(SeekFrom::End(0))
-            .map_err(MmapRegionError::SeekEnd)?;
-        file.rewind().map_err(MmapRegionError::SeekStart)?;
-        if filesize < end {
-            return Err(MmapRegionError::MappingPastEof);
-        }
-    } else {
-        return Err(MmapRegionError::InvalidOffsetLength);
-    }
-
-    Ok(())
-}
-
 /// [`GuestMemoryRegion`](trait.GuestMemoryRegion.html) implementation that mmaps the guest's
 /// memory region in the current process.
 ///

src/mmap/unix.rs

@@ -17,7 +17,6 @@ use std::result;
 
 use crate::bitmap::{Bitmap, NewBitmap, BS};
 use crate::guest_memory::FileOffset;
-use crate::mmap::check_file_offset;
 use crate::volatile_memory::{self, VolatileMemory, VolatileSlice};
 
 /// Error conditions that may arise when creating a new `MmapRegion` object.
@@ -41,12 +40,6 @@ pub enum Error {
     /// The `mmap` call returned an error.
     #[error("{0}")]
     Mmap(io::Error),
-    /// Seeking the end of the file returned an error.
-    #[error("Error seeking the end of the file: {0}")]
-    SeekEnd(io::Error),
-    /// Seeking the start of the file returned an error.
-    #[error("Error seeking the start of the file: {0}")]
-    SeekStart(io::Error),
 }
 
 pub type Result<T> = result::Result<T, Error>;
@@ -137,7 +130,6 @@ impl<B: Bitmap> MmapRegionBuilder<B> {
         }
 
         let (fd, offset) = if let Some(ref f_off) = self.file_offset {
-            check_file_offset(f_off, self.size)?;
             (f_off.file().as_raw_fd(), f_off.start())
         } else {
             (-1, 0)
@@ -558,16 +550,9 @@ mod tests {
             prot,
             flags,
         );
-        assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength");
-
-        // Offset + size is greater than the size of the file (which is 0 at this point).
-        let r = MmapRegion::build(
-            Some(FileOffset::from_arc(a.clone(), offset)),
-            size,
-            prot,
-            flags,
+        assert!(
+            matches!(r.unwrap_err(), Error::Mmap(err) if err.raw_os_error() == Some(libc::EINVAL))
         );
-        assert_eq!(format!("{:?}", r.unwrap_err()), "MappingPastEof");
 
         // MAP_FIXED was specified among the flags.
         let r = MmapRegion::build(

src/mmap/xen.rs

@@ -26,7 +26,6 @@ use tests::ioctl_with_ref;
 
 use crate::bitmap::{Bitmap, NewBitmap, BS};
 use crate::guest_memory::{FileOffset, GuestAddress};
-use crate::mmap::check_file_offset;
 use crate::volatile_memory::{self, VolatileMemory, VolatileSlice};
 
 /// Error conditions that may arise when creating a new `MmapRegion` object.
@@ -44,12 +43,6 @@ pub enum Error {
     /// The `mmap` call returned an error.
     #[error("{0}")]
     Mmap(io::Error),
-    /// Seeking the end of the file returned an error.
-    #[error("Error seeking the end of the file: {0}")]
-    SeekEnd(io::Error),
-    /// Seeking the start of the file returned an error.
-    #[error("Error seeking the start of the file: {0}")]
-    SeekStart(io::Error),
     /// Invalid file offset.
     #[error("Invalid file offset")]
     InvalidFileOffset,
@@ -524,7 +517,6 @@ struct MmapXenUnix(MmapUnix);
 impl MmapXenUnix {
     fn new(range: &MmapRange) -> Result<Self> {
         let (fd, offset) = if let Some(ref f_off) = range.file_offset {
-            check_file_offset(f_off, range.size)?;
             (f_off.file().as_raw_fd(), f_off.start())
         } else {
             (-1, 0)