From c79de2d0b2b75c18adf1b9f1779a6b482247531e Mon Sep 17 00:00:00 2001 From: Stephen Barber Date: Wed, 21 Feb 2018 14:17:27 -0800 Subject: [PATCH] crosvm: add advisory locking for disk images Disk images should never be mounted as writable by multiple VMs at once. Add advisory locking to prevent this. BUG=chromium:810576 TEST=run crosvm twice with same rwdisk, check that second VM fails to start Change-Id: I5e6c178515eafa570812a093449eef5a4edc1740 Reviewed-on: https://chromium-review.googlesource.com/929994 Commit-Ready: Stephen Barber Tested-by: Stephen Barber Reviewed-by: Dylan Reid Reviewed-by: Zach Reizner --- src/linux.rs | 10 ++++++++++ sys_util/src/lib.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/linux.rs b/src/linux.rs index c0b9094932..b4c76d9ec2 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -53,6 +53,7 @@ pub enum Error { DeviceJail(io_jail::Error), DevicePivotRoot(io_jail::Error), Disk(io::Error), + DiskImageLock(sys_util::Error), GetWaylandGroup(sys_util::Error), LoadCmdline(kernel_loader::Error), LoadKernel(kernel_loader::Error), @@ -107,6 +108,7 @@ impl fmt::Display for Error { &Error::DeviceJail(ref e) => write!(f, "failed to jail device: {}", e), &Error::DevicePivotRoot(ref e) => write!(f, "failed to pivot root device: {}", e), &Error::Disk(ref e) => write!(f, "failed to load disk image: {}", e), + &Error::DiskImageLock(ref e) => write!(f, "failed to lock disk image: {:?}", e), &Error::GetWaylandGroup(ref e) => { write!(f, "could not find gid for wayland group: {:?}", e) } @@ -303,6 +305,14 @@ fn setup_mmio_bus(cfg: &Config, .write(disk.writable) .open(&disk.path) .map_err(|e| Error::Disk(e))?; + // Lock the disk image to prevent other crosvm instances from using it. + let lock_op = if disk.writable { + FlockOperation::LockExclusive + } else { + FlockOperation::LockShared + }; + flock(&raw_image, lock_op, true).map_err(Error::DiskImageLock)?; + let block_box: Box = match disk.disk_type { DiskType::FlatFile => { // Access as a raw block device. Box::new(devices::virtio::Block::new(raw_image) diff --git a/sys_util/src/lib.rs b/sys_util/src/lib.rs index 7f9f034e20..e4547bb410 100644 --- a/sys_util/src/lib.rs +++ b/sys_util/src/lib.rs @@ -53,6 +53,7 @@ pub use guest_memory::Error as GuestMemoryError; pub use signalfd::Error as SignalFdError; use std::ffi::CStr; +use std::os::unix::io::AsRawFd; use std::ptr; use libc::{kill, syscall, sysconf, waitpid, c_long, pid_t, uid_t, gid_t, _SC_PAGESIZE, @@ -102,6 +103,37 @@ pub fn chown(path: &CStr, uid: uid_t, gid: gid_t) -> Result<()> { } } +/// The operation to perform with `flock`. +pub enum FlockOperation { + LockShared, + LockExclusive, + Unlock, +} + +/// Safe wrapper for flock(2) with the operation `op` and optionally `nonblocking`. The lock will be +/// dropped automatically when `file` is dropped. +#[inline(always)] +pub fn flock(file: &AsRawFd, op: FlockOperation, nonblocking: bool) -> Result<()> { + let mut operation = match op { + FlockOperation::LockShared => libc::LOCK_SH, + FlockOperation::LockExclusive => libc::LOCK_EX, + FlockOperation::Unlock => libc::LOCK_UN, + }; + + if nonblocking { + operation |= libc::LOCK_NB; + } + + // Safe since we pass in a valid fd and flock operation, and check the return value. + let ret = unsafe { libc::flock(file.as_raw_fd(), operation) }; + + if ret < 0 { + errno_result() + } else { + Ok(()) + } +} + /// Reaps a child process that has terminated. /// /// Returns `Ok(pid)` where `pid` is the process that was reaped or `Ok(0)` if none of the children