From ad2c74d5b499ce714514ab9dc103accc213d0190 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Wed, 21 Apr 2021 07:38:18 -0700 Subject: [PATCH 1/2] Clean up core WASI FS code --- lib/wasi/src/state/mod.rs | 63 +++++++++++---------------------------- 1 file changed, 18 insertions(+), 45 deletions(-) diff --git a/lib/wasi/src/state/mod.rs b/lib/wasi/src/state/mod.rs index eddc679e32d..45076fce923 100644 --- a/lib/wasi/src/state/mod.rs +++ b/lib/wasi/src/state/mod.rs @@ -784,7 +784,7 @@ impl WasiFs { let file_type = metadata.file_type(); // we want to insert newly opened dirs and files, but not transient symlinks // TODO: explain why (think about this deeply when well rested) - let mut should_insert = false; + let should_insert; let kind = if file_type.is_dir() { should_insert = true; @@ -803,6 +803,7 @@ impl WasiFs { fd: None, } } else if file_type.is_symlink() { + should_insert = false; let link_value = file.read_link().ok().ok_or(__WASI_EIO)?; debug!("attempting to decompose path {:?}", link_value); @@ -957,18 +958,15 @@ impl WasiFs { Ok(cur_inode) } - /// Splits a path into the first preopened directory that is a parent of it, - /// if such a preopened directory exists, and the rest of the path. - /// - /// NOTE: this behavior seems to be not the same as what libpreopen is - /// doing in WASI. - /// - /// TODO: evaluate users of this function and explain why this behavior is - /// not the same as libpreopen or update its behavior to be the same. + /// Splits a path into the preopened directory that has the largest prefix of + /// the given path, if such a preopened directory exists, and the rest of the path. fn path_into_pre_open_and_relative_path( &self, path: &Path, ) -> Result<(__wasi_fd_t, PathBuf), __wasi_errno_t> { + let mut max_prefix_len = 0; + let mut max_stripped_path = None; + let mut max_po_fd = None; // for each preopened directory for po_fd in &self.preopen_fds { let po_inode = self.fd_map[po_fd].inode; @@ -978,46 +976,21 @@ impl WasiFs { _ => unreachable!("Preopened FD that's not a directory or the root"), }; // stem path based on it - if let Ok(rest) = path.strip_prefix(po_path) { - // if any path meets this criteria - // (verify that all remaining components are not symlinks except for maybe last? (or do the more complex logic of resolving intermediary symlinks)) - // return preopened dir and the rest of the path - - return Ok((*po_fd, rest.to_owned())); - } - } - 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; - } + if let Ok(stripped_path) = path.strip_prefix(po_path) { + // find the max + let new_prefix_len = po_path.as_os_str().len(); + if new_prefix_len >= max_prefix_len { + max_prefix_len = new_prefix_len; + max_stripped_path = Some(stripped_path); + max_po_fd = Some(*po_fd); } - _ => return Err(__WASI_EINVAL), } } - - let mut out = PathBuf::new(); - for p in stack.iter().rev() { - out.push(p); + if max_prefix_len == 0 { + Err(__WASI_EINVAL) // this may not make sense depending on where it's called + } else { + Ok((max_po_fd.unwrap(), max_stripped_path.unwrap().to_owned())) } - Ok(out) } /// finds the number of directories between the fd and the inode if they're connected From fcb90f4a9971365324893c92799121e85864171c Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Wed, 21 Apr 2021 07:43:47 -0700 Subject: [PATCH 2/2] Add comment and update changelog --- CHANGELOG.md | 1 + lib/wasi/src/state/mod.rs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09ba0f76593..3350ffa714e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Looking for changes that affect our C API? See the [C API Changelog](lib/c-api/C - [#2149](https://github.com/wasmerio/wasmer/pull/2144) `wasmer-engine-native` looks for clang-11 instead of clang-10. ### Fixed +- [#2247](https://github.com/wasmerio/wasmer/pull/2247) Internal WasiFS logic updated to be closer to what WASI libc does when finding a preopened fd for a path. - [#2208](https://github.com/wasmerio/wasmer/pull/2208) Fix ownership in Wasm C API of `wasm_extern_as_func`, `wasm_extern_as_memory`, `wasm_extern_as_table`, `wasm_extern_as_global`, `wasm_func_as_extern`, `wasm_memory_as_extern`, `wasm_table_as_extern`, and `wasm_global_as_extern`. These functions no longer allocate memory and thus their results should not be freed. This is a breaking change to align more closely with the Wasm C API's stated ownership. - [#2210](https://github.com/wasmerio/wasmer/pull/2210) Fix a memory leak in the Wasm C API in the strings used to identify imports and exports coming from user code. - [#2108](https://github.com/wasmerio/wasmer/pull/2108) The Object Native Engine generates code that now compiles correctly with C++. diff --git a/lib/wasi/src/state/mod.rs b/lib/wasi/src/state/mod.rs index 45076fce923..9ae1e5eae33 100644 --- a/lib/wasi/src/state/mod.rs +++ b/lib/wasi/src/state/mod.rs @@ -979,6 +979,8 @@ impl WasiFs { if let Ok(stripped_path) = path.strip_prefix(po_path) { // find the max let new_prefix_len = po_path.as_os_str().len(); + // we use >= to favor later preopens because we iterate in order + // whereas WASI libc iterates in reverse to get this behavior. if new_prefix_len >= max_prefix_len { max_prefix_len = new_prefix_len; max_stripped_path = Some(stripped_path);