diff --git a/src/linux.rs b/src/linux.rs index 43e69fcecd..0416ed4ee6 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -512,18 +512,33 @@ fn simple_jail(cfg: &Config, policy: &str) -> Result> { type DeviceResult = std::result::Result; -fn create_block_device(cfg: &Config, disk: &DiskOption, disk_device_tube: Tube) -> DeviceResult { +/// Open the file with the given path, or if it is of the form `/proc/self/fd/N` then just use the +/// file descriptor. +/// +/// Note that this will not work properly if the same `/proc/self/fd/N` path is used twice in +/// different places, as the metadata (including the offset) will be shared between both file +/// descriptors. +fn open_file(path: &Path, read_only: bool, error_constructor: F) -> Result +where + F: FnOnce(PathBuf, io::Error) -> Error, +{ // Special case '/proc/self/fd/*' paths. The FD is already open, just use it. - let raw_image: File = if disk.path.parent() == Some(Path::new("/proc/self/fd")) { - // Safe because we will validate |raw_fd|. - unsafe { File::from_raw_descriptor(raw_descriptor_from_path(&disk.path)?) } - } else { - OpenOptions::new() - .read(true) - .write(!disk.read_only) - .open(&disk.path) - .map_err(|e| Error::Disk(disk.path.to_path_buf(), e))? - }; + Ok( + if let Some(raw_descriptor) = raw_descriptor_from_path(path)? { + // Safe because we will validate |raw_fd|. + unsafe { File::from_raw_descriptor(raw_descriptor) } + } else { + OpenOptions::new() + .read(true) + .write(!read_only) + .open(path) + .map_err(|e| error_constructor(path.to_path_buf(), e))? + }, + ) +} + +fn create_block_device(cfg: &Config, disk: &DiskOption, disk_device_tube: Tube) -> DeviceResult { + let raw_image: File = open_file(&disk.path, disk.read_only, Error::Disk)?; // Lock the disk image to prevent other crosvm instances from using it. let lock_op = if disk.read_only { FlockOperation::LockShared @@ -1310,17 +1325,7 @@ fn create_pmem_device( index: usize, pmem_device_tube: Tube, ) -> DeviceResult { - // Special case '/proc/self/fd/*' paths. The FD is already open, just use it. - let fd: File = if disk.path.parent() == Some(Path::new("/proc/self/fd")) { - // Safe because we will validate |raw_fd|. - unsafe { File::from_raw_descriptor(raw_descriptor_from_path(&disk.path)?) } - } else { - OpenOptions::new() - .read(true) - .write(!disk.read_only) - .open(&disk.path) - .map_err(|e| Error::Disk(disk.path.to_path_buf(), e))? - }; + let fd = open_file(&disk.path, disk.read_only, Error::Disk)?; let arena_size = { let metadata = @@ -1882,16 +1887,21 @@ fn add_crosvm_user_to_jail(jail: &mut Minijail, feature: &str) -> Result { }) } -fn raw_descriptor_from_path(path: &Path) -> Result { - if !path.is_file() { - return Err(Error::InvalidFdPath); +/// If the given path is of the form /proc/self/fd/N for some N, returns `Ok(Some(N))`. Otherwise +/// returns `Ok(None`). +fn raw_descriptor_from_path(path: &Path) -> Result> { + if path.parent() == Some(Path::new("/proc/self/fd")) { + let raw_descriptor = path + .file_name() + .and_then(|fd_osstr| fd_osstr.to_str()) + .and_then(|fd_str| fd_str.parse::().ok()) + .ok_or(Error::InvalidFdPath)?; + validate_raw_descriptor(raw_descriptor) + .map_err(Error::ValidateRawDescriptor) + .map(Some) + } else { + Ok(None) } - let raw_descriptor = path - .file_name() - .and_then(|fd_osstr| fd_osstr.to_str()) - .and_then(|fd_str| fd_str.parse::().ok()) - .ok_or(Error::InvalidFdPath)?; - validate_raw_descriptor(raw_descriptor).map_err(Error::ValidateRawDescriptor) } trait IntoUnixStream { @@ -1900,9 +1910,9 @@ trait IntoUnixStream { impl<'a> IntoUnixStream for &'a Path { fn into_unix_stream(self) -> Result { - if self.parent() == Some(Path::new("/proc/self/fd")) { + if let Some(raw_descriptor) = raw_descriptor_from_path(self)? { // Safe because we will validate |raw_fd|. - unsafe { Ok(UnixStream::from_raw_fd(raw_descriptor_from_path(self)?)) } + unsafe { Ok(UnixStream::from_raw_fd(raw_descriptor)) } } else { UnixStream::connect(self).map_err(Error::InputEventsOpen) } @@ -2365,18 +2375,18 @@ where fn setup_vm_components(cfg: &Config) -> Result { let initrd_image = if let Some(initrd_path) = &cfg.initrd_path { - Some(File::open(initrd_path).map_err(|e| Error::OpenInitrd(initrd_path.clone(), e))?) + Some(open_file(initrd_path, true, Error::OpenInitrd)?) } else { None }; let vm_image = match cfg.executable_path { - Some(Executable::Kernel(ref kernel_path)) => VmImage::Kernel( - File::open(kernel_path).map_err(|e| Error::OpenKernel(kernel_path.to_path_buf(), e))?, - ), - Some(Executable::Bios(ref bios_path)) => VmImage::Bios( - File::open(bios_path).map_err(|e| Error::OpenBios(bios_path.to_path_buf(), e))?, - ), + Some(Executable::Kernel(ref kernel_path)) => { + VmImage::Kernel(open_file(kernel_path, true, Error::OpenKernel)?) + } + Some(Executable::Bios(ref bios_path)) => { + VmImage::Bios(open_file(bios_path, true, Error::OpenBios)?) + } _ => panic!("Did not receive a bios or kernel, should be impossible."), };