diff options
author | Chirantan Ekbote <chirantan@chromium.org> | 2018-11-16 11:35:24 -0800 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2019-01-07 19:40:14 -0800 |
commit | 2d292331df330b15365d0d2909b9fcaf8fca97ce (patch) | |
tree | 20dba369353353ecd5c1a9ed398af0cfddf57132 | |
parent | 37c4a788a32d5b9c90b63e34062dab26f7280443 (diff) | |
download | crosvm-2d292331df330b15365d0d2909b9fcaf8fca97ce.tar crosvm-2d292331df330b15365d0d2909b9fcaf8fca97ce.tar.gz crosvm-2d292331df330b15365d0d2909b9fcaf8fca97ce.tar.bz2 crosvm-2d292331df330b15365d0d2909b9fcaf8fca97ce.tar.lz crosvm-2d292331df330b15365d0d2909b9fcaf8fca97ce.tar.xz crosvm-2d292331df330b15365d0d2909b9fcaf8fca97ce.tar.zst crosvm-2d292331df330b15365d0d2909b9fcaf8fca97ce.zip |
Move validate_raw_fd to sys_util
validate_raw_fd is needed for the plugin crate. Move it into a common location so that it can be shared by both the linux and plugin code. BUG=b:80150167 TEST=manual Change-Id: I427e10716e75b2619fd0f4ba6725fa40446db4af Signed-off-by: Chirantan Ekbote <chirantan@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1341101 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Reviewed-by: Dylan Reid <dgreid@chromium.org>
-rw-r--r-- | src/linux.rs | 33 | ||||
-rw-r--r-- | sys_util/src/lib.rs | 23 |
2 files changed, 26 insertions, 30 deletions
diff --git a/src/linux.rs b/src/linux.rs index 9e44c91..5f607fb 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -10,7 +10,7 @@ use std::fmt; use std::fs::{File, OpenOptions}; use std::io::{self, stdin, Read}; use std::mem; -use std::os::unix::io::{FromRawFd, RawFd}; +use std::os::unix::io::FromRawFd; use std::os::unix::net::UnixDatagram; use std::path::{Path, PathBuf}; use std::str; @@ -62,8 +62,6 @@ pub enum Error { DevicePivotRoot(io_jail::Error), Disk(io::Error), DiskImageLock(sys_util::Error), - FailedCLOEXECCheck, - FailedToDupFd, InvalidFdPath, InvalidWaylandPath, NetDeviceNew(devices::virtio::NetError), @@ -90,6 +88,7 @@ pub enum Error { SignalFd(sys_util::SignalFdError), SpawnVcpu(io::Error), TimerFd(sys_util::Error), + ValidateRawFd(sys_util::Error), VhostNetDeviceNew(devices::virtio::vhost::Error), VhostVsockDeviceNew(devices::virtio::vhost::Error), VirtioPciDev(sys_util::Error), @@ -115,10 +114,6 @@ impl fmt::Display for Error { Error::DevicePivotRoot(e) => write!(f, "failed to pivot root device: {}", e), Error::Disk(e) => write!(f, "failed to load disk image: {}", e), Error::DiskImageLock(e) => write!(f, "failed to lock disk image: {:?}", e), - Error::FailedCLOEXECCheck => { - write!(f, "/proc/self/fd argument failed check for CLOEXEC") - } - Error::FailedToDupFd => write!(f, "failed to dup fd from /proc/self/fd"), Error::InvalidFdPath => write!(f, "failed parsing a /proc/self/fd/*"), Error::InvalidWaylandPath => { write!(f, "wayland socket path has no parent or file name") @@ -159,6 +154,7 @@ impl fmt::Display for Error { Error::SignalFd(e) => write!(f, "failed to read signal fd: {:?}", e), Error::SpawnVcpu(e) => write!(f, "failed to spawn VCPU thread: {:?}", e), Error::TimerFd(e) => write!(f, "failed to read timer fd: {:?}", e), + Error::ValidateRawFd(e) => write!(f, "failed to validate raw fd: {:?}", e), Error::VhostNetDeviceNew(e) => write!(f, "failed to set up vhost networking: {:?}", e), Error::VhostVsockDeviceNew(e) => { write!(f, "failed to set up virtual socket device: {:?}", e) @@ -178,27 +174,6 @@ impl std::error::Error for Error { type Result<T> = std::result::Result<T, Error>; -// Verifies that |raw_fd| is actually owned by this process and duplicates it to ensure that -// we have a unique handle to it. -fn validate_raw_fd(raw_fd: RawFd) -> std::result::Result<RawFd, Box<error::Error>> { - // Checking that close-on-exec isn't set helps filter out FDs that were opened by - // crosvm as all crosvm FDs are close on exec. - // Safe because this doesn't modify any memory and we check the return value. - let flags = unsafe { libc::fcntl(raw_fd, libc::F_GETFD) }; - if flags < 0 || (flags & libc::FD_CLOEXEC) != 0 { - return Err(Box::new(Error::FailedCLOEXECCheck)); - } - - // Duplicate the fd to ensure that we don't accidentally close an fd previously - // opened by another subsystem. Safe because this doesn't modify any memory and - // we check the return value. - let dup_fd = unsafe { libc::fcntl(raw_fd, libc::F_DUPFD_CLOEXEC, 0) }; - if dup_fd < 0 { - return Err(Box::new(Error::FailedToDupFd)); - } - Ok(dup_fd as RawFd) -} - fn create_base_minijail(root: &Path, seccomp_policy: &Path) -> Result<Minijail> { // All child jails run in a new user namespace without any users mapped, // they run as nobody unless otherwise configured. @@ -258,7 +233,7 @@ fn create_virtio_devs( .and_then(|fd_str| fd_str.parse::<c_int>().ok()) .ok_or(Error::InvalidFdPath)?; // Safe because we will validate |raw_fd|. - unsafe { File::from_raw_fd(validate_raw_fd(raw_fd)?) } + unsafe { File::from_raw_fd(validate_raw_fd(raw_fd).map_err(Error::ValidateRawFd)?) } } else { OpenOptions::new() .read(true) diff --git a/sys_util/src/lib.rs b/sys_util/src/lib.rs index 141c8b5..8e902ab50 100644 --- a/sys_util/src/lib.rs +++ b/sys_util/src/lib.rs @@ -73,7 +73,7 @@ pub use write_zeroes::{PunchHole, WriteZeroes}; use std::ffi::CStr; use std::fs::{remove_file, File}; -use std::os::unix::io::{AsRawFd, FromRawFd}; +use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::os::unix::net::UnixDatagram; use std::ptr; @@ -301,3 +301,24 @@ impl Drop for UnlinkUnixDatagram { } } } + +/// Verifies that |raw_fd| is actually owned by this process and duplicates it to ensure that +/// we have a unique handle to it. +pub fn validate_raw_fd(raw_fd: RawFd) -> Result<RawFd> { + // Checking that close-on-exec isn't set helps filter out FDs that were opened by + // crosvm as all crosvm FDs are close on exec. + // Safe because this doesn't modify any memory and we check the return value. + let flags = unsafe { libc::fcntl(raw_fd, libc::F_GETFD) }; + if flags < 0 || (flags & libc::FD_CLOEXEC) != 0 { + return Err(Error::new(libc::EBADF)); + } + + // Duplicate the fd to ensure that we don't accidentally close an fd previously + // opened by another subsystem. Safe because this doesn't modify any memory and + // we check the return value. + let dup_fd = unsafe { libc::fcntl(raw_fd, libc::F_DUPFD_CLOEXEC, 0) }; + if dup_fd < 0 { + return Err(Error::last()); + } + Ok(dup_fd as RawFd) +} |