From 96a2ad54015b36bed06ec6bd6400f6d144aa20be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=88=CE=BB=CE=BB=CE=B5=CE=BD=20=CE=95=CE=BC=CE=AF=CE=BB?= =?UTF-8?q?=CE=B9=CE=B1=20=CE=86=CE=BD=CE=BD=CE=B1=20Zscheile?= Date: Wed, 11 Feb 2026 15:53:38 +0100 Subject: [PATCH 1/2] feat(isolation/filemap): Use proper tree for all file lookups --- src/hypercall.rs | 67 +++-- src/isolation/fd.rs | 3 +- src/isolation/filemap.rs | 440 ------------------------------ src/isolation/filemap/mod.rs | 165 +++++++++++ src/isolation/filemap/tests.rs | 229 ++++++++++++++++ src/isolation/filemap/tree.rs | 391 ++++++++++++++++++++++++++ src/vm.rs | 6 +- uhyve-interface/src/parameters.rs | 2 +- 8 files changed, 837 insertions(+), 466 deletions(-) delete mode 100644 src/isolation/filemap.rs create mode 100644 src/isolation/filemap/mod.rs create mode 100644 src/isolation/filemap/tests.rs create mode 100644 src/isolation/filemap/tree.rs diff --git a/src/hypercall.rs b/src/hypercall.rs index df4065af..bbacc6da 100644 --- a/src/hypercall.rs +++ b/src/hypercall.rs @@ -15,7 +15,7 @@ use crate::{ HypervisorResult, isolation::{ fd::{FdData, GuestFd, UhyveFileDescriptorLayer}, - filemap::UhyveFileMap, + filemap::{UhyveFileMap, UhyveMapLeaf}, }, mem::MmapMemory, params::EnvVars, @@ -246,7 +246,7 @@ fn translate_last_errno() -> Option { ($($x:ident),*) => {{[ $((libc::$x, hermit_abi::errno::$x)),* ]}} } for (e_host, e_guest) in error_pairs!( - EBADF, EEXIST, EFAULT, EINVAL, EIO, EOVERFLOW, EPERM, ENOENT, EROFS + EBADF, EEXIST, EFAULT, EINVAL, EIO, EISDIR, EOVERFLOW, EPERM, ENOENT, EROFS ) { if errno == e_host { return Some(e_guest); @@ -267,18 +267,27 @@ fn translate_last_errno() -> Option { fn unlink(mem: &MmapMemory, sysunlink: &mut UnlinkParams, file_map: &mut UhyveFileMap) { let requested_path_ptr = mem.host_address(sysunlink.name).unwrap() as *const i8; let guest_path = unsafe { CStr::from_ptr(requested_path_ptr) }; - sysunlink.ret = if let Some(host_path) = file_map.get_host_path(guest_path) { - // We can safely unwrap here, as host_path.as_bytes will never contain internal \0 bytes - // As host_path_c_string is a valid CString, this implementation is presumed to be safe. - let host_path_c_string = CString::new(host_path.as_bytes()).unwrap(); - if unsafe { libc::unlink(host_path_c_string.as_c_str().as_ptr()) } < 0 { - -translate_last_errno().unwrap_or(1) - } else { + sysunlink.ret = match file_map.unlink(guest_path) { + Ok(Some(host_path)) => { + // We can safely unwrap here, as host_path.as_bytes will never contain internal \0 bytes + // As host_path_c_string is a valid CString, this implementation is presumed to be safe. + let host_path_c_string = CString::new(host_path.as_bytes()).unwrap(); + if unsafe { libc::unlink(host_path_c_string.as_c_str().as_ptr()) } < 0 { + -translate_last_errno().unwrap_or(1) + } else { + 0 + } + } + Ok(None) => { + // Removed virtual entry 0 } - } else { - error!("The kernel requested to unlink() an unknown path ({guest_path:?}): Rejecting..."); - -ENOENT + Err(()) => { + error!( + "The kernel requested to unlink() an unknown path ({guest_path:?}): Rejecting..." + ); + -ENOENT + } }; } @@ -319,10 +328,25 @@ fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFileMap) sysopen.ret = if let Some(host_path) = file_map.get_host_path(guest_path) { debug!("{guest_path:#?} found in file map."); - // We can safely unwrap here, as host_path.as_bytes will never contain internal \0 bytes - // As host_path_c_string is a valid CString, this implementation is presumed to be safe. - let host_path_c_string = CString::new(host_path.as_bytes()).unwrap(); - do_open(&mut file_map.fdmap, host_path_c_string, flags, sysopen.mode) + match host_path { + UhyveMapLeaf::OnHost(host_path) => { + // We can safely unwrap here, as host_path.as_bytes will never contain internal \0 bytes + // As host_path_c_string is a valid CString, this implementation is presumed to be safe. + let host_path_c_string = CString::new(host_path.as_os_str().as_bytes()).unwrap(); + do_open(&mut file_map.fdmap, host_path_c_string, flags, sysopen.mode) + } + UhyveMapLeaf::Virtual(data) => { + if let Some(guest_fd) = file_map.fdmap.insert(FdData::Virtual { + // The following only clones a pointer, and increases an `Arc` refcount. + data: data.clone(), + offset: 0, + }) { + guest_fd.0 + } else { + -ENOENT + } + } + } } else { debug!("{guest_path:#?} not found in file map."); if (flags & O_CREAT) == O_CREAT { @@ -336,8 +360,15 @@ fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFileMap) flags |= file_map.get_io_mode_flags(); } - let host_path_c_string = file_map.create_temporary_file(guest_path); - do_open(&mut file_map.fdmap, host_path_c_string, flags, sysopen.mode) + match file_map.create_temporary_file(guest_path) { + Some(host_path_c_string) => { + do_open(&mut file_map.fdmap, host_path_c_string, flags, sysopen.mode) + } + None => { + debug!("Returning -EINVAL for {guest_path:#?}"); + -EINVAL + } + } } else { debug!("Returning -ENOENT for {guest_path:#?}"); -ENOENT diff --git a/src/isolation/fd.rs b/src/isolation/fd.rs index 11d447cc..2453892a 100644 --- a/src/isolation/fd.rs +++ b/src/isolation/fd.rs @@ -47,8 +47,7 @@ pub enum FdData { /// A host file descriptor Raw(RawFd), - #[allow(dead_code)] - /// An in-memory slice (possibly mmap-ed) + /// An in-memory slice. /// /// SAFETY: It is not allowed for `data` to point into guest memory. Virtual { data: Arc<[u8]>, offset: u64 }, diff --git a/src/isolation/filemap.rs b/src/isolation/filemap.rs deleted file mode 100644 index 3ad5aba1..00000000 --- a/src/isolation/filemap.rs +++ /dev/null @@ -1,440 +0,0 @@ -use std::{ - collections::HashMap, - ffi::{CStr, CString, OsStr, OsString}, - fs::{canonicalize, metadata}, - os::unix::ffi::OsStrExt, - path::{Path, PathBuf}, -}; - -use clean_path::clean; -#[cfg(target_os = "linux")] -use libc::{O_DIRECT, O_SYNC}; -use tempfile::TempDir; -use uuid::Uuid; - -use crate::isolation::{ - fd::UhyveFileDescriptorLayer, split_guest_and_host_path, tempdir::create_temp_dir, -}; - -/// Defines cache-related behaviors that will be forced upon [`crate::hypercall::open`], -/// primarily useful for e.g. I/O benchmarking. -#[cfg(target_os = "linux")] -#[derive(Clone, Copy, Debug, Default, PartialEq)] -pub struct UhyveIoMode { - /// Append the O_DIRECT flag to bypass the host's page cache. - direct: bool, - /// Append the O_DIRECT flag to bypass the host's page cache and block until writes are finished on the host. - sync: bool, -} - -#[cfg(target_os = "linux")] -impl From> for UhyveIoMode { - fn from(s: Option) -> Self { - let (prefix, flags) = s - .unwrap_or_default() - .to_lowercase() - .split_once("=") - .map(|(prefix, flags)| (prefix.to_string(), flags.to_string())) - .unwrap_or_default(); - let flags: Vec<_> = flags.split(',').collect(); - match prefix.as_str() { - "host" => { - let direct = flags.contains(&"direct"); - let sync = flags.contains(&"sync"); - UhyveIoMode { direct, sync } - } - "" => UhyveIoMode { - direct: false, - sync: false, - }, - _ => unimplemented!(), - } - } -} - -/// Wrapper around a `HashMap` to map guest paths to arbitrary host paths and track file descriptors. -#[derive(Debug)] -pub struct UhyveFileMap { - files: HashMap, - tempdir: TempDir, - pub fdmap: UhyveFileDescriptorLayer, - #[cfg(target_os = "linux")] - iomode: UhyveIoMode, -} - -impl UhyveFileMap { - /// Creates a UhyveFileMap. - /// - /// * `mappings` - A list of host->guest path mappings with the format "./host_path.txt:guest.txt" - /// * `tempdir` - Path to create temporary directory on - pub fn new( - mappings: &[String], - tempdir: Option, - #[cfg(target_os = "linux")] iomode: UhyveIoMode, - ) -> UhyveFileMap { - let fm = UhyveFileMap { - files: mappings - .iter() - .map(String::as_str) - .map(split_guest_and_host_path) - .map(Result::unwrap) - .collect(), - tempdir: create_temp_dir(tempdir), - fdmap: UhyveFileDescriptorLayer::default(), - #[cfg(target_os = "linux")] - iomode, - }; - assert_eq!( - fm.files.len(), - mappings.len(), - "Error when creating filemap. Are duplicate paths present?" - ); - fm - } - - /// Returns the host_path on the host filesystem given a requested guest_path, if it exists. - /// - /// * `guest_path` - The guest path that is to be looked up in the map. - pub fn get_host_path(&self, guest_path: &CStr) -> Option { - // TODO: Replace clean-path in favor of Path::normalize_lexically, which has not - // been implemented yet. See: https://github.com/rust-lang/libs-team/issues/396 - let guest_pathbuf = clean(OsStr::from_bytes(guest_path.to_bytes())); - if let Some(host_path) = self.files.get(&guest_pathbuf) { - let host_path = OsString::from(host_path); - trace!("get_host_path (host_path): {host_path:#?}"); - Some(host_path) - } else { - debug!("Guest requested to open a path that was not mapped."); - if self.files.is_empty() { - debug!("UhyveFileMap is empty, returning None..."); - return None; - } - - if let Some(parent_of_guest_path) = guest_pathbuf.parent() { - debug!("The file is in a child directory, searching for a parent directory..."); - for searched_parent_guest in parent_of_guest_path.ancestors() { - // If one of the guest paths' parent directories (parent_host) is mapped, - // use the mapped host path and push the "remainder" (the path's components - // that come after the mapped guest path) onto the host path. - if let Some(parent_host) = self.files.get(searched_parent_guest) { - let mut host_path = PathBuf::from(parent_host); - let guest_path_remainder = - guest_pathbuf.strip_prefix(searched_parent_guest).unwrap(); - host_path.push(guest_path_remainder); - - // Handles symbolic links. - return canonicalize(&host_path) - .map_or(host_path.into_os_string(), PathBuf::into_os_string) - .into(); - } - } - } - debug!("The file is not in a child directory, returning None..."); - None - } - } - - /// Returns an array of all host paths (for Landlock). - #[cfg(target_os = "linux")] - pub(crate) fn get_all_host_paths(&self) -> impl Iterator { - self.files.values().map(|i| i.as_os_str()) - } - - /// Returns an iterator (non-unique) over all mountable guest directories. - pub(crate) fn get_all_guest_dirs(&self) -> impl Iterator { - self.files.iter().filter_map(|(gp, hp)| { - // We check the host_path filetype, and return the parent directory for everything non-file. - if let Ok(hp_metadata) = metadata(hp) { - if hp_metadata.is_dir() { - Some(gp.as_path()) - } else if hp_metadata.is_file() { - Some(gp.as_path().parent().unwrap()) - } else if hp_metadata.is_symlink() { - error!( - "{} is a symlink. This is not supported (yet?)", - hp.display() - ); - None - } else { - Some(gp.as_path().parent().unwrap()) - } - } else if let Some(parent_path) = hp.parent() - && let Ok(parent_metadata) = metadata(parent_path) - && parent_metadata.is_dir() - { - // Parent directory exists, so this is a mounted file - Some(gp.as_path().parent().unwrap()) - } else { - error!("{} isn't a valid host path", hp.display()); - // return Err(ErrorKind::InvalidFilename); - None - } - }) - } - - /// Get flags that should be appended to [`crate::hypercall::open`] - /// as per the structure's defined I/O mode. - #[inline] - #[cfg(target_os = "linux")] - pub(crate) fn get_io_mode_flags(&self) -> i32 { - let mut flags: i32 = 0; - if self.iomode.sync { - flags |= O_SYNC; - } - if self.iomode.direct { - flags |= O_DIRECT; - } - flags - } - - /// Returns the path to the temporary directory (for Landlock). - #[cfg(target_os = "linux")] - pub(crate) fn get_temp_dir(&self) -> &Path { - self.tempdir.path() - } - - /// Inserts an opened temporary file into the file map. Returns a CString so that - /// the file can be directly used by [crate::hypercall::open]. - /// - /// * `guest_path` - The requested guest path. - pub fn create_temporary_file(&mut self, guest_path: &CStr) -> CString { - let host_path = self.tempdir.path().join(Uuid::new_v4().to_string()); - trace!("create_temporary_file (host_path): {host_path:#?}"); - let ret = CString::new(host_path.as_os_str().as_bytes()).unwrap(); - self.files.insert( - PathBuf::from(OsStr::from_bytes(guest_path.to_bytes())), - host_path, - ); - ret - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_uhyvefilemap() { - // Our files are in `$CARGO_MANIFEST_DIR/data/fixtures/fs`. - let mut fixture_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - fixture_path.push("tests/data/fixtures/fs"); - assert!(fixture_path.is_dir()); - let path_prefix = fixture_path.to_str().unwrap().to_owned(); - - let map_results = [ - path_prefix.clone() + "/README.md", - path_prefix.clone() + "/this_folder_exists", - path_prefix.clone() + "/this_symlink_exists", - path_prefix.clone() + "/this_symlink_is_dangling", - path_prefix.clone() + "/this_file_does_not_exist", - // Special case: the file's corresponding parameter uses a symlink, - // which should be successfully resolved first. - path_prefix.clone() + "/this_folder_exists/file_in_folder.txt", - ]; - - let map_parameters = [ - map_results[0].clone() + ":readme_file.md", - map_results[1].clone() + ":guest_folder", - map_results[2].clone() + ":guest_symlink", - map_results[3].clone() + ":guest_dangling_symlink", - map_results[4].clone() + ":guest_file", - path_prefix.clone() + "/this_symlink_leads_to_a_file" + ":guest_file_symlink", - ]; - - let map = UhyveFileMap::new( - &map_parameters, - None, - #[cfg(target_os = "linux")] - UhyveIoMode { - direct: false, - sync: false, - }, - ); - - assert_eq!( - map.get_host_path(c"readme_file.md").unwrap(), - OsString::from(&map_results[0]) - ); - assert_eq!( - map.get_host_path(c"guest_folder").unwrap(), - OsString::from(&map_results[1]) - ); - assert_eq!( - map.get_host_path(c"guest_symlink").unwrap(), - OsString::from(&map_results[2]) - ); - assert_eq!( - map.get_host_path(c"guest_dangling_symlink").unwrap(), - OsString::from(&map_results[3]) - ); - assert_eq!( - map.get_host_path(c"guest_file").unwrap(), - OsString::from(&map_results[4]) - ); - assert_eq!( - map.get_host_path(c"guest_file_symlink").unwrap(), - OsString::from(&map_results[5]) - ); - - assert!(map.get_host_path(c"this_file_is_not_mapped").is_none()); - } - - #[test] - fn test_uhyvefilemap_directory() { - let mut fixture_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - fixture_path.push("tests/data/fixtures/fs"); - assert!(fixture_path.is_dir()); - - // Tests successful directory traversal starting from file in child - // directory of a mapped directory. - let mut guest_path_map = PathBuf::from("this_folder_exists"); - let mut host_path_map = fixture_path.clone(); - host_path_map.push("this_folder_exists"); - - let mut target_guest_path = - PathBuf::from("this_folder_exists/folder_in_folder/file_in_second_folder.txt"); - let mut target_host_path = fixture_path.clone(); - target_host_path.push(target_guest_path.clone()); - - let mut uhyvefilemap_params = [format!( - "{}:{}", - host_path_map.to_str().unwrap(), - guest_path_map.to_str().unwrap() - )]; - let mut map = UhyveFileMap::new( - &uhyvefilemap_params, - None, - #[cfg(target_os = "linux")] - UhyveIoMode { - direct: false, - sync: false, - }, - ); - - let mut found_host_path = map.get_host_path( - CString::new(target_guest_path.as_os_str().as_bytes()) - .unwrap() - .as_c_str(), - ); - - assert_eq!( - found_host_path.unwrap(), - target_host_path.as_os_str().to_str().unwrap() - ); - - // Tests successful directory traversal of the child directory. - // The pop() just removes the text file. - // guest_path.pop(); - target_host_path.pop(); - target_guest_path.pop(); - - found_host_path = map.get_host_path( - CString::new(target_guest_path.as_os_str().as_bytes()) - .unwrap() - .as_c_str(), - ); - assert_eq!( - found_host_path.unwrap(), - target_host_path.as_os_str().to_str().unwrap() - ); - - // Tests directory traversal leading to valid symbolic link with an - // empty guest_path_map. - host_path_map = fixture_path.clone(); - guest_path_map = PathBuf::from("/root"); - uhyvefilemap_params = [format!( - "{}:{}", - host_path_map.to_str().unwrap(), - guest_path_map.to_str().unwrap() - )]; - - map = UhyveFileMap::new( - &uhyvefilemap_params, - None, - #[cfg(target_os = "linux")] - UhyveIoMode { - direct: false, - sync: false, - }, - ); - - target_guest_path = PathBuf::from("/root/this_symlink_leads_to_a_file"); - target_host_path = fixture_path.clone(); - target_host_path.push("this_folder_exists/file_in_folder.txt"); - found_host_path = map.get_host_path( - CString::new(target_guest_path.as_os_str().as_bytes()) - .unwrap() - .as_c_str(), - ); - assert_eq!( - found_host_path.unwrap(), - target_host_path.as_os_str().to_str().unwrap() - ); - - // Tests directory traversal with no maps - let empty_array: [String; 0] = []; - map = UhyveFileMap::new( - &empty_array, - None, - #[cfg(target_os = "linux")] - UhyveIoMode { - direct: false, - sync: false, - }, - ); - found_host_path = map.get_host_path( - CString::new(target_guest_path.as_os_str().as_bytes()) - .unwrap() - .as_c_str(), - ); - assert!(found_host_path.is_none()); - } - - #[test] - #[cfg(target_os = "linux")] - fn test_uhyveiomode_input() { - let io_mode_str = |x: &str| UhyveIoMode::from(Some(String::from(x))); - assert_eq!( - io_mode_str(""), - UhyveIoMode { - direct: false, - sync: false - } - ); - assert_eq!( - io_mode_str("host=direct"), - UhyveIoMode { - direct: true, - sync: false - } - ); - assert_eq!( - io_mode_str("host=direct"), - UhyveIoMode { - direct: true, - sync: false - } - ); - assert_eq!( - io_mode_str("host=direct"), - UhyveIoMode { - direct: true, - sync: false - } - ); - assert_eq!( - io_mode_str("host=direct,sync"), - UhyveIoMode { - direct: true, - sync: true - } - ); - assert_eq!( - io_mode_str("host=sync,direct"), - UhyveIoMode { - direct: true, - sync: true - } - ); - } -} diff --git a/src/isolation/filemap/mod.rs b/src/isolation/filemap/mod.rs new file mode 100644 index 00000000..c38f2c1c --- /dev/null +++ b/src/isolation/filemap/mod.rs @@ -0,0 +1,165 @@ +use std::{ + ffi::{CStr, CString, OsString}, + os::unix::ffi::OsStrExt, + path::PathBuf, +}; + +#[cfg(target_os = "linux")] +use libc::{O_DIRECT, O_SYNC}; +use tempfile::TempDir; +use uuid::Uuid; + +use crate::isolation::{ + fd::UhyveFileDescriptorLayer, split_guest_and_host_path, tempdir::create_temp_dir, +}; + +mod tests; +mod tree; + +pub use tree::Leaf as UhyveMapLeaf; + +/// Defines cache-related behaviors that will be forced upon [`crate::hypercall::open`], +/// primarily useful for e.g. I/O benchmarking. +#[cfg(target_os = "linux")] +#[derive(Clone, Copy, Debug, Default, PartialEq)] +pub struct UhyveIoMode { + /// Append the O_DIRECT flag to bypass the host's page cache. + direct: bool, + /// Append the O_DIRECT flag to bypass the host's page cache and block until writes are finished on the host. + sync: bool, +} + +#[cfg(target_os = "linux")] +impl From> for UhyveIoMode { + fn from(s: Option) -> Self { + let (prefix, flags) = s + .unwrap_or_default() + .to_lowercase() + .split_once("=") + .map(|(prefix, flags)| (prefix.to_string(), flags.to_string())) + .unwrap_or_default(); + let flags: Vec<_> = flags.split(',').collect(); + match prefix.as_str() { + "host" => { + let direct = flags.contains(&"direct"); + let sync = flags.contains(&"sync"); + UhyveIoMode { direct, sync } + } + "" => UhyveIoMode { + direct: false, + sync: false, + }, + _ => unimplemented!(), + } + } +} + +/// Wrapper around a `HashMap` to map guest paths to arbitrary host paths and track file descriptors. +#[derive(Debug)] +pub struct UhyveFileMap { + root: tree::Directory, + tempdir: TempDir, + pub fdmap: UhyveFileDescriptorLayer, + #[cfg(target_os = "linux")] + iomode: UhyveIoMode, +} + +impl UhyveFileMap { + /// Creates a UhyveFileMap. + /// + /// * `mappings` - A list of host->guest path mappings with the format "./host_path.txt:guest.txt" + /// * `tempdir` - Path to create temporary directory on + pub fn new( + mappings: &[String], + tempdir: Option, + #[cfg(target_os = "linux")] iomode: UhyveIoMode, + ) -> UhyveFileMap { + let mut fm = UhyveFileMap { + root: tree::Directory::new(), + tempdir: create_temp_dir(tempdir), + fdmap: UhyveFileDescriptorLayer::default(), + #[cfg(target_os = "linux")] + iomode, + }; + for i in mappings { + let (guest_path, host_path) = split_guest_and_host_path(i.as_str()).unwrap(); + if !tree::create_leaf( + &mut fm.root, + guest_path.as_os_str().as_bytes(), + UhyveMapLeaf::OnHost(host_path), + ) { + panic!( + "Error when creating filemap @ guest_path = {guest_path:?}; Are duplicate paths present?" + ); + } + } + fm + } + + /// Returns the host_path on the host filesystem given a requested guest_path, if it exists. + /// + /// * `guest_path` - The guest path that is to be looked up in the map. + pub fn get_host_path(&self, guest_path: &CStr) -> Option { + tree::resolve_guest_path(&self.root, guest_path.to_bytes()) + } + + /// Returns an array of all host paths (for Landlock). + #[cfg(target_os = "linux")] + pub(crate) fn get_all_host_paths(&self) -> impl Iterator { + tree::get_all_host_paths(&self.root).map(|i| i.as_os_str()) + } + + /// Returns an iterator (non-unique) over all mountable guest directories. + pub(crate) fn get_all_guest_dirs(&self) -> impl Iterator { + tree::get_all_guest_dirs(&self.root) + } + + /// Get flags that should be appended to [`crate::hypercall::open`] + /// as per the structure's defined I/O mode. + #[inline] + #[cfg(target_os = "linux")] + pub(crate) fn get_io_mode_flags(&self) -> i32 { + let mut flags: i32 = 0; + if self.iomode.sync { + flags |= O_SYNC; + } + if self.iomode.direct { + flags |= O_DIRECT; + } + flags + } + + /// Returns the path to the temporary directory (for Landlock). + #[cfg(target_os = "linux")] + pub(crate) fn get_temp_dir(&self) -> &std::path::Path { + self.tempdir.path() + } + + /// Inserts a file into the file map. + /// + /// Note that this is also used for the entire setup of the uhyve file tree, + /// and this also called for the entire initial mapping. + pub fn create_leaf(&mut self, guest_path: &CStr, leaf: UhyveMapLeaf) -> bool { + tree::create_leaf(&mut self.root, guest_path.to_bytes(), leaf) + } + + /// Inserts an opened temporary file into the file map. Returns a CString so that + /// the file can be directly used by [crate::hypercall::open]. + /// + /// * `guest_path` - The requested guest path. + pub fn create_temporary_file(&mut self, guest_path: &CStr) -> Option { + let host_path = self.tempdir.path().join(Uuid::new_v4().to_string()); + trace!("create_temporary_file (host_path): {host_path:#?}"); + let ret = CString::new(host_path.as_os_str().as_bytes()).unwrap(); + if self.create_leaf(guest_path, UhyveMapLeaf::OnHost(host_path)) { + Some(ret) + } else { + None + } + } + + /// Attempt to remove a file. Note that this will fail on non-empty directories. + pub fn unlink(&mut self, guest_path: &CStr) -> Result, ()> { + tree::unlink(&mut self.root, guest_path.to_bytes()).map(|i| i.map(|j| j.into_os_string())) + } +} diff --git a/src/isolation/filemap/tests.rs b/src/isolation/filemap/tests.rs new file mode 100644 index 00000000..df480243 --- /dev/null +++ b/src/isolation/filemap/tests.rs @@ -0,0 +1,229 @@ +#![cfg(test)] + +use std::ffi::OsString; + +use super::*; + +#[test] +fn test_uhyvefilemap() { + // Our files are in `$CARGO_MANIFEST_DIR/data/fixtures/fs`. + let mut fixture_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + fixture_path.push("tests/data/fixtures/fs"); + assert!(fixture_path.is_dir()); + let path_prefix = fixture_path.to_str().unwrap().to_owned(); + + let map_results = [ + path_prefix.clone() + "/README.md", + path_prefix.clone() + "/this_folder_exists", + path_prefix.clone() + "/this_symlink_exists", + path_prefix.clone() + "/this_symlink_is_dangling", + path_prefix.clone() + "/this_file_does_not_exist", + // Special case: the file's corresponding parameter uses a symlink, + // which should be successfully resolved first. + path_prefix.clone() + "/this_folder_exists/file_in_folder.txt", + ]; + + let map_parameters = [ + map_results[0].clone() + ":readme_file.md", + map_results[1].clone() + ":guest_folder", + map_results[2].clone() + ":guest_symlink", + map_results[3].clone() + ":guest_dangling_symlink", + map_results[4].clone() + ":guest_file", + path_prefix.clone() + "/this_symlink_leads_to_a_file" + ":guest_file_symlink", + ]; + + let map = UhyveFileMap::new( + &map_parameters, + None, + #[cfg(target_os = "linux")] + UhyveIoMode { + direct: false, + sync: false, + }, + ); + + assert_eq!( + map.get_host_path(c"readme_file.md") + .unwrap() + .unwrap_on_host(), + OsString::from(&map_results[0]) + ); + assert_eq!( + map.get_host_path(c"guest_folder").unwrap().unwrap_on_host(), + OsString::from(&map_results[1]) + ); + assert_eq!( + map.get_host_path(c"guest_symlink") + .unwrap() + .unwrap_on_host(), + OsString::from(&map_results[2]) + ); + assert_eq!( + map.get_host_path(c"guest_dangling_symlink") + .unwrap() + .unwrap_on_host(), + OsString::from(&map_results[3]) + ); + assert_eq!( + map.get_host_path(c"guest_file").unwrap().unwrap_on_host(), + OsString::from(&map_results[4]) + ); + assert_eq!( + map.get_host_path(c"guest_file_symlink") + .unwrap() + .unwrap_on_host(), + OsString::from(&map_results[5]) + ); + + assert!(map.get_host_path(c"this_file_is_not_mapped").is_none()); +} + +#[test] +fn test_uhyvefilemap_directory() { + let mut fixture_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + fixture_path.push("tests/data/fixtures/fs"); + assert!(fixture_path.is_dir()); + + // Tests successful directory traversal starting from file in child + // directory of a mapped directory. + let mut guest_path_map = PathBuf::from("this_folder_exists"); + let mut host_path_map = fixture_path.clone(); + host_path_map.push("this_folder_exists"); + + let mut target_guest_path = + PathBuf::from("this_folder_exists/folder_in_folder/file_in_second_folder.txt"); + let mut target_host_path = fixture_path.clone(); + target_host_path.push(target_guest_path.clone()); + + let mut uhyvefilemap_params = [format!( + "{}:{}", + host_path_map.to_str().unwrap(), + guest_path_map.to_str().unwrap() + )]; + let mut map = UhyveFileMap::new( + &uhyvefilemap_params, + None, + #[cfg(target_os = "linux")] + UhyveIoMode { + direct: false, + sync: false, + }, + ); + + let mut found_host_path = map.get_host_path( + CString::new(target_guest_path.as_os_str().as_bytes()) + .unwrap() + .as_c_str(), + ); + + assert_eq!(found_host_path.unwrap().unwrap_on_host(), target_host_path); + + // Tests successful directory traversal of the child directory. + // The pop() just removes the text file. + // guest_path.pop(); + target_host_path.pop(); + target_guest_path.pop(); + + found_host_path = map.get_host_path( + CString::new(target_guest_path.as_os_str().as_bytes()) + .unwrap() + .as_c_str(), + ); + assert_eq!(found_host_path.unwrap().unwrap_on_host(), target_host_path); + + // Tests directory traversal leading to valid symbolic link with an + // empty guest_path_map. + host_path_map = fixture_path.clone(); + guest_path_map = PathBuf::from("/root"); + uhyvefilemap_params = [format!( + "{}:{}", + host_path_map.to_str().unwrap(), + guest_path_map.to_str().unwrap() + )]; + + map = UhyveFileMap::new( + &uhyvefilemap_params, + None, + #[cfg(target_os = "linux")] + UhyveIoMode { + direct: false, + sync: false, + }, + ); + + target_guest_path = PathBuf::from("/root/this_symlink_leads_to_a_file"); + target_host_path = fixture_path.clone(); + target_host_path.push("this_folder_exists/file_in_folder.txt"); + found_host_path = map.get_host_path( + CString::new(target_guest_path.as_os_str().as_bytes()) + .unwrap() + .as_c_str(), + ); + assert_eq!(found_host_path.unwrap().unwrap_on_host(), target_host_path); + + // Tests directory traversal with no maps + let empty_array: [String; 0] = []; + map = UhyveFileMap::new( + &empty_array, + None, + #[cfg(target_os = "linux")] + UhyveIoMode { + direct: false, + sync: false, + }, + ); + found_host_path = map.get_host_path( + CString::new(target_guest_path.as_os_str().as_bytes()) + .unwrap() + .as_c_str(), + ); + assert!(found_host_path.is_none()); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_uhyveiomode_input() { + let io_mode_str = |x: &str| UhyveIoMode::from(Some(String::from(x))); + assert_eq!( + io_mode_str(""), + UhyveIoMode { + direct: false, + sync: false + } + ); + assert_eq!( + io_mode_str("host=direct"), + UhyveIoMode { + direct: true, + sync: false + } + ); + assert_eq!( + io_mode_str("host=direct"), + UhyveIoMode { + direct: true, + sync: false + } + ); + assert_eq!( + io_mode_str("host=direct"), + UhyveIoMode { + direct: true, + sync: false + } + ); + assert_eq!( + io_mode_str("host=direct,sync"), + UhyveIoMode { + direct: true, + sync: true + } + ); + assert_eq!( + io_mode_str("host=sync,direct"), + UhyveIoMode { + direct: true, + sync: true + } + ); +} diff --git a/src/isolation/filemap/tree.rs b/src/isolation/filemap/tree.rs new file mode 100644 index 00000000..533328e1 --- /dev/null +++ b/src/isolation/filemap/tree.rs @@ -0,0 +1,391 @@ +use std::{ + collections::HashMap, + ffi::OsStr, + fmt, fs, + os::unix::ffi::OsStrExt, + path::{Path, PathBuf}, + sync::Arc, +}; + +pub(super) type Directory = HashMap, Node>; + +/// A virtual file, which can either resolve to an on-host path or a virtual read-only file. +#[derive(Clone)] +pub enum Leaf { + /// A file on the host + OnHost(PathBuf), + + /// An in-memory file + #[allow(dead_code)] + Virtual(Arc<[u8]>), +} + +impl fmt::Debug for Leaf { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::OnHost(hp) => write!(f, "OnHost({:?})", hp), + Self::Virtual(slice) => write!(f, "Virtual(len={})", slice.len()), + } + } +} + +impl Leaf { + #[cfg(test)] + pub(super) fn unwrap_on_host(self) -> PathBuf { + match self { + Self::OnHost(hp) => hp, + _ => panic!("expected on-host file path, got something else: {self:?}"), + } + } +} + +/// A virtual directory tree node, possibly with links to on-host and virtual read-only files. +#[derive(Clone, Debug)] +pub(super) enum Node { + Leaf(Leaf), + Directory(Directory), +} + +/// Returns the remainder between the end of `component` until the end of `guest_path`. +/// +/// SAFETY: `component` must be a substring of `guest_path`. +unsafe fn compute_guest_path_remainder<'a>(guest_path: &'a str, component: &'a str) -> &'a str { + // TODO: use .remainder() instead once it is stabilized. + // See: https://github.com/rust-lang/rust/issues/77998 + + let end = component.as_bytes().as_ptr_range().end; + let main_end = guest_path.as_bytes().as_ptr_range().end; + + assert!(end <= main_end); + + // SAFETY: both `start` and `end` are derived from the same allocation `guest_pathstr`. + // + // TODO: `from_utf8` can be replaced by str::from_raw_parts, once it is stabilized. + // See: https://github.com/rust-lang/rust/issues/119206 + let guest_path_remainder = core::str::from_utf8(unsafe { + core::slice::from_raw_parts(end, main_end.offset_from_unsigned(end)) + }) + .unwrap(); + + // mark guest path as relative + if let Some(guest_path_remainder) = guest_path_remainder.strip_prefix("/") { + guest_path_remainder + } else { + guest_path_remainder + } +} + +/// Clean up a path given by the guest into one that is +/// compatible with our expectations of no redundant path components +/// and without a leading slash. +fn prepare_guest_path(guest_path: &[u8]) -> Option { + // TODO: Replace clean-path in favor of Path::normalize_lexically, which has not + // been implemented yet. See: https://github.com/rust-lang/libs-team/issues/396 + // + // NOTE: Although we use `Path`/`PathBuf` here, these are not semantically correct, + // given that the meaning of a path on the guest and the host can differ. + let guest_pathbuf = clean_path::clean(OsStr::from_bytes(guest_path)); + + // Here, we expect all input paths to be valid UTF-8 strings, + // which is also what the Hermit kernel currently internally uses. + let mut guest_pathstr = match guest_pathbuf.into_os_string().into_string() { + Ok(x) => x, + Err(e) => { + debug!("prepare_guest_path {e:?}: Guest requested non-UTF-8 path, rejecting..."); + return None; + } + }; + + // Mark guest path as relative + if guest_pathstr.starts_with('/') { + guest_pathstr.remove(0); + } + + Some(guest_pathstr) +} + +fn host_path_concat_remainder(host_path: &Path, guest_path_remainder: &str) -> PathBuf { + let mut ret = host_path.to_path_buf(); + + if !guest_path_remainder.is_empty() { + ret.push(guest_path_remainder); + } + + ret +} + +/// Returns the [`Leaf`] corresponding to given a requested guest_path, if it can exist. +/// +/// * `guest_path` - The guest path that is to be looked up in the directory. +pub fn resolve_guest_path(mut this: &Directory, guest_path: &[u8]) -> Option { + let guest_pathstr = prepare_guest_path(guest_path)?; + for component in guest_pathstr.split('/') { + let leaf = match this.get(component) { + None => { + debug!( + "resolve_guest_path {guest_pathstr:?}: Guest reguested to open a path that was not mapped." + ); + return None; + } + Some(Node::Directory(subdir)) => { + this = subdir; + continue; + } + Some(Node::Leaf(leaf)) => leaf, + }; + + let guest_path_remainder = + unsafe { compute_guest_path_remainder(&guest_pathstr, component) }; + + return match leaf { + Leaf::OnHost(host_path) => { + let host_path = host_path_concat_remainder(host_path, guest_path_remainder); + // Handle symbolic links + let resolved = match fs::canonicalize(&host_path) { + Ok(x) => x, + Err(_) => host_path, + }; + + debug!("resolve_guest_path {guest_pathstr:?}: Resolved to host path {resolved:?}"); + Some(Leaf::OnHost(resolved)) + } + Leaf::Virtual(v) => { + if guest_path_remainder.is_empty() { + debug!("resolve_guest_path {guest_pathstr:?}: Resolved to virtual file"); + Some(Leaf::Virtual(v.clone())) + } else { + debug!( + "resolve_guest_path {guest_pathstr:?}: Tried to recurse into virtual file, rejecting..." + ); + None + } + } + }; + } + + None +} + +/// Returns an iterator over all host directories. +#[cfg(target_os = "linux")] +pub fn get_all_host_paths(this: &Directory) -> impl Iterator { + let mut stack = vec![this.values()]; + + core::iter::from_fn(move || { + while let Some(mut top) = stack.pop() { + match top.next() { + // NOTE: this intentionally doesn't put + // finished directories back on the stack. + None => continue, + Some(Node::Directory(dir)) => { + stack.push(dir.values()); + } + Some(Node::Leaf(Leaf::Virtual(_))) => {} + Some(Node::Leaf(Leaf::OnHost(hp))) => { + stack.push(top); + return Some(hp.as_path()); + } + } + stack.push(top); + } + None + }) +} + +/// Returns an iterator over all mountable guest directories. +pub fn get_all_guest_dirs(this: &Directory) -> impl Iterator { + let mut stack = vec![(String::new(), this.iter(), false)]; + + core::iter::from_fn(move || { + while let Some((prefix, mut top, mut marked)) = stack.pop() { + match top.next() { + None => { + // NOTE: this intentionally doesn't put + // finished directories back on the stack. + // + // It also skips the root ("/") for now. + // (because it would be impossible to mount that into Hermit anyways) + if marked && !prefix.is_empty() { + return Some(prefix); + } else { + continue; + } + } + Some((component, Node::Directory(dir))) => { + let mut new_prefix = prefix.clone(); + new_prefix.push('/'); + new_prefix.push_str(component); + stack.push((new_prefix, dir.iter(), false)); + } + Some((_, Node::Leaf(Leaf::Virtual(_)))) => { + marked = true; + } + Some((component, Node::Leaf(Leaf::OnHost(hp)))) => { + // We check the hp filetype, and return the parent directory for everything non-file. + if let Ok(hp_metadata) = fs::metadata(hp) { + if hp_metadata.is_dir() { + let mut new_prefix = prefix.clone(); + new_prefix.push('/'); + new_prefix.push_str(component); + stack.push((prefix, top, marked)); + return Some(new_prefix); + } else if hp_metadata.is_file() { + marked = true; + } else if hp_metadata.is_symlink() { + // NOTE: fs::metadata traverses symlinks. + error!("{} is an unresolvable symlink", hp.display()); + } else { + marked = true; + } + } else if let Some(parent_path) = hp.parent() + && let Ok(parent_metadata) = fs::metadata(parent_path) + && parent_metadata.is_dir() + { + // Parent directory exists, so this is a mounted file. + marked = true; + } else { + error!("{} isn't a valid host path", hp.display()); + } + } + } + stack.push((prefix, top, marked)); + } + None + }) +} + +/// Insert a file (which might be an [`Leaf::OnHost`] mount point) into the directory tree. +/// +/// This fails if any parent of the specified `guest_path` is a file or mount point instead of a directory. +pub fn create_leaf(mut this: &mut Directory, guest_path: &[u8], leaf_data: Leaf) -> bool { + use std::collections::hash_map::Entry; + + let guest_pathstr = match prepare_guest_path(guest_path) { + Some(x) => x, + None => return false, + }; + let mut it = guest_pathstr.split('/'); + let leaf = match it.next_back() { + None | Some("") => { + debug!("create_leaf invoked on empty path, rejecting..."); + return false; + } + Some(leaf) => leaf, + }; + + for component in it { + match this.entry(component.to_string().into_boxed_str()) { + Entry::Vacant(vac) => { + // Create a new directory + this = match vac.insert(Node::Directory(Directory::new())) { + Node::Directory(x) => x, + _ => unreachable!(), + }; + } + Entry::Occupied(occ) => match occ.into_mut() { + Node::Directory(subdir) => { + this = subdir; + } + _ => { + debug!( + "create_leaf {guest_pathstr:?}: Guest reguested to create a file inside of an already mapped file." + ); + return false; + } + }, + } + } + + match this.entry(leaf.to_string().into_boxed_str()) { + Entry::Vacant(vac) => { + trace!("create_leaf {guest_pathstr:?} <- {leaf_data:?}"); + vac.insert(Node::Leaf(leaf_data)); + true + } + Entry::Occupied(_) => { + debug!( + "create_leaf {guest_pathstr:?}: Guest reguested to create a file, but it already exists" + ); + false + } + } +} + +/// Remove a file or empty directory. +/// +/// Note that if `guest_path` points to a mount point (`Leaf::Onhost`) or virtual file (`Leaf::Virtual`), +/// then that entry is removed. +/// +/// TODO: Introduce proper error return type for this. +pub fn unlink(mut this: &mut Directory, guest_path: &[u8]) -> Result, ()> { + use std::collections::hash_map::Entry; + + let guest_pathstr = prepare_guest_path(guest_path).ok_or(())?; + let mut it = guest_pathstr.split('/'); + let leaf = match it.next_back() { + None | Some("") => { + debug!("unlink invoked on empty path, rejecting..."); + return Err(()); + } + Some(leaf) => leaf, + }; + + for component in it { + let leaf_data = match this.get_mut(component) { + None => { + debug!( + "unlink {guest_pathstr:?}: Guest reguested to open a path that was not mapped." + ); + return Err(()); + } + Some(Node::Directory(subdir)) => { + this = subdir; + continue; + } + Some(Node::Leaf(leaf_data)) => leaf_data, + }; + + let guest_path_remainder = + unsafe { compute_guest_path_remainder(&guest_pathstr, component) }; + + return match leaf_data { + Leaf::OnHost(host_path) => { + let host_path = host_path_concat_remainder(host_path, guest_path_remainder); + debug!("unlink {guest_pathstr:?}: Resolved to host path {host_path:?}"); + Ok(Some(host_path)) + } + Leaf::Virtual(_) => { + debug!( + "unlink {guest_pathstr:?}: Tried to recurse into virtual file, rejecting..." + ); + Err(()) + } + }; + } + + match this.entry(leaf.to_string().into_boxed_str()) { + Entry::Vacant(_) => { + trace!("unlink {guest_pathstr:?}: File not found"); + Err(()) + } + Entry::Occupied(occ) => { + let mut ret = None; + debug!("unlink {guest_pathstr:?}: Resolved to {:?}", occ.get()); + match occ.remove_entry() { + (_, Node::Leaf(Leaf::OnHost(oh))) => { + ret = Some(oh); + } + (_, Node::Leaf(Leaf::Virtual(_))) => {} + (_, Node::Directory(dir)) if dir.is_empty() => {} + (leaf, Node::Directory(dir)) => { + debug!( + "unlink {guest_pathstr:?}: Tried to unlink non-empty directory, rejecting..." + ); + this.insert(leaf, Node::Directory(dir)); + return Err(()); + } + } + Ok(ret) + } + } +} diff --git a/src/vm.rs b/src/vm.rs index 588b942e..caab6e0a 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -210,11 +210,7 @@ impl UhyveVm { #[cfg(target_os = "linux")] params.io_mode, ); - let mut mounts: Vec<_> = file_mapping - .get_all_guest_dirs() - .map(|s| s.to_str().unwrap().to_string()) - .collect(); - mounts.dedup(); + let mounts: Vec<_> = file_mapping.get_all_guest_dirs().collect(); let serial = UhyveSerial::from_params(¶ms.output)?; diff --git a/uhyve-interface/src/parameters.rs b/uhyve-interface/src/parameters.rs index ba7171d5..44e08985 100644 --- a/uhyve-interface/src/parameters.rs +++ b/uhyve-interface/src/parameters.rs @@ -3,7 +3,7 @@ pub use hermit_abi::{ O_APPEND, O_CREAT, O_DIRECTORY, O_EXCL, O_RDONLY, O_RDWR, O_TRUNC, O_WRONLY, SEEK_CUR, SEEK_END, SEEK_SET, - errno::{EBADF, EEXIST, EFAULT, EINVAL, EIO, ENOENT, EOVERFLOW, EPERM, EROFS}, + errno::{EBADF, EEXIST, EFAULT, EINVAL, EIO, EISDIR, ENOENT, EOVERFLOW, EPERM, EROFS}, }; // File operations supported by Hermit and Uhyve From 0396ef276786e5beafba05b4f1d92a2fd2b657a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=88=CE=BB=CE=BB=CE=B5=CE=BD=20=CE=95=CE=BC=CE=AF=CE=BB?= =?UTF-8?q?=CE=B9=CE=B1=20=CE=86=CE=BD=CE=BD=CE=B1=20Zscheile?= Date: Tue, 17 Feb 2026 16:07:42 +0100 Subject: [PATCH 2/2] refactor(isolation/filemap): Improve API This shifts usages of `CStr` and `OsStr` onto API consumers. All places where guest paths are used now use `&str`, and all places where host paths are used now use `&Path` (or the owned variants, respectively). --- src/hypercall.rs | 38 ++++++++++++++++++++++--------- src/isolation/filemap/mod.rs | 26 +++++++++------------ src/isolation/filemap/tests.rs | 41 +++++++++------------------------- src/vm.rs | 2 +- 4 files changed, 51 insertions(+), 56 deletions(-) diff --git a/src/hypercall.rs b/src/hypercall.rs index bbacc6da..dd890bc0 100644 --- a/src/hypercall.rs +++ b/src/hypercall.rs @@ -259,19 +259,35 @@ fn translate_last_errno() -> Option { None } +/// Decodes a guest path given at address `path_addr` in `mem`. +/// +/// # Safety +/// +/// The calling convention of hypercalls ensures that the given address doesn't alias with anything mutable. +/// The return value is only valid for the duration of the hypercall. +unsafe fn decode_guest_path(mem: &MmapMemory, path_addr: GuestPhysAddr) -> Option<&str> { + let requested_path_ptr = mem.host_address(path_addr).unwrap() as *const i8; + unsafe { CStr::from_ptr(requested_path_ptr) }.to_str().ok() +} + /// unlink deletes a name from the filesystem. This is used to handle `unlink` syscalls from the guest. /// -/// Note for when using Landlock: Unlinking files results in them being veiled. If a text +/// Note for when using Landlock: Unlinking files results in them being veiled. If a /// file (that existed during initialization) called `log.txt` is unlinked, attempting to /// open `log.txt` again will result in an error. fn unlink(mem: &MmapMemory, sysunlink: &mut UnlinkParams, file_map: &mut UhyveFileMap) { - let requested_path_ptr = mem.host_address(sysunlink.name).unwrap() as *const i8; - let guest_path = unsafe { CStr::from_ptr(requested_path_ptr) }; + let guest_path = if let Some(guest_path) = unsafe { decode_guest_path(mem, sysunlink.name) } { + guest_path + } else { + error!("The kernel requested to unlink() an non-UTF8 path: Rejecting..."); + sysunlink.ret = -EINVAL; + return; + }; sysunlink.ret = match file_map.unlink(guest_path) { Ok(Some(host_path)) => { // We can safely unwrap here, as host_path.as_bytes will never contain internal \0 bytes // As host_path_c_string is a valid CString, this implementation is presumed to be safe. - let host_path_c_string = CString::new(host_path.as_bytes()).unwrap(); + let host_path_c_string = CString::new(host_path.as_os_str().as_bytes()).unwrap(); if unsafe { libc::unlink(host_path_c_string.as_c_str().as_ptr()) } < 0 { -translate_last_errno().unwrap_or(1) } else { @@ -293,9 +309,14 @@ fn unlink(mem: &MmapMemory, sysunlink: &mut UnlinkParams, file_map: &mut UhyveFi /// Handles an open syscall by opening a file on the host. fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFileMap) { - let requested_path_ptr = mem.host_address(sysopen.name).unwrap() as *const i8; + let guest_path = if let Some(guest_path) = unsafe { decode_guest_path(mem, sysopen.name) } { + guest_path + } else { + error!("The kernel requested to open() an non-UTF8 path: Rejecting..."); + sysopen.ret = -EINVAL; + return; + }; let mut flags = sysopen.flags & ALLOWED_OPEN_FLAGS; - let guest_path = unsafe { CStr::from_ptr(requested_path_ptr) }; // See: https://lwn.net/Articles/926782/ // See: https://github.com/hermit-os/kernel/commit/71bc629 if (flags & (O_DIRECTORY | O_CREAT)) == (O_DIRECTORY | O_CREAT) { @@ -378,10 +399,7 @@ fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFileMap) /// Handles an close syscall by closing the file on the host. fn close(sysclose: &mut CloseParams, file_map: &mut UhyveFileMap) { - sysclose.ret = if file_map - .fdmap - .is_fd_present(GuestFd(sysclose.fd.into_raw_fd())) - { + sysclose.ret = if file_map.fdmap.is_fd_present(GuestFd(sysclose.fd)) { match file_map.fdmap.remove(GuestFd(sysclose.fd)) { Some(FdData::Raw(fd)) => { if unsafe { libc::close(fd) } < 0 { diff --git a/src/isolation/filemap/mod.rs b/src/isolation/filemap/mod.rs index c38f2c1c..399f94af 100644 --- a/src/isolation/filemap/mod.rs +++ b/src/isolation/filemap/mod.rs @@ -1,8 +1,4 @@ -use std::{ - ffi::{CStr, CString, OsString}, - os::unix::ffi::OsStrExt, - path::PathBuf, -}; +use std::{ffi::CString, os::unix::ffi::OsStrExt, path::PathBuf}; #[cfg(target_os = "linux")] use libc::{O_DIRECT, O_SYNC}; @@ -99,17 +95,17 @@ impl UhyveFileMap { /// Returns the host_path on the host filesystem given a requested guest_path, if it exists. /// /// * `guest_path` - The guest path that is to be looked up in the map. - pub fn get_host_path(&self, guest_path: &CStr) -> Option { - tree::resolve_guest_path(&self.root, guest_path.to_bytes()) + pub fn get_host_path(&self, guest_path: &str) -> Option { + tree::resolve_guest_path(&self.root, guest_path.as_bytes()) } /// Returns an array of all host paths (for Landlock). #[cfg(target_os = "linux")] - pub(crate) fn get_all_host_paths(&self) -> impl Iterator { - tree::get_all_host_paths(&self.root).map(|i| i.as_os_str()) + pub(crate) fn get_all_host_paths(&self) -> impl Iterator { + tree::get_all_host_paths(&self.root) } - /// Returns an iterator (non-unique) over all mountable guest directories. + /// Returns an iterator over all mountable guest directories. pub(crate) fn get_all_guest_dirs(&self) -> impl Iterator { tree::get_all_guest_dirs(&self.root) } @@ -139,15 +135,15 @@ impl UhyveFileMap { /// /// Note that this is also used for the entire setup of the uhyve file tree, /// and this also called for the entire initial mapping. - pub fn create_leaf(&mut self, guest_path: &CStr, leaf: UhyveMapLeaf) -> bool { - tree::create_leaf(&mut self.root, guest_path.to_bytes(), leaf) + pub fn create_leaf(&mut self, guest_path: &str, leaf: UhyveMapLeaf) -> bool { + tree::create_leaf(&mut self.root, guest_path.as_bytes(), leaf) } /// Inserts an opened temporary file into the file map. Returns a CString so that /// the file can be directly used by [crate::hypercall::open]. /// /// * `guest_path` - The requested guest path. - pub fn create_temporary_file(&mut self, guest_path: &CStr) -> Option { + pub fn create_temporary_file(&mut self, guest_path: &str) -> Option { let host_path = self.tempdir.path().join(Uuid::new_v4().to_string()); trace!("create_temporary_file (host_path): {host_path:#?}"); let ret = CString::new(host_path.as_os_str().as_bytes()).unwrap(); @@ -159,7 +155,7 @@ impl UhyveFileMap { } /// Attempt to remove a file. Note that this will fail on non-empty directories. - pub fn unlink(&mut self, guest_path: &CStr) -> Result, ()> { - tree::unlink(&mut self.root, guest_path.to_bytes()).map(|i| i.map(|j| j.into_os_string())) + pub fn unlink(&mut self, guest_path: &str) -> Result, ()> { + tree::unlink(&mut self.root, guest_path.as_bytes()) } } diff --git a/src/isolation/filemap/tests.rs b/src/isolation/filemap/tests.rs index df480243..8026940e 100644 --- a/src/isolation/filemap/tests.rs +++ b/src/isolation/filemap/tests.rs @@ -43,39 +43,37 @@ fn test_uhyvefilemap() { ); assert_eq!( - map.get_host_path(c"readme_file.md") + map.get_host_path("readme_file.md") .unwrap() .unwrap_on_host(), OsString::from(&map_results[0]) ); assert_eq!( - map.get_host_path(c"guest_folder").unwrap().unwrap_on_host(), + map.get_host_path("guest_folder").unwrap().unwrap_on_host(), OsString::from(&map_results[1]) ); assert_eq!( - map.get_host_path(c"guest_symlink") - .unwrap() - .unwrap_on_host(), + map.get_host_path("guest_symlink").unwrap().unwrap_on_host(), OsString::from(&map_results[2]) ); assert_eq!( - map.get_host_path(c"guest_dangling_symlink") + map.get_host_path("guest_dangling_symlink") .unwrap() .unwrap_on_host(), OsString::from(&map_results[3]) ); assert_eq!( - map.get_host_path(c"guest_file").unwrap().unwrap_on_host(), + map.get_host_path("guest_file").unwrap().unwrap_on_host(), OsString::from(&map_results[4]) ); assert_eq!( - map.get_host_path(c"guest_file_symlink") + map.get_host_path("guest_file_symlink") .unwrap() .unwrap_on_host(), OsString::from(&map_results[5]) ); - assert!(map.get_host_path(c"this_file_is_not_mapped").is_none()); + assert!(map.get_host_path("this_file_is_not_mapped").is_none()); } #[test] @@ -110,12 +108,7 @@ fn test_uhyvefilemap_directory() { }, ); - let mut found_host_path = map.get_host_path( - CString::new(target_guest_path.as_os_str().as_bytes()) - .unwrap() - .as_c_str(), - ); - + let mut found_host_path = map.get_host_path(target_guest_path.to_str().unwrap()); assert_eq!(found_host_path.unwrap().unwrap_on_host(), target_host_path); // Tests successful directory traversal of the child directory. @@ -124,11 +117,7 @@ fn test_uhyvefilemap_directory() { target_host_path.pop(); target_guest_path.pop(); - found_host_path = map.get_host_path( - CString::new(target_guest_path.as_os_str().as_bytes()) - .unwrap() - .as_c_str(), - ); + found_host_path = map.get_host_path(target_guest_path.to_str().unwrap()); assert_eq!(found_host_path.unwrap().unwrap_on_host(), target_host_path); // Tests directory traversal leading to valid symbolic link with an @@ -154,11 +143,7 @@ fn test_uhyvefilemap_directory() { target_guest_path = PathBuf::from("/root/this_symlink_leads_to_a_file"); target_host_path = fixture_path.clone(); target_host_path.push("this_folder_exists/file_in_folder.txt"); - found_host_path = map.get_host_path( - CString::new(target_guest_path.as_os_str().as_bytes()) - .unwrap() - .as_c_str(), - ); + found_host_path = map.get_host_path(target_guest_path.to_str().unwrap()); assert_eq!(found_host_path.unwrap().unwrap_on_host(), target_host_path); // Tests directory traversal with no maps @@ -172,11 +157,7 @@ fn test_uhyvefilemap_directory() { sync: false, }, ); - found_host_path = map.get_host_path( - CString::new(target_guest_path.as_os_str().as_bytes()) - .unwrap() - .as_c_str(), - ); + found_host_path = map.get_host_path(target_guest_path.to_str().unwrap()); assert!(found_host_path.is_none()); } diff --git a/src/vm.rs b/src/vm.rs index caab6e0a..7a7a781c 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -353,7 +353,7 @@ impl UhyveVm { file_sandbox_mode, kernel_path.into(), output, - host_paths, + host_paths.map(|i| i.as_os_str()), temp_dir, #[cfg(feature = "instrument")] trace,