summary refs log tree commit diff
diff options
context:
space:
mode:
authorDylan Reid <dgreid@chromium.org>2020-03-27 19:07:59 +0000
committerCommit Bot <commit-bot@chromium.org>2020-04-05 21:32:17 +0000
commit252d5b3cf3fd7a48fe9d610b59e3d6da9f2c6fe9 (patch)
treedc25f8bcc4d2923222a7552b999aede2ec321855
parent146450b4569e86657d1d8c4ffe17524781aae7e3 (diff)
downloadcrosvm-252d5b3cf3fd7a48fe9d610b59e3d6da9f2c6fe9.tar
crosvm-252d5b3cf3fd7a48fe9d610b59e3d6da9f2c6fe9.tar.gz
crosvm-252d5b3cf3fd7a48fe9d610b59e3d6da9f2c6fe9.tar.bz2
crosvm-252d5b3cf3fd7a48fe9d610b59e3d6da9f2c6fe9.tar.lz
crosvm-252d5b3cf3fd7a48fe9d610b59e3d6da9f2c6fe9.tar.xz
crosvm-252d5b3cf3fd7a48fe9d610b59e3d6da9f2c6fe9.tar.zst
crosvm-252d5b3cf3fd7a48fe9d610b59e3d6da9f2c6fe9.zip
handle mmap of large offsets on 32 bit systems
While only 32 bits of address can be mapped, that 32 bits can be offset
by further than 32 bits in to a large file. As chirantan points out, the
try_mmap call was already casting the usize to u64 on all architectures.

Convert the usize offset in mmap to u64 and address users of the API as
well.

Change-Id: I67aed928ea521049fb51eb7aa61ea4de8b4d096c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2124879
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Tested-by: Dylan Reid <dgreid@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Dylan Reid <dgreid@chromium.org>
-rw-r--r--devices/src/pci/ac97_bus_master.rs10
-rw-r--r--devices/src/pci/vfio_pci.rs2
-rw-r--r--src/plugin/process.rs2
-rw-r--r--sys_util/src/guest_memory.rs10
-rw-r--r--sys_util/src/mmap.rs22
-rw-r--r--vm_control/src/lib.rs4
6 files changed, 28 insertions, 22 deletions
diff --git a/devices/src/pci/ac97_bus_master.rs b/devices/src/pci/ac97_bus_master.rs
index 5f4ca75..809f31f 100644
--- a/devices/src/pci/ac97_bus_master.rs
+++ b/devices/src/pci/ac97_bus_master.rs
@@ -5,6 +5,7 @@
 use std;
 use std::collections::VecDeque;
 use std::convert::AsRef;
+use std::convert::TryInto;
 use std::error::Error;
 use std::fmt::{self, Display};
 use std::os::unix::io::{AsRawFd, RawFd};
