diff --git a/Cargo.lock b/Cargo.lock index 33c3257837..d9955a123e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -76,6 +76,7 @@ dependencies = [ name = "base" version = "0.1.0" dependencies = [ + "data_model 0.1.0", "sys_util 0.1.0", ] @@ -166,6 +167,7 @@ dependencies = [ "remain 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "resources 0.1.0", "sync 0.1.0", + "tempfile 3.0.7", "vhost 0.1.0", "vm_control 0.1.0", "vm_memory 0.1.0", @@ -249,6 +251,7 @@ dependencies = [ "protobuf 2.8.1 (registry+https://github.com/rust-lang/crates.io-index)", "protos 0.1.0", "remain 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "tempfile 3.0.7", "vm_memory 0.1.0", ] @@ -418,6 +421,7 @@ version = "0.1.0" dependencies = [ "base 0.1.0", "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", + "tempfile 3.0.7", "vm_memory 0.1.0", ] diff --git a/Cargo.toml b/Cargo.toml index a7d1c12106..4bf4097cc7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,6 +75,7 @@ rand_ish = { path = "rand_ish" } remain = "*" resources = { path = "resources" } sync = { path = "sync" } +tempfile = "*" vhost = { path = "vhost" } vm_control = { path = "vm_control" } acpi_tables = { path = "acpi_tables" } diff --git a/arch/src/lib.rs b/arch/src/lib.rs index 8890b98f70..7b0072a2fc 100644 --- a/arch/src/lib.rs +++ b/arch/src/lib.rs @@ -17,7 +17,7 @@ use std::path::PathBuf; use std::sync::Arc; use acpi_tables::sdt::SDT; -use base::{syslog, EventFd}; +use base::{syslog, AsRawDescriptor, EventFd}; use devices::virtio::VirtioDevice; use devices::{ Bus, BusDevice, BusError, IrqChip, PciAddress, PciDevice, PciDeviceError, PciInterruptPin, @@ -373,7 +373,7 @@ pub fn load_image( max_size: u64, ) -> Result where - F: Read + Seek + AsRawFd, + F: Read + Seek + AsRawDescriptor, { let size = image.seek(SeekFrom::End(0)).map_err(LoadImageError::Seek)?; @@ -415,7 +415,7 @@ pub fn load_image_high( align: u64, ) -> Result<(GuestAddress, usize), LoadImageError> where - F: Read + Seek + AsRawFd, + F: Read + Seek + AsRawDescriptor, { if !align.is_power_of_two() { return Err(LoadImageError::BadAlignment(align)); diff --git a/arch/src/pstore.rs b/arch/src/pstore.rs index c76f2131a0..6f41993bba 100644 --- a/arch/src/pstore.rs +++ b/arch/src/pstore.rs @@ -63,7 +63,7 @@ pub fn create_memory_region( .map_err(Error::ResourcesError)?; let memory_mapping = - MemoryMapping::from_fd(&file, pstore.size as usize).map_err(Error::MmapError)?; + MemoryMapping::from_descriptor(&file, pstore.size as usize).map_err(Error::MmapError)?; vm.add_memory_region( GuestAddress(address), diff --git a/base/Cargo.toml b/base/Cargo.toml index fb3f1a1fd9..b3bb63cc99 100644 --- a/base/Cargo.toml +++ b/base/Cargo.toml @@ -8,4 +8,5 @@ edition = "2018" chromeos = ["sys_util/chromeos"] [dependencies] +data_model = { path = "../data_model" } sys_util = { path = "../sys_util" } diff --git a/base/src/lib.rs b/base/src/lib.rs index 49eb33a370..4a8e2af06a 100644 --- a/base/src/lib.rs +++ b/base/src/lib.rs @@ -3,3 +3,15 @@ // found in the LICENSE file. pub use sys_util::*; + +mod mmap; +mod shm; + +pub use mmap::MemoryMapping; +pub use shm::{SharedMemory, Unix as SharedMemoryUnix}; + +/// Wraps an AsRawDescriptor in the simple Descriptor struct, which +/// has AsRawFd methods for interfacing with sys_util +fn wrap_descriptor(descriptor: &dyn AsRawDescriptor) -> Descriptor { + Descriptor(descriptor.as_raw_descriptor()) +} diff --git a/base/src/mmap.rs b/base/src/mmap.rs new file mode 100644 index 0000000000..3a8979b72b --- /dev/null +++ b/base/src/mmap.rs @@ -0,0 +1,142 @@ +// Copyright 2020 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +use crate::{wrap_descriptor, AsRawDescriptor, MappedRegion, MmapError, Protection}; +use data_model::volatile_memory::*; +use data_model::DataInit; +use sys_util::MemoryMapping as SysUtilMmap; + +pub type Result = std::result::Result; + +/// See [MemoryMapping](sys_util::MemoryMapping) for struct- and method-level +/// documentation. +#[derive(Debug)] +pub struct MemoryMapping(SysUtilMmap); +impl MemoryMapping { + pub fn new(size: usize) -> Result { + SysUtilMmap::new(size).map(|mmap| MemoryMapping(mmap)) + } + + pub fn from_descriptor(descriptor: &dyn AsRawDescriptor, size: usize) -> Result { + SysUtilMmap::from_fd(&wrap_descriptor(descriptor), size).map(|mmap| MemoryMapping(mmap)) + } + + pub fn from_descriptor_offset( + descriptor: &dyn AsRawDescriptor, + size: usize, + offset: u64, + ) -> Result { + SysUtilMmap::from_fd_offset(&wrap_descriptor(descriptor), size, offset) + .map(|mmap| MemoryMapping(mmap)) + } + + pub fn new_protection(size: usize, prot: Protection) -> Result { + SysUtilMmap::new_protection(size, prot).map(|mmap| MemoryMapping(mmap)) + } + + pub fn from_descriptor_offset_protection( + descriptor: &dyn AsRawDescriptor, + size: usize, + offset: u64, + prot: Protection, + ) -> Result { + SysUtilMmap::from_fd_offset_protection(&wrap_descriptor(descriptor), size, offset, prot) + .map(|mmap| MemoryMapping(mmap)) + } + + pub unsafe fn new_protection_fixed( + addr: *mut u8, + size: usize, + prot: Protection, + ) -> Result { + SysUtilMmap::new_protection_fixed(addr, size, prot).map(|mmap| MemoryMapping(mmap)) + } + + pub unsafe fn from_descriptor_offset_protection_fixed( + addr: *mut u8, + descriptor: &dyn AsRawDescriptor, + size: usize, + offset: u64, + prot: Protection, + ) -> Result { + SysUtilMmap::from_fd_offset_protection_fixed( + addr, + &wrap_descriptor(descriptor), + size, + offset, + prot, + ) + .map(|mmap| MemoryMapping(mmap)) + } + + pub fn from_descriptor_offset_populate( + descriptor: &dyn AsRawDescriptor, + size: usize, + offset: u64, + ) -> Result { + SysUtilMmap::from_fd_offset_populate(&wrap_descriptor(descriptor), size, offset) + .map(|mmap| MemoryMapping(mmap)) + } + + pub fn write_slice(&self, buf: &[u8], offset: usize) -> Result { + self.0.write_slice(buf, offset) + } + + pub fn read_slice(&self, buf: &mut [u8], offset: usize) -> Result { + self.0.read_slice(buf, offset) + } + + pub fn write_obj(&self, val: T, offset: usize) -> Result<()> { + self.0.write_obj(val, offset) + } + + pub fn read_obj(&self, offset: usize) -> Result { + self.0.read_obj(offset) + } + + pub fn msync(&self) -> Result<()> { + self.0.msync() + } + + pub fn read_to_memory( + &self, + mem_offset: usize, + src: &dyn AsRawDescriptor, + count: usize, + ) -> Result<()> { + self.0 + .read_to_memory(mem_offset, &wrap_descriptor(src), count) + } + + pub fn write_from_memory( + &self, + mem_offset: usize, + dst: &dyn AsRawDescriptor, + count: usize, + ) -> Result<()> { + self.0 + .write_from_memory(mem_offset, &wrap_descriptor(dst), count) + } + + pub fn remove_range(&self, mem_offset: usize, count: usize) -> Result<()> { + self.0.remove_range(mem_offset, count) + } +} + +impl VolatileMemory for MemoryMapping { + fn get_slice(&self, offset: usize, count: usize) -> VolatileMemoryResult { + self.0.get_slice(offset, count) + } +} + +// Safe because it exclusively forwards calls to a safe implementation. +unsafe impl MappedRegion for MemoryMapping { + fn as_ptr(&self) -> *mut u8 { + self.0.as_ptr() + } + + fn size(&self) -> usize { + self.0.size() + } +} diff --git a/base/src/shm.rs b/base/src/shm.rs new file mode 100644 index 0000000000..59594f6f4d --- /dev/null +++ b/base/src/shm.rs @@ -0,0 +1,86 @@ +// Copyright 2020 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +use crate::{ + AsRawDescriptor, FromRawDescriptor, IntoRawDescriptor, MemfdSeals, RawDescriptor, Result, + SafeDescriptor, +}; +use std::ffi::CStr; +use std::fs::File; +use std::os::unix::io::{AsRawFd, RawFd}; +use sys_util::SharedMemory as SysUtilSharedMemory; + +/// See [SharedMemory](sys_util::SharedMemory) for struct- and method-level +/// documentation. +pub struct SharedMemory(SysUtilSharedMemory); +impl SharedMemory { + pub fn named>>(name: T, size: u64) -> Result { + SysUtilSharedMemory::named(name) + .and_then(|mut shm| shm.set_size(size).map(|_| shm)) + .map(|shm| SharedMemory(shm)) + } + + pub fn anon(size: u64) -> Result { + Self::new(None, size) + } + + pub fn new(name: Option<&CStr>, size: u64) -> Result { + SysUtilSharedMemory::new(name) + .and_then(|mut shm| shm.set_size(size).map(|_| shm)) + .map(|shm| SharedMemory(shm)) + } + + pub fn size(&self) -> u64 { + self.0.size() + } + + /// Unwraps the sys_util::SharedMemory stored within this type. + /// This should be used only when necessary for interacting with + /// external libraries. + pub fn inner(&self) -> &SysUtilSharedMemory { + &self.0 + } +} + +pub trait Unix { + /// Creates a SharedMemory instance from a SafeDescriptor owning a reference to a + /// shared memory descriptor. Ownership of the underlying descriptor is transferred to the + /// new SharedMemory object. + fn from_safe_descriptor(descriptor: SafeDescriptor) -> Result { + let file = unsafe { File::from_raw_descriptor(descriptor.into_raw_descriptor()) }; + SysUtilSharedMemory::from_file(file).map(|shm| SharedMemory(shm)) + } + + fn from_file(file: File) -> Result { + SysUtilSharedMemory::from_file(file).map(|shm| SharedMemory(shm)) + } + + fn get_seals(&self) -> Result; + + fn add_seals(&mut self, seals: MemfdSeals) -> Result<()>; +} + +impl Unix for SharedMemory { + fn get_seals(&self) -> Result { + self.0.get_seals() + } + + fn add_seals(&mut self, seals: MemfdSeals) -> Result<()> { + self.0.add_seals(seals) + } +} + +// TODO(mikehoyle): Remove this in favor of just AsRawDescriptor +// when the rest of the codebase is ready. +impl AsRawFd for SharedMemory { + fn as_raw_fd(&self) -> RawFd { + self.0.as_raw_fd() + } +} + +impl AsRawDescriptor for SharedMemory { + fn as_raw_descriptor(&self) -> RawDescriptor { + self.0.as_raw_fd() + } +} diff --git a/devices/src/pci/ac97_bus_master.rs b/devices/src/pci/ac97_bus_master.rs index 253446dae2..41b9a4fd4e 100644 --- a/devices/src/pci/ac97_bus_master.rs +++ b/devices/src/pci/ac97_bus_master.rs @@ -550,7 +550,7 @@ impl Ac97BusMaster { DEVICE_SAMPLE_RATE, buffer_frames, &Self::stream_effects(func), - self.mem.as_ref(), + self.mem.as_ref().inner(), starting_offsets, ) .map_err(AudioError::CreateStream)?; diff --git a/devices/src/pci/vfio_pci.rs b/devices/src/pci/vfio_pci.rs index 11ee077c4e..2aa253235a 100644 --- a/devices/src/pci/vfio_pci.rs +++ b/devices/src/pci/vfio_pci.rs @@ -740,7 +740,7 @@ impl VfioPciDevice { // Even if vm has mapped this region, but it is in vm main process, // device process doesn't has this mapping, but vfio_dma_map() need it // in device process, so here map it again. - let mmap = match MemoryMapping::from_fd_offset( + let mmap = match MemoryMapping::from_descriptor_offset( self.device.as_ref(), mmap_size as usize, offset, diff --git a/devices/src/vfio.rs b/devices/src/vfio.rs index 3ba1fa921b..23597d97c0 100644 --- a/devices/src/vfio.rs +++ b/devices/src/vfio.rs @@ -17,8 +17,8 @@ use std::u32; use sync::Mutex; use base::{ - ioctl, ioctl_with_mut_ref, ioctl_with_ptr, ioctl_with_ref, ioctl_with_val, warn, Error, - EventFd, SafeDescriptor, + ioctl, ioctl_with_mut_ref, ioctl_with_ptr, ioctl_with_ref, ioctl_with_val, warn, + AsRawDescriptor, Error, EventFd, RawDescriptor, SafeDescriptor, }; use hypervisor::{DeviceKind, Vm}; use vm_memory::GuestMemory; @@ -807,8 +807,15 @@ impl VfioDevice { } } +// TODO(mikehoyle): Remove this in favor of AsRawDescriptor impl AsRawFd for VfioDevice { fn as_raw_fd(&self) -> RawFd { self.dev.as_raw_fd() } } + +impl AsRawDescriptor for VfioDevice { + fn as_raw_descriptor(&self) -> RawDescriptor { + self.dev.as_raw_descriptor() + } +} diff --git a/devices/src/virtio/wl.rs b/devices/src/virtio/wl.rs index 527d01a16d..ff47752841 100644 --- a/devices/src/virtio/wl.rs +++ b/devices/src/virtio/wl.rs @@ -56,8 +56,8 @@ use data_model::*; #[cfg(feature = "wl-dmabuf")] use base::ioctl_iow_nr; use base::{ - error, pipe, round_up_to_page_size, warn, Error, EventFd, FileFlags, PollContext, PollToken, - Result, ScmSocket, SharedMemory, + error, pipe, round_up_to_page_size, warn, AsRawDescriptor, Error, EventFd, FileFlags, + PollContext, PollToken, Result, ScmSocket, SharedMemory, SharedMemoryUnix, }; use msg_socket::{MsgError, MsgReceiver, MsgSender}; #[cfg(feature = "wl-dmabuf")] @@ -263,7 +263,6 @@ fn encode_resp(writer: &mut Writer, resp: WlResp) -> WlResult<()> { enum WlError { NewAlloc(Error), NewPipe(Error), - AllocSetSize(Error), SocketConnect(io::Error), SocketNonBlock(io::Error), VmControl(MsgError), @@ -278,6 +277,7 @@ enum WlError { ReadPipe(io::Error), PollContextAdd(Error), DmabufSync(io::Error), + FromSharedMemory(Error), WriteResponse(io::Error), InvalidString(std::str::Utf8Error), UnknownSocketName(String), @@ -290,7 +290,6 @@ impl Display for WlError { match self { NewAlloc(e) => write!(f, "failed to create shared memory allocation: {}", e), NewPipe(e) => write!(f, "failed to create pipe: {}", e), - AllocSetSize(e) => write!(f, "failed to set size of shared memory: {}", e), SocketConnect(e) => write!(f, "failed to connect socket: {}", e), SocketNonBlock(e) => write!(f, "failed to set socket as non-blocking: {}", e), VmControl(e) => write!(f, "failed to control parent VM: {}", e), @@ -305,6 +304,9 @@ impl Display for WlError { ReadPipe(e) => write!(f, "failed to read a pipe: {}", e), PollContextAdd(e) => write!(f, "failed to listen to FD on poll context: {}", e), DmabufSync(e) => write!(f, "failed to synchronize DMABuf access: {}", e), + FromSharedMemory(e) => { + write!(f, "failed to create shared memory from descriptor: {}", e) + } WriteResponse(e) => write!(f, "failed to write response: {}", e), InvalidString(e) => write!(f, "invalid string: {}", e), UnknownSocketName(name) => write!(f, "unknown socket name: {}", name), @@ -519,7 +521,7 @@ impl<'a> WlResp<'a> { #[derive(Default)] struct WlVfd { socket: Option, - guest_shared_memory: Option<(u64 /* size */, File)>, + guest_shared_memory: Option<(u64 /* size */, SharedMemory)>, remote_pipe: Option, local_pipe: Option<(u32 /* flags */, File)>, slot: Option<(MemSlot, u64 /* pfn */, VmRequester)>, @@ -556,18 +558,17 @@ impl WlVfd { fn allocate(vm: VmRequester, size: u64) -> WlResult { let size_page_aligned = round_up_to_page_size(size as usize) as u64; - let mut vfd_shm = SharedMemory::named("virtwl_alloc").map_err(WlError::NewAlloc)?; - vfd_shm - .set_size(size_page_aligned) - .map_err(WlError::AllocSetSize)?; + let vfd_shm = + SharedMemory::named("virtwl_alloc", size_page_aligned).map_err(WlError::NewAlloc)?; + let register_response = vm.request(VmMemoryRequest::RegisterMemory( - MaybeOwnedFd::Borrowed(vfd_shm.as_raw_fd()), + MaybeOwnedFd::Borrowed(vfd_shm.as_raw_descriptor()), vfd_shm.size() as usize, ))?; match register_response { VmMemoryResponse::RegisterMemory { pfn, slot } => { let mut vfd = WlVfd::default(); - vfd.guest_shared_memory = Some((vfd_shm.size(), vfd_shm.into())); + vfd.guest_shared_memory = Some((vfd_shm.size(), vfd_shm)); vfd.slot = Some((slot, pfn, vm)); Ok(vfd) } @@ -597,7 +598,7 @@ impl WlVfd { } => { let mut vfd = WlVfd::default(); let vfd_shm = SharedMemory::from_file(file).map_err(WlError::NewAlloc)?; - vfd.guest_shared_memory = Some((vfd_shm.size(), vfd_shm.into())); + vfd.guest_shared_memory = Some((vfd_shm.size(), vfd_shm)); vfd.slot = Some((slot, pfn, vm)); vfd.is_dmabuf = true; Ok((vfd, desc)) @@ -661,7 +662,10 @@ impl WlVfd { match register_response { VmMemoryResponse::RegisterMemory { pfn, slot } => { let mut vfd = WlVfd::default(); - vfd.guest_shared_memory = Some((size, fd)); + vfd.guest_shared_memory = Some(( + size, + SharedMemory::from_file(fd).map_err(WlError::FromSharedMemory)?, + )); vfd.slot = Some((slot, pfn, vm)); Ok(vfd) } @@ -716,7 +720,7 @@ impl WlVfd { fn send_fd(&self) -> Option { self.guest_shared_memory .as_ref() - .map(|(_, fd)| fd.as_raw_fd()) + .map(|(_, fd)| fd.as_raw_descriptor()) .or(self.socket.as_ref().map(|s| s.as_raw_fd())) .or(self.remote_pipe.as_ref().map(|p| p.as_raw_fd())) } diff --git a/disk/Cargo.toml b/disk/Cargo.toml index 79e7a968bb..468eef58ed 100644 --- a/disk/Cargo.toml +++ b/disk/Cargo.toml @@ -16,6 +16,7 @@ base = { path = "../base" } libc = "*" protobuf = { version = "2.3", optional = true } remain = "*" +tempfile = "*" cros_async = { path = "../cros_async" } data_model = { path = "../data_model" } protos = { path = "../protos", optional = true } diff --git a/disk/src/android_sparse.rs b/disk/src/android_sparse.rs index 70e84e7591..511f807aeb 100644 --- a/disk/src/android_sparse.rs +++ b/disk/src/android_sparse.rs @@ -329,8 +329,8 @@ impl FileReadWriteAtVolatile for AndroidSparse { #[cfg(test)] mod tests { use super::*; - use base::SharedMemory; use std::io::{Cursor, Write}; + use tempfile::tempfile; const CHUNK_SIZE: usize = mem::size_of::(); @@ -419,7 +419,7 @@ mod tests { } fn test_image(chunks: Vec) -> AndroidSparse { - let file: File = SharedMemory::anon().unwrap().into(); + let file = tempfile().expect("failed to create tempfile"); let size = chunks.iter().map(|x| x.expanded_size).sum(); AndroidSparse::from_parts(file, size, chunks).expect("Could not create image") } diff --git a/disk/src/qcow/mod.rs b/disk/src/qcow/mod.rs index 95e82c9a71..4091827564 100644 --- a/disk/src/qcow/mod.rs +++ b/disk/src/qcow/mod.rs @@ -1761,9 +1761,9 @@ fn div_round_up_u32(dividend: u32, divisor: u32) -> u32 { #[cfg(test)] mod tests { use super::*; - use base::{SharedMemory, WriteZeroes}; - use std::fs::File; + use base::WriteZeroes; use std::io::{Read, Seek, SeekFrom, Write}; + use tempfile::tempfile; fn valid_header() -> Vec { vec![ @@ -1813,8 +1813,7 @@ mod tests { } fn basic_file(header: &[u8]) -> File { - let shm = SharedMemory::anon().unwrap(); - let mut disk_file: File = shm.into(); + let mut disk_file = tempfile().expect("failed to create tempfile"); disk_file.write_all(&header).unwrap(); disk_file.set_len(0x8000_0000).unwrap(); disk_file.seek(SeekFrom::Start(0)).unwrap(); @@ -1832,8 +1831,8 @@ mod tests { where F: FnMut(QcowFile), { - let shm = SharedMemory::anon().unwrap(); - let qcow_file = QcowFile::new(shm.into(), file_size).unwrap(); + let file = tempfile().expect("failed to create tempfile"); + let qcow_file = QcowFile::new(file, file_size).unwrap(); testfn(qcow_file); // File closed when the function exits. } @@ -1841,8 +1840,7 @@ mod tests { #[test] fn default_header() { let header = QcowHeader::create_for_size_and_path(0x10_0000, None); - let shm = SharedMemory::anon().unwrap(); - let mut disk_file: File = shm.into(); + let mut disk_file = tempfile().expect("failed to create tempfile"); header .expect("Failed to create header.") .write_to(&mut disk_file) @@ -1862,8 +1860,7 @@ mod tests { fn header_with_backing() { let header = QcowHeader::create_for_size_and_path(0x10_0000, Some("/my/path/to/a/file")) .expect("Failed to create header."); - let shm = SharedMemory::anon().unwrap(); - let mut disk_file: File = shm.into(); + let mut disk_file = tempfile().expect("failed to create tempfile"); header .write_to(&mut disk_file) .expect("Failed to write header to shm."); diff --git a/gpu_display/src/gpu_display_wl.rs b/gpu_display/src/gpu_display_wl.rs index 2eba9d6c2a..38c2746754 100644 --- a/gpu_display/src/gpu_display_wl.rs +++ b/gpu_display/src/gpu_display_wl.rs @@ -21,7 +21,7 @@ use std::os::unix::io::{AsRawFd, RawFd}; use std::path::Path; use std::ptr::{null, null_mut}; -use base::{round_up_to_page_size, MemoryMapping, SharedMemory}; +use base::{round_up_to_page_size, AsRawDescriptor, MemoryMapping, SharedMemory}; use data_model::VolatileMemory; const BUFFER_COUNT: usize = 3; @@ -204,12 +204,9 @@ impl DisplayT for DisplayWl { let row_size = width * BYTES_PER_PIXEL; let fb_size = row_size * height; let buffer_size = round_up_to_page_size(fb_size as usize * BUFFER_COUNT); - let mut buffer_shm = - SharedMemory::named("GpuDisplaySurface").map_err(GpuDisplayError::CreateShm)?; - buffer_shm - .set_size(buffer_size as u64) - .map_err(GpuDisplayError::SetSize)?; - let buffer_mem = MemoryMapping::from_fd(&buffer_shm, buffer_size).unwrap(); + let buffer_shm = SharedMemory::named("GpuDisplaySurface", buffer_size as u64) + .map_err(GpuDisplayError::CreateShm)?; + let buffer_mem = MemoryMapping::from_descriptor(&buffer_shm, buffer_size).unwrap(); // Safe because only a valid context, parent pointer (if not None), and buffer FD are used. // The returned surface is checked for validity before being filed away. @@ -217,7 +214,7 @@ impl DisplayT for DisplayWl { dwl_context_surface_new( self.ctx(), parent_ptr, - buffer_shm.as_raw_fd(), + buffer_shm.as_raw_descriptor(), buffer_size, fb_size as usize, width, diff --git a/gpu_display/src/lib.rs b/gpu_display/src/lib.rs index 3ccc73dd3c..7c859d851a 100644 --- a/gpu_display/src/lib.rs +++ b/gpu_display/src/lib.rs @@ -31,8 +31,6 @@ pub enum GpuDisplayError { CreateEventFd, /// Creating shared memory failed. CreateShm(SysError), - /// Setting the size of shared memory failed. - SetSize(SysError), /// Failed to create a surface on the compositor. CreateSurface, /// Failed to import a buffer to the compositor. @@ -61,7 +59,6 @@ impl Display for GpuDisplayError { InvalidPath => write!(f, "invalid path"), InvalidSurfaceId => write!(f, "invalid surface ID"), RequiredFeature(feature) => write!(f, "required feature was missing: {}", feature), - SetSize(e) => write!(f, "failed to set size of shared memory: {}", e), Unsupported => write!(f, "unsupported by the implementation"), } } diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index a31e947c5d..ce5219a4a1 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -227,7 +227,7 @@ impl KvmVm { let vcpu = unsafe { SafeDescriptor::from_raw_descriptor(fd) }; let run_mmap = - MemoryMapping::from_fd(&vcpu, run_mmap_size).map_err(|_| Error::new(ENOSPC))?; + MemoryMapping::from_descriptor(&vcpu, run_mmap_size).map_err(|_| Error::new(ENOSPC))?; Ok(KvmVcpu { vm: self.vm.try_clone()?, @@ -607,8 +607,8 @@ impl Vcpu for KvmVcpu { fn try_clone(&self) -> Result { let vm = self.vm.try_clone()?; let vcpu = self.vcpu.try_clone()?; - let run_mmap = - MemoryMapping::from_fd(&vcpu, self.run_mmap.size()).map_err(|_| Error::new(ENOSPC))?; + let run_mmap = MemoryMapping::from_descriptor(&vcpu, self.run_mmap.size()) + .map_err(|_| Error::new(ENOSPC))?; Ok(KvmVcpu { vm, vcpu, run_mmap }) } diff --git a/io_uring/src/uring.rs b/io_uring/src/uring.rs index 6ca7fca342..ca005e522a 100644 --- a/io_uring/src/uring.rs +++ b/io_uring/src/uring.rs @@ -121,7 +121,7 @@ impl URingContext { // Safe because we trust the kernel to set valid sizes in `io_uring_setup` and any error // is checked. let submit_ring = SubmitQueueState::new( - MemoryMapping::from_fd_offset_populate( + MemoryMapping::from_descriptor_offset_populate( &ring_file, ring_params.sq_off.array as usize + ring_params.sq_entries as usize * std::mem::size_of::(), @@ -133,7 +133,7 @@ impl URingContext { let num_sqe = ring_params.sq_entries as usize; let submit_queue_entries = SubmitQueueEntries { - mmap: MemoryMapping::from_fd_offset_populate( + mmap: MemoryMapping::from_descriptor_offset_populate( &ring_file, ring_params.sq_entries as usize * std::mem::size_of::(), u64::from(IORING_OFF_SQES), @@ -143,7 +143,7 @@ impl URingContext { }; let complete_ring = CompleteQueueState::new( - MemoryMapping::from_fd_offset_populate( + MemoryMapping::from_descriptor_offset_populate( &ring_file, ring_params.cq_off.cqes as usize + ring_params.cq_entries as usize * std::mem::size_of::(), diff --git a/kernel_loader/Cargo.toml b/kernel_loader/Cargo.toml index 45880b0111..3731fa0cac 100644 --- a/kernel_loader/Cargo.toml +++ b/kernel_loader/Cargo.toml @@ -7,3 +7,6 @@ edition = "2018" libc = "*" base = { path = "../base" } vm_memory = { path = "../vm_memory" } + +[dev-dependencies] +tempfile = "*" \ No newline at end of file diff --git a/kernel_loader/src/lib.rs b/kernel_loader/src/lib.rs index a24f3a0111..7f45b34d78 100644 --- a/kernel_loader/src/lib.rs +++ b/kernel_loader/src/lib.rs @@ -6,8 +6,8 @@ use std::ffi::CStr; use std::fmt::{self, Display}; use std::io::{Read, Seek, SeekFrom}; use std::mem; -use std::os::unix::io::AsRawFd; +use base::AsRawDescriptor; use vm_memory::{GuestAddress, GuestMemory}; #[allow(dead_code)] @@ -76,7 +76,7 @@ pub fn load_kernel( kernel_image: &mut F, ) -> Result where - F: Read + Seek + AsRawFd, + F: Read + Seek + AsRawDescriptor, { let mut ehdr: elf::Elf64_Ehdr = Default::default(); kernel_image @@ -177,9 +177,9 @@ pub fn load_cmdline( #[cfg(test)] mod test { use super::*; - use base::SharedMemory; use std::fs::File; use std::io::Write; + use tempfile::tempfile; use vm_memory::{GuestAddress, GuestMemory}; const MEM_SIZE: u64 = 0x8000; @@ -233,12 +233,10 @@ mod test { // Elf64 image that prints hello world on x86_64. fn make_elf_bin() -> File { let elf_bytes = include_bytes!("test_elf.bin"); - let mut shm = SharedMemory::anon().expect("failed to create shared memory"); - shm.set_size(elf_bytes.len() as u64) - .expect("failed to set shared memory size"); - shm.write_all(elf_bytes) + let mut file = tempfile().expect("failed to create tempfile"); + file.write_all(elf_bytes) .expect("failed to write elf to shared memoy"); - shm.into() + file } fn mutate_elf_bin(mut f: &File, offset: u64, val: u8) { diff --git a/kvm/src/lib.rs b/kvm/src/lib.rs index 7687cb3c39..d50f328680 100644 --- a/kvm/src/lib.rs +++ b/kvm/src/lib.rs @@ -18,6 +18,7 @@ use std::ptr::copy_nonoverlapping; use std::sync::Arc; use sync::Mutex; +use base::{AsRawDescriptor, RawDescriptor}; use data_model::vec_with_array_field; use libc::sigset_t; @@ -947,7 +948,7 @@ impl Vcpu { let vcpu = unsafe { File::from_raw_fd(vcpu_fd) }; let run_mmap = - MemoryMapping::from_fd(&vcpu, run_mmap_size).map_err(|_| Error::new(ENOSPC))?; + MemoryMapping::from_descriptor(&vcpu, run_mmap_size).map_err(|_| Error::new(ENOSPC))?; Ok(Vcpu { vcpu, run_mmap }) } @@ -1458,6 +1459,12 @@ impl AsRawFd for Vcpu { } } +impl AsRawDescriptor for Vcpu { + fn as_raw_descriptor(&self) -> RawDescriptor { + self.vcpu.as_raw_descriptor() + } +} + /// A Vcpu that has a thread and can be run. Created by calling `to_runnable` on a `Vcpu`. /// Implements `Deref` to a `Vcpu` so all `Vcpu` methods are usable, with the addition of the `run` /// function to execute the guest. @@ -1619,7 +1626,7 @@ impl DerefMut for RunnableVcpu { impl AsRawFd for RunnableVcpu { fn as_raw_fd(&self) -> RawFd { - self.vcpu.as_raw_fd() + self.vcpu.as_raw_descriptor() } } diff --git a/kvm/tests/dirty_log.rs b/kvm/tests/dirty_log.rs index 0a75cbe522..12127a82f1 100644 --- a/kvm/tests/dirty_log.rs +++ b/kvm/tests/dirty_log.rs @@ -19,11 +19,9 @@ fn test_run() { let mem_size = 0x10000; let load_addr = GuestAddress(0x1000); let guest_mem = GuestMemory::new(&[]).unwrap(); - let mut mem = SharedMemory::anon().expect("failed to create shared memory"); - mem.set_size(mem_size) - .expect("failed to set shared memory size"); - let mmap = - MemoryMapping::from_fd(&mem, mem_size as usize).expect("failed to create memory mapping"); + let mem = SharedMemory::anon(mem_size).expect("failed to create shared memory"); + let mmap = MemoryMapping::from_descriptor(&mem, mem_size as usize) + .expect("failed to create memory mapping"); mmap.write_slice(&code[..], load_addr.offset() as usize) .expect("Writing code to memory failed."); @@ -47,7 +45,7 @@ fn test_run() { .add_memory_region( GuestAddress(0), Box::new( - MemoryMapping::from_fd(&mem, mem_size as usize) + MemoryMapping::from_descriptor(&mem, mem_size as usize) .expect("failed to create memory mapping"), ), false, diff --git a/kvm/tests/read_only_memory.rs b/kvm/tests/read_only_memory.rs index 982b5824a4..9444ba2aa9 100644 --- a/kvm/tests/read_only_memory.rs +++ b/kvm/tests/read_only_memory.rs @@ -21,11 +21,9 @@ fn test_run() { let mem_size = 0x2000; let load_addr = GuestAddress(0x1000); let guest_mem = GuestMemory::new(&[]).unwrap(); - let mut mem = SharedMemory::anon().expect("failed to create shared memory"); - mem.set_size(mem_size) - .expect("failed to set shared memory size"); - let mmap = - MemoryMapping::from_fd(&mem, mem_size as usize).expect("failed to create memory mapping"); + let mem = SharedMemory::anon(mem_size).expect("failed to create shared memory"); + let mmap = MemoryMapping::from_descriptor(&mem, mem_size as usize) + .expect("failed to create memory mapping"); mmap.write_slice(&code[..], load_addr.offset() as usize) .expect("Writing code to memory failed."); @@ -49,7 +47,7 @@ fn test_run() { vm.add_memory_region( GuestAddress(0), Box::new( - MemoryMapping::from_fd(&mem, mem_size as usize) + MemoryMapping::from_descriptor(&mem, mem_size as usize) .expect("failed to create memory mapping"), ), false, @@ -59,17 +57,18 @@ fn test_run() { // Give some read only memory for the test code to read from and force a vcpu exit when it reads // from it. - let mut mem_ro = SharedMemory::anon().expect("failed to create shared memory"); - mem_ro - .set_size(0x1000) - .expect("failed to set shared memory size"); - let mmap_ro = MemoryMapping::from_fd(&mem_ro, 0x1000).expect("failed to create memory mapping"); + let mem_ro = SharedMemory::anon(0x1000).expect("failed to create shared memory"); + let mmap_ro = + MemoryMapping::from_descriptor(&mem_ro, 0x1000).expect("failed to create memory mapping"); mmap_ro .write_obj(vcpu_regs.rax as u8, 0) .expect("failed writing data to ro memory"); vm.add_memory_region( GuestAddress(vcpu_sregs.es.base), - Box::new(MemoryMapping::from_fd(&mem_ro, 0x1000).expect("failed to create memory mapping")), + Box::new( + MemoryMapping::from_descriptor(&mem_ro, 0x1000) + .expect("failed to create memory mapping"), + ), true, false, ) diff --git a/src/plugin/process.rs b/src/plugin/process.rs index 3f90fc1f00..4de63cd3f7 100644 --- a/src/plugin/process.rs +++ b/src/plugin/process.rs @@ -22,7 +22,7 @@ use protobuf::Message; use base::{ error, Error as SysError, EventFd, Killable, MemoryMapping, Result as SysResult, ScmSocket, - SharedMemory, SIGRTMIN, + SharedMemory, SharedMemoryUnix, SIGRTMIN, }; use kvm::{dirty_log_bitmap_size, Datamatch, IoeventAddress, IrqRoute, IrqSource, PicId, Vm}; use kvm_sys::{kvm_clock_data, kvm_ioapic_state, kvm_pic_state, kvm_pit_state2}; @@ -360,7 +360,7 @@ impl Process { None => return Err(SysError::new(EOVERFLOW)), _ => {} } - let mem = MemoryMapping::from_fd_offset(&shm, length as usize, offset) + let mem = MemoryMapping::from_descriptor_offset(&shm, length as usize, offset) .map_err(mmap_to_sys_err)?; let slot = vm.add_memory_region(GuestAddress(start), Box::new(mem), read_only, dirty_log)?; diff --git a/sys_util/src/descriptor.rs b/sys_util/src/descriptor.rs index 1a6da4f491..0eff3d05b6 100644 --- a/sys_util/src/descriptor.rs +++ b/sys_util/src/descriptor.rs @@ -2,9 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +use std::fs::File; use std::mem; use std::ops::Drop; -use std::os::unix::io::{AsRawFd, RawFd}; +use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; use crate::{errno_result, Result}; @@ -120,6 +121,52 @@ impl AsRawDescriptor for Descriptor { } } +// AsRawFd for interoperability with interfaces that require it. Within crosvm, +// always use AsRawDescriptor when possible. +impl AsRawFd for Descriptor { + fn as_raw_fd(&self) -> RawFd { + self.0 + } +} + +macro_rules! AsRawDescriptor { + ($name:ident) => { + impl AsRawDescriptor for $name { + fn as_raw_descriptor(&self) -> RawDescriptor { + self.as_raw_fd() + } + } + }; +} + +macro_rules! FromRawDescriptor { + ($name:ident) => { + impl FromRawDescriptor for $name { + unsafe fn from_raw_descriptor(descriptor: RawDescriptor) -> Self { + $name::from_raw_fd(descriptor) + } + } + }; +} + +macro_rules! IntoRawDescriptor { + ($name:ident) => { + impl IntoRawDescriptor for $name { + fn into_raw_descriptor(self) -> RawDescriptor { + self.into_raw_fd() + } + } + }; +} + +// Implementations for File. This enables the File-type to use +// RawDescriptor, but does not mean File should be used as a generic +// descriptor container. That should go to either SafeDescriptor or another more +// relevant container type. +AsRawDescriptor!(File); +FromRawDescriptor!(File); +IntoRawDescriptor!(File); + #[test] fn clone_equality() { let ret = unsafe { libc::eventfd(0, 0) }; diff --git a/tests/plugins.rs b/tests/plugins.rs index 7f2cb84c07..054ce6adf4 100644 --- a/tests/plugins.rs +++ b/tests/plugins.rs @@ -6,7 +6,7 @@ use std::env::{current_exe, var_os}; use std::ffi::OsString; -use std::fs::{remove_file, File}; +use std::fs::remove_file; use std::io::{Read, Write}; use std::os::unix::io::AsRawFd; use std::path::{Path, PathBuf}; @@ -14,8 +14,9 @@ use std::process::{Command, Stdio}; use std::thread::sleep; use std::time::Duration; -use base::{ioctl, SharedMemory}; +use base::{ioctl, AsRawDescriptor}; use rand_ish::urandom_str; +use tempfile::tempfile; struct RemovePath(PathBuf); impl Drop for RemovePath { @@ -134,28 +135,27 @@ fn keep_fd_on_exec(f: &F) { /// Takes assembly source code and returns the resulting assembly code. fn build_assembly(src: &str) -> Vec { - // Creates a shared memory region with the assembly source code in it. - let in_shm = SharedMemory::anon().unwrap(); - let mut in_shm_file: File = in_shm.into(); - keep_fd_on_exec(&in_shm_file); - in_shm_file.write_all(src.as_bytes()).unwrap(); + // Creates a file with the assembly source code in it. + let mut in_file = tempfile().expect("failed to create tempfile"); + keep_fd_on_exec(&in_file); + in_file.write_all(src.as_bytes()).unwrap(); - // Creates a shared memory region that will hold the nasm output. - let mut out_shm_file: File = SharedMemory::anon().unwrap().into(); - keep_fd_on_exec(&out_shm_file); + // Creates a file that will hold the nasm output. + let mut out_file = tempfile().expect("failed to create tempfile"); + keep_fd_on_exec(&out_file); // Runs nasm with the input and output files set to the FDs of the above shared memory regions, // which we have preserved accross exec. let status = Command::new("nasm") - .arg(format!("/proc/self/fd/{}", in_shm_file.as_raw_fd())) + .arg(format!("/proc/self/fd/{}", in_file.as_raw_descriptor())) .args(&["-f", "bin", "-o"]) - .arg(format!("/proc/self/fd/{}", out_shm_file.as_raw_fd())) + .arg(format!("/proc/self/fd/{}", out_file.as_raw_descriptor())) .status() .expect("failed to spawn assembler"); assert!(status.success()); let mut out_bytes = Vec::new(); - out_shm_file.read_to_end(&mut out_bytes).unwrap(); + out_file.read_to_end(&mut out_bytes).unwrap(); out_bytes } diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs index 15e7171065..d315cb5168 100644 --- a/vm_control/src/lib.rs +++ b/vm_control/src/lib.rs @@ -20,8 +20,8 @@ use std::sync::Arc; use libc::{EINVAL, EIO, ENODEV}; use base::{ - error, Error as SysError, EventFd, ExternalMapping, MappedRegion, MemoryMapping, MmapError, - Result, + error, AsRawDescriptor, Error as SysError, EventFd, ExternalMapping, MappedRegion, + MemoryMapping, MmapError, RawDescriptor, Result, }; use hypervisor::{IrqRoute, IrqSource, Vm}; use msg_socket::{MsgError, MsgOnSocket, MsgReceiver, MsgResult, MsgSender, MsgSocket}; @@ -37,15 +37,22 @@ pub enum MaybeOwnedFd { /// Owned by this enum variant, and will be destructed automatically if not moved out. Owned(File), /// A file descriptor borrwed by this enum. - Borrowed(RawFd), + Borrowed(RawDescriptor), } +impl AsRawDescriptor for MaybeOwnedFd { + fn as_raw_descriptor(&self) -> RawDescriptor { + match self { + MaybeOwnedFd::Owned(f) => f.as_raw_descriptor(), + MaybeOwnedFd::Borrowed(descriptor) => *descriptor, + } + } +} + +// TODO(mikehoyle): Remove this in favor of just AsRawDescriptor impl AsRawFd for MaybeOwnedFd { fn as_raw_fd(&self) -> RawFd { - match self { - MaybeOwnedFd::Owned(f) => f.as_raw_fd(), - MaybeOwnedFd::Borrowed(fd) => *fd, - } + self.as_raw_descriptor() } } @@ -370,7 +377,7 @@ impl VmMemoryRequest { offset, gpa, } => { - let mmap = match MemoryMapping::from_fd_offset(fd, size, offset as u64) { + let mmap = match MemoryMapping::from_descriptor_offset(fd, size, offset as u64) { Ok(v) => v, Err(_e) => return VmMemoryResponse::Err(SysError::new(EINVAL)), }; @@ -571,11 +578,11 @@ pub enum VmRequest { fn register_memory( vm: &mut impl Vm, allocator: &mut SystemAllocator, - fd: &dyn AsRawFd, + fd: &dyn AsRawDescriptor, size: usize, pci_allocation: Option<(Alloc, u64)>, ) -> Result<(u64, MemSlot)> { - let mmap = match MemoryMapping::from_fd(fd, size) { + let mmap = match MemoryMapping::from_descriptor(fd, size) { Ok(v) => v, Err(MmapError::SystemCallFailed(e)) => return Err(e), _ => return Err(SysError::new(EINVAL)), diff --git a/vm_memory/src/guest_memory.rs b/vm_memory/src/guest_memory.rs index b63e6cdc33..c0b11dedc1 100644 --- a/vm_memory/src/guest_memory.rs +++ b/vm_memory/src/guest_memory.rs @@ -14,8 +14,10 @@ use std::sync::Arc; use crate::guest_address::GuestAddress; use base::{pagesize, Error as SysError}; -use base::{MappedRegion, MemoryMapping, MmapError}; -use base::{MemfdSeals, SharedMemory}; +use base::{ + AsRawDescriptor, MappedRegion, MemfdSeals, MemoryMapping, MmapError, SharedMemory, + SharedMemoryUnix, +}; use cros_async::{ uring_mem::{self, BorrowedIoVec}, BackingMemory, @@ -33,7 +35,6 @@ pub enum Error { MemoryRegionTooLarge(u64), MemoryNotAligned, MemoryCreationFailed(SysError), - MemorySetSizeFailed(SysError), MemoryAddSealsFailed(SysError), ShortWrite { expected: usize, completed: usize }, ShortRead { expected: usize, completed: usize }, @@ -62,7 +63,6 @@ impl Display for Error { MemoryRegionTooLarge(size) => write!(f, "memory region size {} is too large", size), MemoryNotAligned => write!(f, "memfd regions must be page aligned"), MemoryCreationFailed(_) => write!(f, "failed to create memfd region"), - MemorySetSizeFailed(e) => write!(f, "failed to set memfd region size: {}", e), MemoryAddSealsFailed(e) => write!(f, "failed to set seals on memfd region: {}", e), ShortWrite { expected, @@ -117,7 +117,7 @@ pub struct GuestMemory { impl AsRawFd for GuestMemory { fn as_raw_fd(&self) -> RawFd { - self.memfd.as_raw_fd() + self.memfd.as_raw_descriptor() } } @@ -146,10 +146,8 @@ impl GuestMemory { seals.set_grow_seal(); seals.set_seal_seal(); - let mut memfd = SharedMemory::named("crosvm_guest").map_err(Error::MemoryCreationFailed)?; - memfd - .set_size(aligned_size) - .map_err(Error::MemorySetSizeFailed)?; + let mut memfd = SharedMemory::named("crosvm_guest", aligned_size) + .map_err(Error::MemoryCreationFailed)?; memfd .add_seals(seals) .map_err(Error::MemoryAddSealsFailed)?; @@ -180,7 +178,7 @@ impl GuestMemory { let size = usize::try_from(range.1).map_err(|_| Error::MemoryRegionTooLarge(range.1))?; - let mapping = MemoryMapping::from_fd_offset(&memfd, size, offset) + let mapping = MemoryMapping::from_descriptor_offset(&memfd, size, offset) .map_err(Error::MemoryMappingFailed)?; regions.push(MemoryRegion { mapping, @@ -545,7 +543,7 @@ impl GuestMemory { pub fn read_to_memory( &self, guest_addr: GuestAddress, - src: &dyn AsRawFd, + src: &dyn AsRawDescriptor, count: usize, ) -> Result<()> { self.do_in_region(guest_addr, move |mapping, offset| { @@ -583,7 +581,7 @@ impl GuestMemory { pub fn write_from_memory( &self, guest_addr: GuestAddress, - dst: &dyn AsRawFd, + dst: &dyn AsRawDescriptor, count: usize, ) -> Result<()> { self.do_in_region(guest_addr, move |mapping, offset| { @@ -853,7 +851,7 @@ mod tests { .unwrap(); let _ = gm.with_regions::<_, ()>(|index, _, size, _, memfd_offset| { - let mmap = MemoryMapping::from_fd_offset(&gm, size, memfd_offset).unwrap(); + let mmap = MemoryMapping::from_descriptor_offset(&gm, size, memfd_offset).unwrap(); if index == 0 { assert!(mmap.read_obj::(0x0).unwrap() == 0x1337u16); diff --git a/x86_64/src/bzimage.rs b/x86_64/src/bzimage.rs index 86ff400cf8..7dd3041e34 100644 --- a/x86_64/src/bzimage.rs +++ b/x86_64/src/bzimage.rs @@ -7,8 +7,8 @@ use std::fmt::{self, Display}; use std::io::{Read, Seek, SeekFrom}; -use std::os::unix::io::AsRawFd; +use base::AsRawDescriptor; use vm_memory::{GuestAddress, GuestMemory}; use crate::bootparam::boot_params; @@ -58,7 +58,7 @@ pub fn load_bzimage( kernel_image: &mut F, ) -> Result<(boot_params, u64)> where - F: Read + Seek + AsRawFd, + F: Read + Seek + AsRawDescriptor, { let mut params: boot_params = Default::default(); kernel_image