diff --git a/lib/wasi-tests/tests/wasitests/close_preopen_fd.rs b/lib/wasi-tests/tests/wasitests/close_preopen_fd.rs new file mode 100644 index 00000000000..616ab1d6e2c --- /dev/null +++ b/lib/wasi-tests/tests/wasitests/close_preopen_fd.rs @@ -0,0 +1,14 @@ +#[test] +fn test_close_preopen_fd() { + assert_wasi_output!( + "../../wasitests/close_preopen_fd.wasm", + "close_preopen_fd", + vec![], + vec![( + "hamlet".to_string(), + ::std::path::PathBuf::from("wasitests/test_fs/hamlet") + ),], + vec![], + "../../wasitests/close_preopen_fd.out" + ); +} diff --git a/lib/wasi-tests/tests/wasitests/mod.rs b/lib/wasi-tests/tests/wasitests/mod.rs index 2849ccd5e68..e469511fdf8 100644 --- a/lib/wasi-tests/tests/wasitests/mod.rs +++ b/lib/wasi-tests/tests/wasitests/mod.rs @@ -5,6 +5,7 @@ // The _common module is not autogenerated. It provides common macros for the wasitests #[macro_use] mod _common; +mod close_preopen_fd; mod create_dir; mod envvar; mod fd_allocate; diff --git a/lib/wasi-tests/wasitests/close_preopen_fd.out b/lib/wasi-tests/wasitests/close_preopen_fd.out new file mode 100644 index 00000000000..e6e9b9f6737 --- /dev/null +++ b/lib/wasi-tests/wasitests/close_preopen_fd.out @@ -0,0 +1,3 @@ +accessing preopen fd was a success +Closing preopen fd was a success +accessing closed preopen fd was an EBADF error: true diff --git a/lib/wasi-tests/wasitests/close_preopen_fd.rs b/lib/wasi-tests/wasitests/close_preopen_fd.rs new file mode 100644 index 00000000000..ba0a875617d --- /dev/null +++ b/lib/wasi-tests/wasitests/close_preopen_fd.rs @@ -0,0 +1,44 @@ +// Args: +// mapdir: hamlet:wasitests/test_fs/hamlet + +use std::fs; +use std::path::PathBuf; + +#[cfg(target_os = "wasi")] +#[link(wasm_import_module = "wasi_unstable")] +extern "C" { + fn fd_close(fd: u32) -> u16; + fn fd_fdstat_set_flags(fd: u32, flags: u16) -> u16; +} + +const FIRST_PREOPEN_FD: u32 = 4; + +fn main() { + #[cfg(target_os = "wasi")] + { + let result = unsafe { fd_fdstat_set_flags(FIRST_PREOPEN_FD, 1 << 2) }; + println!( + "accessing preopen fd was a {}", + if result == 0 { "success" } else { "failure" } + ); + + let result = unsafe { fd_close(FIRST_PREOPEN_FD) }; + println!( + "Closing preopen fd was a {}", + if result == 0 { "success" } else { "failure" } + ); + + let result = unsafe { fd_fdstat_set_flags(FIRST_PREOPEN_FD, 1 << 2) }; + println!( + "accessing closed preopen fd was an EBADF error: {}", + if result == 8 { "true" } else { "false" } + ); + } + + #[cfg(not(target_os = "wasi"))] + { + println!("accessing preopen fd was a success"); + println!("Closing preopen fd was a success"); + println!("accessing closed preopen fd was an EBADF error: true"); + } +} diff --git a/lib/wasi-tests/wasitests/close_preopen_fd.wasm b/lib/wasi-tests/wasitests/close_preopen_fd.wasm new file mode 100755 index 00000000000..7b69083506e Binary files /dev/null and b/lib/wasi-tests/wasitests/close_preopen_fd.wasm differ diff --git a/lib/wasi/src/state/mod.rs b/lib/wasi/src/state/mod.rs index 64a5aee84ca..aff57ff9a96 100644 --- a/lib/wasi/src/state/mod.rs +++ b/lib/wasi/src/state/mod.rs @@ -664,7 +664,7 @@ impl WasiFs { // even if it's false, it still follows symlinks, just not the last // symlink so // This will be resolved when we have tests asserting the correct behavior - pub fn get_inode_at_path( + pub(crate) fn get_inode_at_path( &mut self, base: __wasi_fd_t, path: &str, @@ -675,7 +675,7 @@ impl WasiFs { /// Returns the parent Dir or Root that the file at a given path is in and the file name /// stripped off - pub fn get_parent_inode_at_path( + pub(crate) fn get_parent_inode_at_path( &mut self, base: __wasi_fd_t, path: &Path, @@ -787,7 +787,7 @@ impl WasiFs { } /// Creates an inode and inserts it given a Kind and some extra data - pub fn create_inode( + pub(crate) fn create_inode( &mut self, kind: Kind, is_preopened: bool, @@ -805,7 +805,7 @@ 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( + pub(crate) fn create_inode_with_default_stat( &mut self, kind: Kind, is_preopened: bool, @@ -849,7 +849,7 @@ impl WasiFs { /// This function is unsafe because it's the caller's responsibility to ensure that /// all refences to the given inode have been removed from the filesystem /// - /// returns true if the inode existed and was removed + /// returns the inode if it existed and was removed pub unsafe fn remove_inode(&mut self, inode: Inode) -> Option { self.inodes.remove(inode) } @@ -941,6 +941,61 @@ impl WasiFs { ..__wasi_filestat_t::default() }) } + + /// Closes an open FD, handling all details such as FD being preopen + pub(crate) fn close_fd(&mut self, fd: __wasi_fd_t) -> Result<(), __wasi_errno_t> { + let inodeval_mut = self.get_inodeval_mut(fd)?; + let is_preopened = inodeval_mut.is_preopened; + + match &mut inodeval_mut.kind { + Kind::File { ref mut handle, .. } => { + let mut empty_handle = None; + std::mem::swap(handle, &mut empty_handle); + } + Kind::Dir { parent, path, .. } => { + debug!("Closing dir {:?}", &path); + let key = path + .file_name() + .ok_or(__WASI_EINVAL)? + .to_string_lossy() + .to_string(); + if let Some(p) = parent.clone() { + match &mut self.inodes[p].kind { + Kind::Dir { entries, .. } | Kind::Root { entries } => { + self.fd_map.remove(&fd).unwrap(); + if is_preopened { + let mut idx = None; + for (i, po_fd) in self.preopen_fds.iter().enumerate() { + if *po_fd == fd { + idx = Some(i); + break; + } + } + if let Some(i) = idx { + // only remove entry properly if this is the original preopen FD + // calling `path_open` can give you an fd to the same inode as a preopen fd + entries.remove(&key); + self.preopen_fds.remove(i); + // Maybe recursively closes fds if original preopen? + } + } + } + _ => unreachable!( + "Fatal internal logic error, directory's parent is not a directory" + ), + } + } else { + // this shouldn't be possible anymore due to Root + debug!("HIT UNREACHABLE CODE! Non-root directory does not have a parent"); + return Err(__WASI_EINVAL); + } + } + Kind::Root { .. } => return Err(__WASI_EACCES), + Kind::Symlink { .. } | Kind::Buffer { .. } => return Err(__WASI_EINVAL), + } + + Ok(()) + } } #[derive(Debug, Serialize, Deserialize)] diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs index 9801d4ac048..28c7efac4b0 100644 --- a/lib/wasi/src/syscalls/mod.rs +++ b/lib/wasi/src/syscalls/mod.rs @@ -374,23 +374,11 @@ pub fn fd_allocate( /// If `fd` is invalid or not open pub fn fd_close(ctx: &mut Ctx, fd: __wasi_fd_t) -> __wasi_errno_t { debug!("wasi::fd_close"); - - let memory = ctx.memory(0); let state = get_wasi_state(ctx); - let inode_val = wasi_try!(state.fs.get_inodeval_mut(fd)); - if inode_val.is_preopened { - return __WASI_EACCES; - } - match &mut inode_val.kind { - Kind::File { ref mut handle, .. } => { - let mut empty_handle = None; - std::mem::swap(handle, &mut empty_handle); - } - Kind::Dir { .. } => return __WASI_EISDIR, - Kind::Root { .. } => return __WASI_EACCES, - Kind::Symlink { .. } | Kind::Buffer { .. } => return __WASI_EINVAL, - } + let fd_entry = wasi_try!(state.fs.get_fd(fd)).clone(); + + wasi_try!(state.fs.close_fd(fd)); __WASI_ESUCCESS } @@ -1666,9 +1654,6 @@ pub fn path_open( dirflags & __WASI_LOOKUP_SYMLINK_FOLLOW != 0, ); - if let Ok(m) = maybe_inode { - &state.fs.inodes[m]; - } let mut open_flags = 0; // TODO: traverse rights of dirs properly @@ -1820,6 +1805,7 @@ pub fn path_open( )); fd_cell.set(out_fd); + debug!("wasi::path_open returning fd {}", out_fd); __WASI_ESUCCESS }