T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Allow WASI to open directories without O_DIRECTORY (#6163)

Browse Source 
* Allow WASI to open directories without O_DIRECTORY

The `O_DIRECTORY` flag is a request that open should fail if the named
path is not a directory. Opening a path which turns out to be a
directory is not supposed to fail if this flag is not specified.
However, wasi-common required callers to use it when opening
directories.

With this PR, we always open the path the same way whether or not the
`O_DIRECTORY` flag is specified. However, after opening it, we `stat` it
to check whether it turned out to be a directory, and determine which
operations the file descriptor should support accordingly. In addition,
we explicitly check whether the precondition defined by `O_DIRECTORY` is
satisfied.

Closes #4947 and closes #4967, which were earlier attempts at fixing the
same issue, but which had race conditions.

prtest:full

* Add tests from #4967/#4947

This test was authored by Roman Volosatovs <rvolosatovs@riseup.net> as
part of #4947.

* Tests: Close FDs before trying to unlink files

On Windows, when opening a path which might be a directory using
`CreateFile`, cap-primitives also removes the `FILE_SHARE_DELETE` mode.

That means that if we implement WASI's `path_open` such that it always
uses `CreateFile` on Windows, for both files and directories, then
holding an open file handle prevents deletion of that file.

So I'm changing these test programs to make sure they've closed the
handle before trying to delete the file.
This commit is contained in:
Jamey Sharp
2023-04-21 09:55:35 -07:00
committed by GitHub
parent 91d1d246cd
commit efdfc361f8
8 changed files with 170 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
crates/wasi-common/cap-std-sync/src/dir.rs
View File

@@ -1,18 +1,24 @@
use crate::file::{filetype_from, File};
use cap_fs_ext::{DirEntryExt, DirExt, MetadataExt, SystemTimeSpec};
use cap_fs_ext::{DirEntryExt, DirExt, MetadataExt, OpenOptionsMaybeDirExt, SystemTimeSpec};
use cap_std::fs;
use std::any::Any;
use std::path::{Path, PathBuf};
use system_interface::fs::GetSetFdFlags;
use wasi_common::{
dir::{ReaddirCursor, ReaddirEntity, WasiDir},
file::{FdFlags, FileType, Filestat, OFlags, WasiFile},
file::{FdFlags, FileType, Filestat, OFlags},
Error, ErrorExt,
};
pub struct Dir(cap_std::fs::Dir);
pub struct Dir(fs::Dir);
pub enum OpenResult {
File(File),
Dir(Dir),
}
impl Dir {
pub fn from_cap_std(dir: cap_std::fs::Dir) -> Self {
pub fn from_cap_std(dir: fs::Dir) -> Self {
Dir(dir)
}
@@ -24,10 +30,11 @@ impl Dir {
read: bool,
write: bool,
fdflags: FdFlags,
) -> Result<File, Error> {
) -> Result<OpenResult, Error> {
use cap_fs_ext::{FollowSymlinks, OpenOptionsFollowExt};
let mut opts = cap_std::fs::OpenOptions::new();
let mut opts = fs::OpenOptions::new();
opts.maybe_dir(true);
if oflags.contains(OFlags::CREATE | OFlags::EXCLUSIVE) {
opts.create_new(true);
@@ -71,22 +78,31 @@ impl Dir {
return Err(Error::not_supported().context("SYNC family of FdFlags"));
}
if oflags.contains(OFlags::DIRECTORY) {
if oflags.contains(OFlags::CREATE)
|| oflags.contains(OFlags::EXCLUSIVE)
|| oflags.contains(OFlags::TRUNCATE)
{
return Err(Error::invalid_argument().context("directory oflags"));
}
}
let mut f = self.0.open_with(Path::new(path), &opts)?;
// NONBLOCK does not have an OpenOption either, but we can patch that on with set_fd_flags:
if fdflags.contains(wasi_common::file::FdFlags::NONBLOCK) {
let set_fd_flags = f.new_set_fd_flags(system_interface::fs::FdFlags::NONBLOCK)?;
f.set_fd_flags(set_fd_flags)?;
}
Ok(File::from_cap_std(f))
}
pub fn open_dir_(&self, symlink_follow: bool, path: &str) -> Result<Self, Error> {
let d = if symlink_follow {
self.0.open_dir(Path::new(path))?
if f.metadata()?.is_dir() {
Ok(OpenResult::Dir(Dir::from_cap_std(fs::Dir::from_std_file(
f.into_std(),
))))
} else if oflags.contains(OFlags::DIRECTORY) {
Err(Error::not_dir().context("expected directory but got file"))
} else {
self.0.open_dir_nofollow(Path::new(path))?
};
Ok(Dir::from_cap_std(d))
Ok(OpenResult::File(File::from_cap_std(f)))
}
}
pub fn rename_(&self, src_path: &str, dest_dir: &Self, dest_path: &str) -> Result<(), Error> {
@@ -120,14 +136,12 @@ impl WasiDir for Dir {
read: bool,
write: bool,
fdflags: FdFlags,
) -> Result<Box<dyn WasiFile>, Error> {
) -> Result<wasi_common::dir::OpenResult, Error> {
let f = self.open_file_(symlink_follow, path, oflags, read, write, fdflags)?;
Ok(Box::new(f))
}
async fn open_dir(&self, symlink_follow: bool, path: &str) -> Result<Box<dyn WasiDir>, Error> {
let d = self.open_dir_(symlink_follow, path)?;
Ok(Box::new(d))
match f {
OpenResult::File(f) => Ok(wasi_common::dir::OpenResult::File(Box::new(f))),
OpenResult::Dir(d) => Ok(wasi_common::dir::OpenResult::Dir(Box::new(d))),
}
}
async fn create_dir(&self, path: &str) -> Result<(), Error> {
@@ -327,6 +341,7 @@ fn convert_systimespec(t: Option<wasi_common::SystemTimeSpec>) -> Option<SystemT
mod test {
use super::Dir;
use cap_std::ambient_authority;
use wasi_common::file::{FdFlags, OFlags};
#[test]
fn scratch_dir() {
let tempdir = tempfile::Builder::new()
@@ -336,8 +351,16 @@ mod test {
let preopen_dir = cap_std::fs::Dir::open_ambient_dir(tempdir.path(), ambient_authority())
.expect("open ambient temporary dir");
let preopen_dir = Dir::from_cap_std(preopen_dir);
run(wasi_common::WasiDir::open_dir(&preopen_dir, false, "."))
.expect("open the same directory via WasiDir abstraction");
run(wasi_common::WasiDir::open_file(
&preopen_dir,
false,
".",
OFlags::empty(),
false,
false,
FdFlags::empty(),
))
.expect("open the same directory via WasiDir abstraction");
}
// Readdir does not work on windows, so we won't test it there.

15
crates/wasi-common/src/dir.rs
View File

@@ -5,6 +5,11 @@ use std::any::Any;
use std::path::PathBuf;
use std::sync::{Arc, RwLock};
pub enum OpenResult {
File(Box<dyn WasiFile>),
Dir(Box<dyn WasiDir>),
}
#[wiggle::async_trait]
pub trait WasiDir: Send + Sync {
fn as_any(&self) -> &dyn Any;
@@ -17,15 +22,7 @@ pub trait WasiDir: Send + Sync {
_read: bool,
_write: bool,
_fdflags: FdFlags,
) -> Result<Box<dyn WasiFile>, Error> {
Err(Error::not_supported())
}
async fn open_dir(
&self,
_symlink_follow: bool,
_path: &str,
) -> Result<Box<dyn WasiDir>, Error> {
) -> Result<OpenResult, Error> {
Err(Error::not_supported())
}

66
crates/wasi-common/src/snapshots/preview_1.rs
View File

@@ -1,5 +1,8 @@
use crate::{
dir::{DirCaps, DirEntry, DirEntryExt, DirFdStat, ReaddirCursor, ReaddirEntity, TableDirExt},
dir::{
DirCaps, DirEntry, DirEntryExt, DirFdStat, OpenResult, ReaddirCursor, ReaddirEntity,
TableDirExt,
},
file::{
Advice, FdFlags, FdStat, FileCaps, FileEntry, FileEntryExt, FileType, Filestat, OFlags,
RiFlags, RoFlags, SdFlags, SiFlags, TableFileExt, WasiFile,
@@ -747,41 +750,36 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
let oflags = OFlags::from(&oflags);
let fdflags = FdFlags::from(fdflags);
let path = path.as_cow()?;
if oflags.contains(OFlags::DIRECTORY) {
if oflags.contains(OFlags::CREATE)
|| oflags.contains(OFlags::EXCLUSIVE)
|| oflags.contains(OFlags::TRUNCATE)
{
return Err(Error::invalid_argument().context("directory oflags"));
}
let dir_caps = dir_entry.child_dir_caps(DirCaps::from(&fs_rights_base));
let file_caps = dir_entry.child_file_caps(FileCaps::from(&fs_rights_inheriting));
let dir = dir_entry.get_cap(DirCaps::OPEN)?;
let child_dir = dir.open_dir(symlink_follow, path.deref()).await?;
drop(dir);
let fd = table.push(Arc::new(DirEntry::new(
dir_caps, file_caps, None, child_dir,
)))?;
Ok(types::Fd::from(fd))
} else {
let mut required_caps = DirCaps::OPEN;
if oflags.contains(OFlags::CREATE) {
required_caps = required_caps | DirCaps::CREATE_FILE;
}
let file_caps = dir_entry.child_file_caps(FileCaps::from(&fs_rights_base));
let dir = dir_entry.get_cap(required_caps)?;
let read = file_caps.contains(FileCaps::READ);
let write = file_caps.contains(FileCaps::WRITE)
|| file_caps.contains(FileCaps::ALLOCATE)
|| file_caps.contains(FileCaps::FILESTAT_SET_SIZE);
let file = dir
.open_file(symlink_follow, path.deref(), oflags, read, write, fdflags)
.await?;
drop(dir);
let fd = table.push(Arc::new(FileEntry::new(file_caps, file)))?;
Ok(types::Fd::from(fd))
let mut required_caps = DirCaps::OPEN;
if oflags.contains(OFlags::CREATE) {
required_caps = required_caps | DirCaps::CREATE_FILE;
}
let dir_dir_caps = dir_entry.child_dir_caps(DirCaps::from(&fs_rights_base));
let dir_file_caps = dir_entry.child_file_caps(FileCaps::from(&fs_rights_inheriting));
let file_caps = dir_entry.child_file_caps(FileCaps::from(&fs_rights_base));
let dir = dir_entry.get_cap(required_caps)?;
let read = file_caps.contains(FileCaps::READ);
let write = file_caps.contains(FileCaps::WRITE)
|| file_caps.contains(FileCaps::ALLOCATE)
|| file_caps.contains(FileCaps::FILESTAT_SET_SIZE);
let file = dir
.open_file(symlink_follow, path.deref(), oflags, read, write, fdflags)
.await?;
drop(dir);
let fd = match file {
OpenResult::File(file) => table.push(Arc::new(FileEntry::new(file_caps, file)))?,
OpenResult::Dir(child_dir) => table.push(Arc::new(DirEntry::new(
dir_dir_caps,
dir_file_caps,
None,
child_dir,
)))?,
};
Ok(types::Fd::from(fd))
}
async fn path_readlink<'a>(

34
crates/wasi-common/tokio/src/dir.rs
View File

@@ -3,7 +3,7 @@ use std::any::Any;
use std::path::PathBuf;
use wasi_common::{
dir::{ReaddirCursor, ReaddirEntity, WasiDir},
file::{FdFlags, Filestat, OFlags, WasiFile},
file::{FdFlags, Filestat, OFlags},
Error, ErrorExt,
};
@@ -28,18 +28,19 @@ impl WasiDir for Dir {
read: bool,
write: bool,
fdflags: FdFlags,
) -> Result<Box<dyn WasiFile>, Error> {
) -> Result<wasi_common::dir::OpenResult, Error> {
let f = block_on_dummy_executor(move || async move {
self.0
.open_file_(symlink_follow, path, oflags, read, write, fdflags)
})?;
Ok(Box::new(File::from_inner(f)))
}
async fn open_dir(&self, symlink_follow: bool, path: &str) -> Result<Box<dyn WasiDir>, Error> {
let d =
block_on_dummy_executor(move || async move { self.0.open_dir_(symlink_follow, path) })?;
Ok(Box::new(Dir(d)))
match f {
wasi_cap_std_sync::dir::OpenResult::File(f) => Ok(wasi_common::dir::OpenResult::File(
Box::new(File::from_inner(f)),
)),
wasi_cap_std_sync::dir::OpenResult::Dir(d) => {
Ok(wasi_common::dir::OpenResult::Dir(Box::new(Dir(d))))
}
}
}
async fn create_dir(&self, path: &str) -> Result<(), Error> {
@@ -127,6 +128,7 @@ impl WasiDir for Dir {
mod test {
use super::Dir;
use cap_std::ambient_authority;
use wasi_common::file::{FdFlags, OFlags};
#[tokio::test(flavor = "multi_thread")]
async fn scratch_dir() {
@@ -137,9 +139,17 @@ mod test {
let preopen_dir = cap_std::fs::Dir::open_ambient_dir(tempdir.path(), ambient_authority())
.expect("open ambient temporary dir");
let preopen_dir = Dir::from_cap_std(preopen_dir);
wasi_common::WasiDir::open_dir(&preopen_dir, false, ".")
.await
.expect("open the same directory via WasiDir abstraction");
wasi_common::WasiDir::open_file(
&preopen_dir,
false,
".",
OFlags::empty(),
false,
false,
FdFlags::empty(),
)
.await
.expect("open the same directory via WasiDir abstraction");
}
// Readdir does not work on windows, so we won't test it there.

27
crates/wasi-common/tokio/tests/poll_oneoff.rs
View File

@@ -2,6 +2,7 @@ use anyhow::{Context, Error};
use cap_std::time::Duration;
use std::collections::HashMap;
use wasi_common::{
dir::OpenResult,
file::{FdFlags, OFlags},
sched::{Poll, RwEventFlags, SubscriptionResult, Userdata},
WasiDir, WasiFile,
@@ -25,18 +26,26 @@ async fn empty_file_readable() -> Result<(), Error> {
.await
.context("create writable file f")?;
let to_write: Vec<u8> = vec![0];
f.write_vectored(&vec![std::io::IoSlice::new(&to_write)])
.await
.context("write to f")?;
if let OpenResult::File(ref f) = f {
f.write_vectored(&vec![std::io::IoSlice::new(&to_write)])
.await
.context("write to f")?;
} else {
unreachable!();
}
drop(f);
let mut f = d
let f = d
.open_file(false, "f", OFlags::empty(), true, false, FdFlags::empty())
.await
.context("open f as readable")?;
let mut poll = Poll::new();
poll.subscribe_read(&mut *f, Userdata::from(123));
if let OpenResult::File(ref f) = f {
poll.subscribe_read(f.as_ref(), Userdata::from(123));
} else {
unreachable!();
}
// Timeout bounds time in poll_oneoff
let monotonic = &*clocks.monotonic()?.abs_clock;
poll.subscribe_monotonic_clock(
@@ -73,13 +82,17 @@ async fn empty_file_writable() -> Result<(), Error> {
let d = workspace.open_dir("d").context("open dir")?;
let d = Dir::from_cap_std(d);
let mut writable_f = d
let writable_f = d
.open_file(false, "f", OFlags::CREATE, true, true, FdFlags::empty())
.await
.context("create writable file")?;
let mut poll = Poll::new();
poll.subscribe_write(&mut *writable_f, Userdata::from(123));
if let OpenResult::File(ref writable_f) = writable_f {
poll.subscribe_write(writable_f.as_ref(), Userdata::from(123));
} else {
unreachable!();
}
// Timeout bounds time in poll_oneoff
let monotonic = &*clocks.monotonic()?.abs_clock;
poll.subscribe_monotonic_clock(