@@ -106,6 +107,8 @@ type GuestMemoryResult<T> = std::result::Result<T, GuestMemoryError>;
 enum AudioError {
     // Failed to create a new stream.
     CreateStream(Box<dyn Error>),
+    // Invalid buffer offset received from the audio server.
+    InvalidBufferOffset,
     // Guest did not provide a buffer when needed.
     NoBufferAvailable,
     // Failure to read guest memory.
@@ -124,6 +127,7 @@ impl Display for AudioError {
 
         match self {
             CreateStream(e) => write!(f, "Failed to create audio stream: {}.", e),
+            InvalidBufferOffset => write!(f, "Offset > max usize"),
             NoBufferAvailable => write!(f, "No buffer was available from the Guest"),
             ReadingGuestError(e) => write!(f, "Failed to read guest memory: {}.", e),
             RespondRequest(e) => write!(f, "Failed to respond to the ServerRequest: {}", e),
@@ -610,7 +614,7 @@ fn get_buffer_offset(
     func_regs: &Ac97FunctionRegs,
     mem: &GuestMemory,
     index: u8,
-) -> GuestMemoryResult<usize> {
+) -> GuestMemoryResult<u64> {
     let descriptor_addr = func_regs.bdbar + u32::from(index) * DESCRIPTOR_LENGTH as u32;
     let buffer_addr_reg: u32 = mem
         .read_obj_from_addr(GuestAddress(u64::from(descriptor_addr)))
@@ -673,7 +677,9 @@ fn next_guest_buffer<'a>(
         return Ok(None);
     }
 
-    let offset = get_buffer_offset(func_regs, mem, index)?;
+    let offset = get_buffer_offset(func_regs, mem, index)?
+        .try_into()
+        .map_err(|_| AudioError::InvalidBufferOffset)?;
     let frames = get_buffer_samples(func_regs, mem, index)? / DEVICE_CHANNEL_COUNT;
 
     Ok(Some(GuestBuffer {
diff --git a/devices/src/pci/vfio_pci.rs b/devices/src/pci/vfio_pci.rs
index 4d5ad9e..d1b6f2b 100644
--- a/devices/src/pci/vfio_pci.rs
+++ b/devices/src/pci/vfio_pci.rs
@@ -709,7 +709,7 @@ impl VfioPciDevice {
                 let mmap_size = mmap.size;
                 let guest_map_start = bar_addr + mmap_offset;
                 let region_offset = self.device.get_region_offset(index);
-                let offset: usize = (region_offset + mmap_offset) as usize;
+                let offset = region_offset + mmap_offset;
                 if self
                     .vm_socket_mem
                     .send(&VmMemoryRequest::RegisterMmapMemory {
diff --git a/src/plugin/process.rs b/src/plugin/process.rs
index ea7a78c..51fc892 100644
--- a/src/plugin/process.rs
+++ b/src/plugin/process.rs
@@ -361,7 +361,7 @@ impl Process {
             None => return Err(SysError::new(EOVERFLOW)),
             _ => {}
         }
-        let mem = MemoryMapping::from_fd_offset(&shm, length as usize, offset as usize)
+        let mem = MemoryMapping::from_fd_offset(&shm, length as usize, offset)
             .map_err(mmap_to_sys_err)?;
         let slot = vm.add_mmio_memory(GuestAddress(start), mem, read_only, dirty_log)?;
         entry.insert(PluginObject::Memory {
diff --git a/sys_util/src/guest_memory.rs b/sys_util/src/guest_memory.rs
index 2390b92..e8f620b 100644
--- a/sys_util/src/guest_memory.rs
+++ b/sys_util/src/guest_memory.rs
@@ -84,7 +84,7 @@ impl Display for Error {
 struct MemoryRegion {
     mapping: MemoryMapping,
     guest_base: GuestAddress,
-    memfd_offset: usize,
+    memfd_offset: u64,
 }
 
 fn region_end(region: &MemoryRegion) -> GuestAddress {
@@ -175,7 +175,7 @@ impl GuestMemory {
                 memfd_offset: offset,
             });
 
-            offset += size;
+            offset += size as u64;
         }
 
         Ok(GuestMemory {
@@ -262,7 +262,7 @@ impl GuestMemory {
     ///  * memfd_offset: usize
     pub fn with_regions<F, E>(&self, mut cb: F) -> result::Result<(), E>
     where
-        F: FnMut(usize, GuestAddress, usize, usize, usize) -> result::Result<(), E>,
+        F: FnMut(usize, GuestAddress, usize, usize, u64) -> result::Result<(), E>,
     {
         for (index, region) in self.regions.iter().enumerate() {
             cb(
@@ -584,10 +584,10 @@ impl GuestMemory {
     ///                .expect("failed to get offset");
     /// assert_eq!(offset, 0x3500);
     /// ```
-    pub fn offset_from_base(&self, guest_addr: GuestAddress) -> Result<usize> {
+    pub fn offset_from_base(&self, guest_addr: GuestAddress) -> Result<u64> {
         for region in self.regions.iter() {
             if guest_addr >= region.guest_base && guest_addr < region_end(region) {
-                return Ok(region.memfd_offset + guest_addr.offset_from(region.guest_base) as usize);
+                return Ok(region.memfd_offset + guest_addr.offset_from(region.guest_base) as u64);
             }
         }
         Err(Error::InvalidGuestAddress(guest_addr))
diff --git a/sys_util/src/mmap.rs b/sys_util/src/mmap.rs
index d8ba6b6..006b0a8 100644
--- a/sys_util/src/mmap.rs
+++ b/sys_util/src/mmap.rs
@@ -164,7 +164,7 @@ impl MemoryMapping {
         MemoryMapping::from_fd_offset(fd, size, 0)
     }
 
-    pub fn from_fd_offset(fd: &dyn AsRawFd, size: usize, offset: usize) -> Result<MemoryMapping> {
+    pub fn from_fd_offset(fd: &dyn AsRawFd, size: usize, offset: u64) -> Result<MemoryMapping> {
         MemoryMapping::from_fd_offset_protection(fd, size, offset, Protection::read_write())
     }
 
@@ -177,7 +177,7 @@ impl MemoryMapping {
     pub fn from_fd_offset_populate(
         fd: &dyn AsRawFd,
         size: usize,
-        offset: usize,
+        offset: u64,
     ) -> Result<MemoryMapping> {
         MemoryMapping::from_fd_offset_flags(
             fd,
@@ -199,7 +199,7 @@ impl MemoryMapping {
     fn from_fd_offset_flags(
         fd: &dyn AsRawFd,
         size: usize,
-        offset: usize,
+        offset: u64,
         flags: c_int,
         prot: Protection,
     ) -> Result<MemoryMapping> {
@@ -220,7 +220,7 @@ impl MemoryMapping {
     pub fn from_fd_offset_protection(
         fd: &dyn AsRawFd,
         size: usize,
-        offset: usize,
+        offset: u64,
         prot: Protection,
     ) -> Result<MemoryMapping> {
         MemoryMapping::from_fd_offset_flags(fd, size, offset, libc::MAP_SHARED, prot)
@@ -261,7 +261,7 @@ impl MemoryMapping {
         addr: *mut u8,
         fd: &dyn AsRawFd,
         size: usize,
-        offset: usize,
+        offset: u64,
         prot: Protection,
     ) -> Result<MemoryMapping> {
         MemoryMapping::try_mmap(
@@ -280,7 +280,7 @@ impl MemoryMapping {
         size: usize,
         prot: c_int,
         flags: c_int,
-        fd: Option<(&dyn AsRawFd, usize)>,
+        fd: Option<(&dyn AsRawFd, u64)>,
     ) -> Result<MemoryMapping> {
         let mut flags = flags;
         // If addr is provided, set the FIXED flag, and validate addr alignment
@@ -297,7 +297,7 @@ impl MemoryMapping {
         // If fd is provided, validate fd offset is within bounds
         let (fd, offset) = match fd {
             Some((fd, offset)) => {
-                if offset > libc::off_t::max_value() as usize {
+                if offset > libc::off_t::max_value() as u64 {
                     return Err(Error::InvalidOffset);
                 }
                 (fd.as_raw_fd(), offset as libc::off_t)
@@ -714,7 +714,7 @@ impl MemoryMappingArena {
         offset: usize,
         size: usize,
         fd: &dyn AsRawFd,
-        fd_offset: usize,
+        fd_offset: u64,
     ) -> Result<()> {
         self.add_fd_offset_protection(offset, size, fd, fd_offset, Protection::read_write())
     }
@@ -734,7 +734,7 @@ impl MemoryMappingArena {
         offset: usize,
         size: usize,
         fd: &dyn AsRawFd,
-        fd_offset: usize,
+        fd_offset: u64,
         prot: Protection,
     ) -> Result<()> {
         self.try_add(offset, size, prot, Some((fd, fd_offset)))
@@ -747,7 +747,7 @@ impl MemoryMappingArena {
         offset: usize,
         size: usize,
         prot: Protection,
-        fd: Option<(&dyn AsRawFd, usize)>,
+        fd: Option<(&dyn AsRawFd, u64)>,
     ) -> Result<()> {
         self.validate_range(offset, size)?;
 
@@ -953,7 +953,7 @@ mod tests {
     #[test]
     fn from_fd_offset_invalid() {
         let fd = unsafe { std::fs::File::from_raw_fd(-1) };
-        let res = MemoryMapping::from_fd_offset(&fd, 4096, (libc::off_t::max_value() as usize) + 1)
+        let res = MemoryMapping::from_fd_offset(&fd, 4096, (libc::off_t::max_value() as u64) + 1)
             .unwrap_err();
         match res {
             Error::InvalidOffset => {}
diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs
index cb836be..4d48ff4 100644
--- a/vm_control/src/lib.rs
+++ b/vm_control/src/lib.rs
@@ -235,7 +235,7 @@ pub enum VmMemoryRequest {
     RegisterMmapMemory {
         fd: MaybeOwnedFd,
         size: usize,
-        offset: usize,
+        offset: u64,
         gpa: u64,
     },
 }
@@ -303,7 +303,7 @@ impl VmMemoryRequest {
                 offset,
                 gpa,
             } => {
-                let mmap = match MemoryMapping::from_fd_offset(fd, size, offset) {
+                let mmap = match MemoryMapping::from_fd_offset(fd, size, offset as u64) {
                     Ok(v) => v,
                     Err(_e) => return VmMemoryResponse::Err(SysError::new(EINVAL)),
                 };