diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b490f36e44..d93da9b6836 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Blocks of changes will separated by version increments. Special thanks to @YaronWittenstein @penberg for their contributions. +- [#643](https://github.com/wasmerio/wasmer/issues/643) Implement `wasi::path_symlink` and improve WASI FS public api IO error reporting - [#608](https://github.com/wasmerio/wasmer/issues/608) Implement wasi syscalls `fd_allocate`, `fd_sync`, `fd_pread`, `path_link`, `path_filestat_set_times`; update WASI fs API in a WIP way; reduce coupling of WASI code to host filesystem; make debug messages from WASI more readable; improve rights-checking when calling syscalls; implement reference counting on inodes; misc bug fixes and improvements - [#616](https://github.com/wasmerio/wasmer/issues/616) Create the import object separately from instance instantiation in `runtime-c-api` - [#620](https://github.com/wasmerio/wasmer/issues/620) Replace one `throw()` with `noexcept` in llvm backend diff --git a/lib/wasi-tests/tests/wasitests/mod.rs b/lib/wasi-tests/tests/wasitests/mod.rs index a4830aa51f1..c9309aea072 100644 --- a/lib/wasi-tests/tests/wasitests/mod.rs +++ b/lib/wasi-tests/tests/wasitests/mod.rs @@ -16,6 +16,7 @@ mod fseek; mod hello; mod mapdir; mod path_link; +mod path_symlink; mod quine; mod readlink; mod wasi_sees_virtual_root; diff --git a/lib/wasi-tests/tests/wasitests/path_symlink.rs b/lib/wasi-tests/tests/wasitests/path_symlink.rs new file mode 100644 index 00000000000..43f0836bf0a --- /dev/null +++ b/lib/wasi-tests/tests/wasitests/path_symlink.rs @@ -0,0 +1,20 @@ +#[test] +fn test_path_symlink() { + assert_wasi_output!( + "../../wasitests/path_symlink.wasm", + "path_symlink", + vec![], + vec![ + ( + "temp".to_string(), + ::std::path::PathBuf::from("wasitests/test_fs/temp") + ), + ( + "hamlet".to_string(), + ::std::path::PathBuf::from("wasitests/test_fs/hamlet") + ), + ], + vec![], + "../../wasitests/path_symlink.out" + ); +} diff --git a/lib/wasi-tests/wasitests/path_symlink.out b/lib/wasi-tests/wasitests/path_symlink.out new file mode 100644 index 00000000000..085a9281695 --- /dev/null +++ b/lib/wasi-tests/wasitests/path_symlink.out @@ -0,0 +1,4 @@ +ACT III +SCENE I. A room in the castle. + + Enter KING CLAUDIUS, diff --git a/lib/wasi-tests/wasitests/path_symlink.rs b/lib/wasi-tests/wasitests/path_symlink.rs new file mode 100644 index 00000000000..2c5a71c8550 --- /dev/null +++ b/lib/wasi-tests/wasitests/path_symlink.rs @@ -0,0 +1,30 @@ +// Args: +// mapdir: temp:wasitests/test_fs/temp +// mapdir: hamlet:wasitests/test_fs/hamlet + +use std::fs; +use std::io::{Read, Seek, SeekFrom}; +use std::path::PathBuf; + +fn main() { + #[cfg(not(target_os = "wasi"))] + let mut base = PathBuf::from("wasitests/test_fs"); + #[cfg(target_os = "wasi")] + let mut base = PathBuf::from("/"); + + let symlink_loc = base.join("temp/act3"); + let symlink_target = "../hamlet/act3"; + let scene1 = symlink_loc.join("scene1.txt"); + + std::fs::soft_link(&symlink_target, &symlink_loc); + + let mut file = fs::File::open(&scene1).expect("Could not open file"); + + let mut buffer = [0u8; 64]; + + assert_eq!(file.read(&mut buffer).unwrap(), 64); + let str_val = std::str::from_utf8(&buffer[..]).unwrap(); + println!("{}", str_val); + + std::fs::remove_file(symlink_loc).unwrap(); +} diff --git a/lib/wasi-tests/wasitests/path_symlink.wasm b/lib/wasi-tests/wasitests/path_symlink.wasm new file mode 100755 index 00000000000..3a773eaedd8 Binary files /dev/null and b/lib/wasi-tests/wasitests/path_symlink.wasm differ diff --git a/lib/wasi/src/state/mod.rs b/lib/wasi/src/state/mod.rs index b74dc927c9a..fb1c4f63ac8 100644 --- a/lib/wasi/src/state/mod.rs +++ b/lib/wasi/src/state/mod.rs @@ -393,8 +393,11 @@ impl WasiFs { let path: &Path = Path::new(path); let mut cur_inode = base_dir.inode; + let n_components = path.components().count(); // TODO: rights checks - 'path_iter: for component in path.components() { + 'path_iter: for (i, component) in path.components().enumerate() { + // used to terminate symlink resolution properly + let last_component = i + 1 == n_components; // for each component traverse file structure // loading inodes as necessary 'symlink_resolution: while symlink_count < MAX_SYMLINKS { @@ -430,7 +433,6 @@ impl WasiFs { cd.push(component); cd }; - // TODO: verify this returns successfully when given a non-symlink let metadata = file.symlink_metadata().ok().ok_or(__WASI_EINVAL)?; let file_type = metadata.file_type(); // we want to insert newly opened dirs and files, but not transient symlinks @@ -488,6 +490,7 @@ impl WasiFs { cur_inode = new_inode; if loop_for_symlink && follow_symlinks { + debug!("Following symlink to {:?}", cur_inode); continue 'symlink_resolution; } } @@ -530,6 +533,7 @@ impl WasiFs { base.push(relative_path); base.to_string_lossy().to_string() }; + debug!("Following symlink recursively"); let symlink_inode = self.get_inode_at_path_inner( new_base_dir, &new_path, @@ -537,7 +541,15 @@ impl WasiFs { follow_symlinks, )?; cur_inode = symlink_inode; - //continue 'symlink_resolution; + // if we're at the very end and we found a file, then we're done + // TODO: figure out if this should also happen for directories? + if let Kind::File { .. } = &self.inodes[cur_inode].kind { + // check if on last step + if last_component { + break 'symlink_resolution; + } + } + continue 'symlink_resolution; } } break 'symlink_resolution; @@ -571,6 +583,64 @@ impl WasiFs { Err(__WASI_EINVAL) // this may not make sense } + // if this is still dead code and the year is 2020 or later, please delete this function + #[allow(dead_code)] + pub(crate) fn path_relative_to_fd( + &self, + fd: __wasi_fd_t, + inode: Inode, + ) -> Result { + let mut stack = vec![]; + let base_fd = self.get_fd(fd)?; + let base_inode = base_fd.inode; + let mut cur_inode = inode; + + while cur_inode != base_inode { + stack.push(self.inodes[cur_inode].name.clone()); + match &self.inodes[cur_inode].kind { + Kind::Dir { parent, .. } => { + if let Some(p) = parent { + cur_inode = *p; + } + } + _ => return Err(__WASI_EINVAL), + } + } + + let mut out = PathBuf::new(); + for p in stack.iter().rev() { + out.push(p); + } + Ok(out) + } + + /// finds the number of directories between the fd and the inode if they're connected + /// expects inode to point to a directory + pub(crate) fn path_depth_from_fd( + &self, + fd: __wasi_fd_t, + inode: Inode, + ) -> Result { + let mut counter = 0; + let base_fd = self.get_fd(fd)?; + let base_inode = base_fd.inode; + let mut cur_inode = inode; + + while cur_inode != base_inode { + counter += 1; + match &self.inodes[cur_inode].kind { + Kind::Dir { parent, .. } => { + if let Some(p) = parent { + cur_inode = *p; + } + } + _ => return Err(__WASI_EINVAL), + } + } + + Ok(counter) + } + /// gets a host file from a base directory and a path /// this function ensures the fs remains sandboxed // NOTE: follow symlinks is super weird right now @@ -704,6 +774,24 @@ impl WasiFs { })) } + /// creates an inode and inserts it given a Kind, does not assume the file exists to + pub fn create_inode_with_default_stat( + &mut self, + kind: Kind, + is_preopened: bool, + name: String, + ) -> Inode { + let mut stat = __wasi_filestat_t::default(); + stat.st_ino = self.get_next_inode_index(); + + self.inodes.insert(InodeVal { + stat, + is_preopened, + name, + kind, + }) + } + pub fn create_fd( &mut self, rights: __wasi_rights_t, diff --git a/lib/wasi/src/state/types.rs b/lib/wasi/src/state/types.rs index 5390d6a8046..57ee29fdd6a 100644 --- a/lib/wasi/src/state/types.rs +++ b/lib/wasi/src/state/types.rs @@ -8,7 +8,7 @@ use std::{ }; /// Error type for external users -#[derive(Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] #[allow(dead_code)] // dead code beacuse this is for external use pub enum WasiFsError { @@ -23,6 +23,38 @@ pub enum WasiFsError { /// Something failed when doing IO. These errors can generally not be handled. /// It may work if tried again. IOError, + /// The address was in use + AddressInUse, + /// The address could not be found + AddressNotAvailable, + /// A pipe was closed + BrokenPipe, + /// The connection was aborted + ConnectionAborted, + /// The connection request was refused + ConnectionRefused, + /// The connection was reset + ConnectionReset, + /// The operation was interrupted before it could finish + Interrupted, + /// Invalid internal data, if the argument data is invalid, use `InvalidInput` + InvalidData, + /// The provided data is invalid + InvalidInput, + /// Could not perform the operation because there was not an open connection + NotConnected, + /// The requested file or directory could not be found + EntityNotFound, + /// Caller was not allowed to perform this operation + PermissionDenied, + /// The operation did not complete within the given amount of time + TimedOut, + /// Found EOF when EOF was not expected + UnexpectedEof, + /// Operation would block, this error lets the caller know that they can try again + WouldBlock, + /// A call to write returned 0 + WriteZero, /// A WASI error without an external name. If you encounter this it means /// that there's probably a bug on our side (maybe as simple as forgetting to wrap /// this error, but perhaps something broke) @@ -35,9 +67,51 @@ impl WasiFsError { __WASI_EBADF => WasiFsError::InvalidFd, __WASI_EEXIST => WasiFsError::AlreadyExists, __WASI_EIO => WasiFsError::IOError, + __WASI_EADDRINUSE => WasiFsError::AddressInUse, + __WASI_EADDRNOTAVAIL => WasiFsError::AddressNotAvailable, + __WASI_EPIPE => WasiFsError::BrokenPipe, + __WASI_ECONNABORTED => WasiFsError::ConnectionAborted, + __WASI_ECONNREFUSED => WasiFsError::ConnectionRefused, + __WASI_ECONNRESET => WasiFsError::ConnectionReset, + __WASI_EINTR => WasiFsError::Interrupted, + __WASI_EINVAL => WasiFsError::InvalidInput, + __WASI_ENOTCONN => WasiFsError::NotConnected, + __WASI_ENOENT => WasiFsError::EntityNotFound, + __WASI_EPERM => WasiFsError::PermissionDenied, + __WASI_ETIMEDOUT => WasiFsError::TimedOut, + __WASI_EPROTO => WasiFsError::UnexpectedEof, + __WASI_EAGAIN => WasiFsError::WouldBlock, + __WASI_ENOSPC => WasiFsError::WriteZero, _ => WasiFsError::UnknownError(err), } } + + pub fn into_wasi_err(self) -> __wasi_errno_t { + match self { + WasiFsError::AlreadyExists => __WASI_EEXIST, + WasiFsError::AddressInUse => __WASI_EADDRINUSE, + WasiFsError::AddressNotAvailable => __WASI_EADDRNOTAVAIL, + WasiFsError::BaseNotDirectory => __WASI_ENOTDIR, + WasiFsError::BrokenPipe => __WASI_EPIPE, + WasiFsError::ConnectionAborted => __WASI_ECONNABORTED, + WasiFsError::ConnectionRefused => __WASI_ECONNREFUSED, + WasiFsError::ConnectionReset => __WASI_ECONNRESET, + WasiFsError::Interrupted => __WASI_EINTR, + WasiFsError::InvalidData => __WASI_EIO, + WasiFsError::InvalidFd => __WASI_EBADF, + WasiFsError::InvalidInput => __WASI_EINVAL, + WasiFsError::IOError => __WASI_EIO, + WasiFsError::NotAFile => __WASI_EINVAL, + WasiFsError::NotConnected => __WASI_ENOTCONN, + WasiFsError::EntityNotFound => __WASI_ENOENT, + WasiFsError::PermissionDenied => __WASI_EPERM, + WasiFsError::TimedOut => __WASI_ETIMEDOUT, + WasiFsError::UnexpectedEof => __WASI_EPROTO, + WasiFsError::WouldBlock => __WASI_EAGAIN, + WasiFsError::WriteZero => __WASI_ENOSPC, + WasiFsError::UnknownError(ec) => ec, + } + } } /// This trait relies on your file closing when it goes out of scope via `Drop` @@ -68,20 +142,20 @@ pub trait WasiFile: std::fmt::Debug + Write + Read + Seek { /// Change the size of the file, if the `new_size` is greater than the current size /// the extra bytes will be allocated and zeroed // TODO: stablize this in 0.7.0 by removing default impl - fn set_len(&mut self, _new_size: __wasi_filesize_t) -> Option<()> { + fn set_len(&mut self, _new_size: __wasi_filesize_t) -> Result<(), WasiFsError> { panic!("Default implementation for compatibilty in the 0.6.X releases; this will be removed in 0.7.0. Please implement WasiFile::allocate for your type before then"); } /// Request deletion of the file // TODO: break this out into a WasiPath trait which is dynamically in Kind::File // this change can't be done until before release - fn unlink(&mut self) -> Option<()> { + fn unlink(&mut self) -> Result<(), WasiFsError> { panic!("Default implementation for compatibilty in the 0.6.X releases; this will be removed in 0.7.0. Please implement WasiFile::unlink for your type before then"); } /// Store file contents and metadata to disk // TODO: stablize this in 0.7.0 by removing default impl - fn sync_to_disk(&self) -> Option<()> { + fn sync_to_disk(&self) -> Result<(), WasiFsError> { panic!("Default implementation for compatibilty in the 0.6.X releases; this will be removed in 0.7.0. Please implement WasiFile::sync_to_disk for your type before then"); } } @@ -187,15 +261,42 @@ impl WasiFile for HostFile { self.metadata().len() } - fn set_len(&mut self, new_size: __wasi_filesize_t) -> Option<()> { - fs::File::set_len(&self.inner, new_size).ok() + fn set_len(&mut self, new_size: __wasi_filesize_t) -> Result<(), WasiFsError> { + fs::File::set_len(&self.inner, new_size).map_err(Into::into) } - fn unlink(&mut self) -> Option<()> { - std::fs::remove_file(&self.host_path).ok() + fn unlink(&mut self) -> Result<(), WasiFsError> { + std::fs::remove_file(&self.host_path).map_err(Into::into) + } + fn sync_to_disk(&self) -> Result<(), WasiFsError> { + self.inner.sync_all().map_err(Into::into) } - fn sync_to_disk(&self) -> Option<()> { - self.inner.sync_all().ok() +} + +impl From for WasiFsError { + fn from(io_error: io::Error) -> Self { + match io_error.kind() { + io::ErrorKind::AddrInUse => WasiFsError::AddressInUse, + io::ErrorKind::AddrNotAvailable => WasiFsError::AddressNotAvailable, + io::ErrorKind::AlreadyExists => WasiFsError::AlreadyExists, + io::ErrorKind::BrokenPipe => WasiFsError::BrokenPipe, + io::ErrorKind::ConnectionAborted => WasiFsError::ConnectionAborted, + io::ErrorKind::ConnectionRefused => WasiFsError::ConnectionRefused, + io::ErrorKind::ConnectionReset => WasiFsError::ConnectionReset, + io::ErrorKind::Interrupted => WasiFsError::Interrupted, + io::ErrorKind::InvalidData => WasiFsError::InvalidData, + io::ErrorKind::InvalidInput => WasiFsError::InvalidInput, + io::ErrorKind::NotConnected => WasiFsError::NotConnected, + io::ErrorKind::NotFound => WasiFsError::EntityNotFound, + io::ErrorKind::PermissionDenied => WasiFsError::PermissionDenied, + io::ErrorKind::TimedOut => WasiFsError::TimedOut, + io::ErrorKind::UnexpectedEof => WasiFsError::UnexpectedEof, + io::ErrorKind::WouldBlock => WasiFsError::WouldBlock, + io::ErrorKind::WriteZero => WasiFsError::WriteZero, + io::ErrorKind::Other => WasiFsError::IOError, + // if the following triggers, a new error type was added to this non-exhaustive enum + _ => WasiFsError::UnknownError(__WASI_EIO), + } } } diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs index 422b67dcee8..0000cb6d6b3 100644 --- a/lib/wasi/src/syscalls/mod.rs +++ b/lib/wasi/src/syscalls/mod.rs @@ -10,7 +10,7 @@ use crate::{ ptr::{Array, WasmPtr}, state::{ self, host_file_type_to_wasi_file_type, Fd, HostFile, Inode, InodeVal, Kind, WasiFile, - WasiState, MAX_SYMLINKS, + WasiFsError, WasiState, MAX_SYMLINKS, }, ExitCode, }; @@ -338,7 +338,7 @@ pub fn fd_allocate( match &mut state.fs.inodes[inode].kind { Kind::File { handle, .. } => { if let Some(handle) = handle { - wasi_try!(handle.set_len(new_size), __WASI_EIO); + wasi_try!(handle.set_len(new_size).map_err(WasiFsError::into_wasi_err)); } else { return __WASI_EBADF; } @@ -549,7 +549,7 @@ pub fn fd_filestat_set_size( match &mut state.fs.inodes[inode].kind { Kind::File { handle, .. } => { if let Some(handle) = handle { - wasi_try!(handle.set_len(st_size), __WASI_EIO); + wasi_try!(handle.set_len(st_size).map_err(WasiFsError::into_wasi_err)); } else { return __WASI_EBADF; } @@ -1159,7 +1159,7 @@ pub fn fd_sync(ctx: &mut Ctx, fd: __wasi_fd_t) -> __wasi_errno_t { match &mut state.fs.inodes[inode].kind { Kind::File { handle, .. } => { if let Some(h) = handle { - wasi_try!(h.sync_to_disk(), __WASI_EIO); + wasi_try!(h.sync_to_disk().map_err(WasiFsError::into_wasi_err)); } else { return __WASI_EINVAL; } @@ -1679,6 +1679,10 @@ pub fn path_open( dirflags & __WASI_LOOKUP_SYMLINK_FOLLOW != 0, ); + if let Ok(m) = maybe_inode { + dbg!(&state.fs.inodes[m]); + } + // TODO: traverse rights of dirs properly // COMMENTED OUT: WASI isn't giving appropriate rights here when opening // TODO: look into this; file a bug report if this is a bug @@ -1935,6 +1939,20 @@ pub fn path_rename( debug!("wasi::path_rename"); unimplemented!("wasi::path_rename") } + +/// ### `path_symlink()` +/// Create a symlink +/// Inputs: +/// - `const char *old_path` +/// Array of UTF-8 bytes representing the source path +/// - `u32 old_path_len` +/// The number of bytes to read from `old_path` +/// - `__wasi_fd_t fd` +/// The base directory from which the paths are understood +/// - `const char *new_path` +/// Array of UTF-8 bytes representing the target path +/// - `u32 new_path_len` +/// The number of bytes to read from `new_path` pub fn path_symlink( ctx: &mut Ctx, old_path: WasmPtr, @@ -1944,16 +1962,89 @@ pub fn path_symlink( new_path_len: u32, ) -> __wasi_errno_t { debug!("wasi::path_symlink"); - unimplemented!("wasi::path_symlink") + let state = get_wasi_state(ctx); + let memory = ctx.memory(0); + let old_path_str = wasi_try!( + old_path.get_utf8_string(memory, old_path_len), + __WASI_EINVAL + ); + let new_path_str = wasi_try!( + new_path.get_utf8_string(memory, new_path_len), + __WASI_EINVAL + ); + let base_fd = wasi_try!(state.fs.get_fd(fd)); + if !has_rights(base_fd.rights, __WASI_RIGHT_PATH_SYMLINK) { + return __WASI_EACCES; + } + + // get the depth of the parent + 1 (UNDER INVESTIGATION HMMMMMMMM THINK FISH ^ THINK FISH) + let old_path_path = std::path::Path::new(old_path_str); + let (source_inode, _) = wasi_try!(state.fs.get_parent_inode_at_path(fd, old_path_path, true)); + let depth = wasi_try!(state.fs.path_depth_from_fd(fd, source_inode)) - 1; + + let new_path_path = std::path::Path::new(new_path_str); + let (target_parent_inode, entry_name) = + wasi_try!(state.fs.get_parent_inode_at_path(fd, new_path_path, true)); + + // short circuit if anything is wrong, before we create an inode + match &state.fs.inodes[target_parent_inode].kind { + Kind::Dir { entries, .. } => { + if entries.contains_key(&entry_name) { + return __WASI_EEXIST; + } + } + Kind::Root { .. } => return __WASI_ENOTCAPABLE, + Kind::File { .. } | Kind::Symlink { .. } | Kind::Buffer { .. } => { + unreachable!("get_parent_inode_at_path returned something other than a Dir or Root") + } + } + + let mut source_path = std::path::Path::new(old_path_str); + let mut relative_path = std::path::PathBuf::new(); + for _ in 0..depth { + relative_path.push(".."); + } + relative_path.push(source_path); + debug!( + "Symlinking {} to {}", + new_path_str, + relative_path.to_string_lossy() + ); + + let kind = Kind::Symlink { + base_po_dir: fd, + path_to_symlink: std::path::PathBuf::from(new_path_str), + relative_path, + }; + let new_inode = state + .fs + .create_inode_with_default_stat(kind, false, entry_name.clone()); + + if let Kind::Dir { + ref mut entries, .. + } = &mut state.fs.inodes[target_parent_inode].kind + { + entries.insert(entry_name, new_inode); + } + + __WASI_ESUCCESS } +/// ### `path_unlink_file()` +/// Unlink a file, deleting if the number of hardlinks is 1 +/// Inputs: +/// - `__wasi_fd_t fd` +/// The base file descriptor from which the path is understood +/// - `const char *path` +/// Array of UTF-8 bytes representing the path +/// - `u32 path_len` +/// The number of bytes in the `path` array pub fn path_unlink_file( ctx: &mut Ctx, fd: __wasi_fd_t, path: WasmPtr, path_len: u32, ) -> __wasi_errno_t { - // TODO check if fd is a dir, ensure it's within sandbox, etc. debug!("wasi::path_unlink_file"); let state = get_wasi_state(ctx); let memory = ctx.memory(0); @@ -1992,7 +2083,7 @@ pub fn path_unlink_file( match &mut state.fs.inodes[removed_inode].kind { Kind::File { handle, path } => { if let Some(h) = handle { - wasi_try!(h.unlink().ok_or(__WASI_EIO)); + wasi_try!(h.unlink().map_err(WasiFsError::into_wasi_err)); } else { // File is closed // problem with the abstraction, we can't call unlink because there's no handle @@ -2000,7 +2091,11 @@ pub fn path_unlink_file( wasi_try!(std::fs::remove_file(path).map_err(|_| __WASI_EIO)); } } - _ => unimplemented!("wasi::path_unlink_file for non-files"), + Kind::Dir { .. } | Kind::Root { .. } => return __WASI_EISDIR, + Kind::Symlink { .. } => { + // TODO: actually delete real symlinks and do nothing for virtual symlinks + } + _ => unimplemented!("wasi::path_unlink_file for Buffer"), } let inode_was_removed = unsafe { state.fs.remove_inode(removed_inode) }; assert!(