diff --git a/lib/wasix/src/fs/mod.rs b/lib/wasix/src/fs/mod.rs index d19ac4035ef..9c92fa69212 100644 --- a/lib/wasix/src/fs/mod.rs +++ b/lib/wasix/src/fs/mod.rs @@ -1,3 +1,10 @@ +// TODO: currently, hard links are broken in the presence or renames. +// It is impossible to fix them with the current setup, since a hard +// link must point to the actual file rather than its path, but the +// only way we can get to a file on a FileSystem instance is by going +// through its repective FileOpener and giving it a path as input. +// TODO: refactor away the InodeVal type + mod fd; mod fd_list; mod inode_guard; @@ -1262,32 +1269,7 @@ impl WasiFs { follow_symlinks: bool, ) -> Result { let base_inode = self.get_fd_inode(base)?; - let name = base_inode.name.read().unwrap(); - - let start_inode; - if name.starts_with('/') { - drop(name); - start_inode = base_inode; - } else { - let kind = base_inode.kind.read().unwrap(); - - // HACK: We preopen '/' with an alias of '.' by default in `prepare_webc_env`. If wasix-libc - // picks that up as the base FD, we want to do the same thing we do when the base is '/' - if *name == "." - && matches!(kind.deref(), Kind::Dir { path, .. } if path.as_os_str().as_encoded_bytes() == b"/") - { - drop(name); - drop(kind); - start_inode = base_inode; - } else { - drop(name); - drop(kind); - let (cur_inode, _) = self.get_current_dir(inodes, base)?; - start_inode = cur_inode; - } - }; - - self.get_inode_at_path_inner(inodes, start_inode, path, 0, follow_symlinks) + self.get_inode_at_path_inner(inodes, base_inode, path, 0, follow_symlinks) } /// Returns the parent Dir or Root that the file at a given path is in and the file name diff --git a/lib/wasix/src/runners/wasi_common.rs b/lib/wasix/src/runners/wasi_common.rs index 9e0de3a0e03..52a46ac6e86 100644 --- a/lib/wasix/src/runners/wasi_common.rs +++ b/lib/wasix/src/runners/wasi_common.rs @@ -73,10 +73,6 @@ impl CommonWasiOptions { builder.add_map_dir(".", path)?; } - // wasix-libc favors later FDs, and we want it to pass in the preopen - // for '/' when opening things since that's faster, so do this after - // the '.' alias. Purely a performance thing, since we also account - // for the alias in `get_inode_at_path`. builder.add_preopen_dir("/")?; builder.set_fs(Box::new(fs)); diff --git a/lib/wasix/src/syscalls/wasi/path_rename.rs b/lib/wasix/src/syscalls/wasi/path_rename.rs index 7f1fe0b51cb..71c654323bc 100644 --- a/lib/wasix/src/syscalls/wasi/path_rename.rs +++ b/lib/wasix/src/syscalls/wasi/path_rename.rs @@ -1,3 +1,7 @@ +use std::path::PathBuf; + +use anyhow::Context; + use super::*; use crate::syscalls::*; @@ -98,9 +102,7 @@ pub fn path_rename_internal( if entries.contains_key(&target_entry_name) { need_create = false; } - let mut out_path = path.clone(); - out_path.push(std::path::Path::new(&target_entry_name)); - out_path + path.join(&target_entry_name) } Kind::Root { .. } => return Ok(Errno::Notcapable), Kind::Socket { .. } @@ -157,13 +159,11 @@ pub fn path_rename_internal( return Ok(e); } } else { - { - let mut guard = source_entry.write(); - if let Kind::File { ref mut path, .. } = guard.deref_mut() { - *path = host_adjusted_target_path; - } else { - unreachable!() - } + let mut guard = source_entry.write(); + if let Kind::File { ref mut path, .. } = guard.deref_mut() { + *path = host_adjusted_target_path; + } else { + unreachable!() } } } @@ -182,19 +182,17 @@ pub fn path_rename_internal( return Ok(e); } { + let source_dir_path = path.clone(); drop(guard); - let mut guard = source_entry.write(); - if let Kind::Dir { path, .. } = guard.deref_mut() { - *path = host_adjusted_target_path; - } + rename_inode_tree(&source_entry, &source_dir_path, &host_adjusted_target_path); } } - Kind::Buffer { .. } => {} - Kind::Symlink { .. } => {} - Kind::Socket { .. } => {} - Kind::Pipe { .. } => {} - Kind::Epoll { .. } => {} - Kind::EventNotifications { .. } => {} + Kind::Buffer { .. } + | Kind::Symlink { .. } + | Kind::Socket { .. } + | Kind::Pipe { .. } + | Kind::Epoll { .. } + | Kind::EventNotifications { .. } => {} Kind::Root { .. } => unreachable!("The root can not be moved"), } } @@ -213,12 +211,46 @@ pub fn path_rename_internal( } // The target entry is created, one way or the other - let target_inode = - wasi_try_ok!(state - .fs - .get_inode_at_path(inodes, target_fd, target_path, true)); + let target_inode = state + .fs + .get_inode_at_path(inodes, target_fd, target_path, true) + .expect("Expected target inode to exist, and it's too late to safely fail"); *target_inode.name.write().unwrap() = target_entry_name.into(); target_inode.stat.write().unwrap().st_size = source_size; Ok(Errno::Success) } + +fn rename_inode_tree(inode: &InodeGuard, source_dir_path: &Path, target_dir_path: &Path) { + let children; + + let mut guard = inode.write(); + match guard.deref_mut() { + Kind::File { ref mut path, .. } => { + *path = adjust_path(path, source_dir_path, target_dir_path); + return; + } + Kind::Dir { + ref mut path, + entries, + .. + } => { + *path = adjust_path(path, source_dir_path, target_dir_path); + children = entries.values().cloned().collect::>(); + } + _ => return, + } + drop(guard); + + for child in children { + rename_inode_tree(&child, source_dir_path, target_dir_path); + } +} + +fn adjust_path(path: &Path, source_dir_path: &Path, target_dir_path: &Path) -> PathBuf { + let relative_path = path + .strip_prefix(source_dir_path) + .with_context(|| format!("Expected path {path:?} to be a subpath of {source_dir_path:?}")) + .expect("Fatal filesystem error"); + target_dir_path.join(relative_path) +} diff --git a/tests/wasix/create-move-open/main.c b/tests/wasix/create-move-open/main.c new file mode 100644 index 00000000000..12d3c8da83b --- /dev/null +++ b/tests/wasix/create-move-open/main.c @@ -0,0 +1,71 @@ +// There used to be an issue where, when moving a file, you could no longer +// open it afterwards. This is a regression test for that issue. + +#include +#include +#include +#include +#include +#include +#include +#include + +void error(const char *message) +{ + perror(message); + exit(-1); +} + +int main() +{ + // Create two directories + if (mkdir("test1", S_IRWXU)) + { + error("mkdir test1"); + } + if (mkdir("test1/inner", S_IRWXU)) + { + error("mkdir t2"); + } + + FILE *file; + if (!(file = fopen("test1/inner/file", "wt"))) + { + error("create file"); + } + if (!fwrite("hello\n", 1, 6, file)) + { + error("write to file"); + } + if (fflush(file)) + { + error("fflush"); + } + if (fclose(file)) + { + error("fclose"); + } + + if (rename("test1", "test2")) + { + error("rename"); + } + + if (!(file = fopen("test2/inner/file", "rt"))) + { + error("open renamed file"); + } + char buf[7] = {0}; + if (!fread((void *)buf, 1, 7, file)) + { + error("fread"); + } + if (strcmp(buf, "hello\n")) + { + printf("Invalid file contents: %s\n", buf); + return -1; + } + + printf("0"); + return 0; +} diff --git a/tests/wasix/create-move-open/run.sh b/tests/wasix/create-move-open/run.sh new file mode 100755 index 00000000000..8b18c8e3484 --- /dev/null +++ b/tests/wasix/create-move-open/run.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +$WASMER -q run main.wasm --dir . > output + +rm -rf test1 test2 2>/dev/null && printf "0" | diff -u output - 1>/dev/null diff --git a/tests/wasix/fstatat-with-chdir/main.c b/tests/wasix/fstatat-with-chdir/main.c new file mode 100644 index 00000000000..10cdfdaec02 --- /dev/null +++ b/tests/wasix/fstatat-with-chdir/main.c @@ -0,0 +1,57 @@ +#include +#include +#include +#include +#include +#include +#include + +void error(const char *message) +{ + perror(message); + exit(-1); +} + +int main() +{ + // Create two directories + if (mkdir("test1", S_IRWXU)) + { + error("mkdir test1"); + } + if (mkdir("test2", S_IRWXU)) + { + error("mkdir test2"); + } + + // open the second directory for fstatat + int fd; + if ((fd = open("test2", O_RDONLY | O_DIRECTORY)) < 0) + { + error("open"); + } + + // chdir into the first directory + if (chdir("/home/test1")) + { + error("chdir"); + } + + // Now stat the second directory with a relative path. + // CWD should not be taken into account, and the stat + // should succeed. + struct stat st; + if (fstatat(fd, ".", &st, 0)) + { + error("fstatat"); + } + + if (!S_ISDIR(st.st_mode)) + { + printf("Expected a directory\n"); + return -1; + } + + printf("0"); + return 0; +} diff --git a/tests/wasix/fstatat-with-chdir/run.sh b/tests/wasix/fstatat-with-chdir/run.sh new file mode 100755 index 00000000000..8b18c8e3484 --- /dev/null +++ b/tests/wasix/fstatat-with-chdir/run.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +$WASMER -q run main.wasm --dir . > output + +rm -rf test1 test2 2>/dev/null && printf "0" | diff -u output - 1>/dev/null