Skip to content

Commit

Permalink
release 9.0.0 branch: fix directory open rights (#6479)
Browse files Browse the repository at this point in the history
* fix the directory base & inheriting rights

in order to work with wasi-testsuite, it needs to be possible to
path_open(dirfd, ".", ...) with the same rights reported in the
fdstat of that dirfd. When we report the Rights::all() set, both
FD_READ and FD_WRITE are set in the base rights, which results in
unix rejecting an openat2(dirfd, ".", O_RDWR) with EISDIR.

By not having the FD_READ and FD_WRITE rights present in the base
rights, the open syscall defaults to O_RDONLY, which is the only
access mode allowed for opening directories.

* path_open of a directory with read and write succeeds on windows

* fix test introduced as part of 9.0.2
  • Loading branch information
Pat Hickey authored May 31, 2023
1 parent 0aa0047 commit 61cc2fa
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 6 deletions.
92 changes: 92 additions & 0 deletions crates/test-programs/wasi-tests/src/bin/path_open_preopen.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
const FIRST_PREOPEN: u32 = 3;

unsafe fn path_open_preopen() {
let prestat = wasi::fd_prestat_get(FIRST_PREOPEN).expect("fd 3 is a preopen");
assert_eq!(
prestat.tag,
wasi::PREOPENTYPE_DIR.raw(),
"prestat is a directory"
);
let mut dst = Vec::with_capacity(prestat.u.dir.pr_name_len);
wasi::fd_prestat_dir_name(FIRST_PREOPEN, dst.as_mut_ptr(), dst.capacity())
.expect("get preopen dir name");
dst.set_len(prestat.u.dir.pr_name_len);

let fdstat = wasi::fd_fdstat_get(FIRST_PREOPEN).expect("get fdstat");

println!(
"preopen dir: {:?} base {:?} inheriting {:?}",
String::from_utf8_lossy(&dst),
fdstat.fs_rights_base,
fdstat.fs_rights_inheriting
);

// Open with same rights it has now:
let _ = wasi::path_open(
FIRST_PREOPEN,
0,
".",
0,
fdstat.fs_rights_base,
fdstat.fs_rights_inheriting,
0,
)
.expect("open with same rights");

// Open with an empty set of rights:
let _ = wasi::path_open(FIRST_PREOPEN, 0, ".", 0, 0, 0, 0).expect("open with empty rights");

// Open OFLAGS_DIRECTORY with an empty set of rights:
let _ = wasi::path_open(FIRST_PREOPEN, 0, ".", wasi::OFLAGS_DIRECTORY, 0, 0, 0)
.expect("open with O_DIRECTORY empty rights");

// Open OFLAGS_DIRECTORY with just the read right:
let _ = wasi::path_open(
FIRST_PREOPEN,
0,
".",
wasi::OFLAGS_DIRECTORY,
wasi::RIGHTS_FD_READ,
0,
0,
)
.expect("open with O_DIRECTORY and read right");

if !wasi_tests::TESTCONFIG.errno_expect_windows() {
// Open OFLAGS_DIRECTORY and read/write rights should fail with isdir:
let err = wasi::path_open(
FIRST_PREOPEN,
0,
".",
wasi::OFLAGS_DIRECTORY,
wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
0,
0,
)
.err()
.expect("open with O_DIRECTORY and read/write should fail");
assert_eq!(
err,
wasi::ERRNO_ISDIR,
"opening directory read/write should fail with ISDIR"
);
} else {
// Open OFLAGS_DIRECTORY and read/write rights will succeed, only on windows:
let _ = wasi::path_open(
FIRST_PREOPEN,
0,
".",
wasi::OFLAGS_DIRECTORY,
wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
0,
0,
)
.expect("open with O_DIRECTORY and read/write should succeed on windows");
}
}

fn main() {
unsafe {
path_open_preopen();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ use wasi_tests::{assert_errno, create_file, open_scratch_directory};
unsafe fn test_path_open_read_write(dir_fd: wasi::Fd) {
let stat = wasi::fd_fdstat_get(dir_fd).expect("get dirfd stat");
assert!(
stat.fs_rights_base & wasi::RIGHTS_FD_READ == wasi::RIGHTS_FD_READ,
"dirfd has base read right"
stat.fs_rights_base & wasi::RIGHTS_FD_READ == 0,
"dirfd does not have base read right"
);
assert!(
stat.fs_rights_inheriting & wasi::RIGHTS_FD_READ == wasi::RIGHTS_FD_READ,
"dirfd has inheriting read right"
);
assert!(
stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == wasi::RIGHTS_FD_WRITE,
"dirfd has base write right"
stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == 0,
"dirfd does not have base write right"
);
assert!(
stat.fs_rights_inheriting & wasi::RIGHTS_FD_WRITE == wasi::RIGHTS_FD_WRITE,
Expand Down
46 changes: 44 additions & 2 deletions crates/wasi-common/src/snapshots/preview_1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
let _dir_entry: Arc<DirEntry> = table.get(fd)?;
let dir_fdstat = types::Fdstat {
fs_filetype: types::Filetype::Directory,
fs_rights_base: types::Rights::all(),
fs_rights_inheriting: types::Rights::all(),
fs_rights_base: directory_base_rights(),
fs_rights_inheriting: directory_inheriting_rights(),
fs_flags: types::Fdflags::empty(),
};
Ok(dir_fdstat)
Expand Down Expand Up @@ -1446,3 +1446,45 @@ fn systimespec(
Ok(None)
}
}

// This is the default subset of base Rights reported for directories prior to
// https://github.com/bytecodealliance/wasmtime/pull/6265. Some
// implementations still expect this set of rights to be reported.
pub(crate) fn directory_base_rights() -> types::Rights {
types::Rights::PATH_CREATE_DIRECTORY
| types::Rights::PATH_CREATE_FILE
| types::Rights::PATH_LINK_SOURCE
| types::Rights::PATH_LINK_TARGET
| types::Rights::PATH_OPEN
| types::Rights::FD_READDIR
| types::Rights::PATH_READLINK
| types::Rights::PATH_RENAME_SOURCE
| types::Rights::PATH_RENAME_TARGET
| types::Rights::PATH_SYMLINK
| types::Rights::PATH_REMOVE_DIRECTORY
| types::Rights::PATH_UNLINK_FILE
| types::Rights::PATH_FILESTAT_GET
| types::Rights::PATH_FILESTAT_SET_TIMES
| types::Rights::FD_FILESTAT_GET
| types::Rights::FD_FILESTAT_SET_TIMES
}

// This is the default subset of inheriting Rights reported for directories
// prior to https://github.com/bytecodealliance/wasmtime/pull/6265. Some
// implementations still expect this set of rights to be reported.
pub(crate) fn directory_inheriting_rights() -> types::Rights {
types::Rights::FD_DATASYNC
| types::Rights::FD_READ
| types::Rights::FD_SEEK
| types::Rights::FD_FDSTAT_SET_FLAGS
| types::Rights::FD_SYNC
| types::Rights::FD_TELL
| types::Rights::FD_WRITE
| types::Rights::FD_ADVISE
| types::Rights::FD_ALLOCATE
| types::Rights::FD_FILESTAT_GET
| types::Rights::FD_FILESTAT_SET_SIZE
| types::Rights::FD_FILESTAT_SET_TIMES
| types::Rights::POLL_FD_READWRITE
| directory_base_rights()
}

0 comments on commit 61cc2fa

Please sign in to comment.