Introduce strongly-typed system primitives (#1561)

* Introduce strongly-typed system primitives

This commit does a lot of reshuffling and even some more. It introduces
strongly-typed system primitives which are: `OsFile`, `OsDir`, `Stdio`,
and `OsOther`. Those primitives are separate structs now, each implementing
a subset of `Handle` methods, rather than all being an enumeration of some
supertype such as `OsHandle`. To summarise the structs:

* `OsFile` represents a regular file, and implements fd-ops
  of `Handle` trait
* `OsDir` represents a directory, and primarily implements path-ops, plus
  `readdir` and some common fd-ops such as `fdstat`, etc.
* `Stdio` represents a stdio handle, and implements a subset of fd-ops
  such as `fdstat` _and_ `read_` and `write_vectored` calls
* `OsOther` currently represents anything else and implements a set similar
  to that implemented by `Stdio`

This commit is effectively an experiment and an excercise into better
understanding what's going on for each OS resource/type under-the-hood.
It's meant to give us some intuition in order to move on with the idea
of having strongly-typed handles in WASI both in the syscall impl as well
as at the libc level.

Some more minor changes include making `OsHandle` represent an OS-specific
wrapper for a raw OS handle (Unix fd or Windows handle). Also, since `OsDir`
is tricky across OSes, we also have a supertype of `OsHandle` called
`OsDirHandle` which may store a `DIR*` stream pointer (mainly BSD). Last but not
least, the `Filetype` and `Rights` are now computed when the resource is created,
rather than every time we call `Handle::get_file_type` and `Handle::get_rights`.
Finally, in order to facilitate the latter, I've converted `EntryRights` into
`HandleRights` and pushed them into each `Handle` implementor.

* Do not adjust rights on Stdio

* Clean up testing for TTY and escaping writes

* Implement AsFile for dyn Handle

This cleans up a lot of repeating boilerplate code todo with
dynamic dispatch.

* Delegate definition of OsDir to OS-specific modules

Delegates defining `OsDir` struct to OS-specific modules (BSD, Linux,
Emscripten, Windows). This way, `OsDir` can safely re-use `OsHandle`
for raw OS handle storage, and can store some aux data such as an
initialized stream ptr in case of BSD. As a result, we can safely
get rid of `OsDirHandle` which IMHO was causing unnecessary noise and
overcomplicating the design. On the other hand, delegating definition
of `OsDir` to OS-specific modules isn't super clean in and of itself
either. Perhaps there's a better way of handling this?

* Check if filetype of OS handle matches WASI filetype when creating

It seems prudent to check if the passed in `File` instance is of
type matching that of the requested WASI filetype. In other words,
we'd like to avoid situations where `OsFile` is created from a
pipe.

* Make AsFile fallible

Return `EBADF` in `AsFile` in case a `Handle` cannot be made into
a `std::fs::File`.

* Remove unnecessary as_file conversion

* Remove unnecessary check for TTY for Stdio handle type

* Fix incorrect stdio ctors on Unix

* Split Stdio into three separate types: Stdin, Stdout, Stderr

* Rename PendingEntry::File to PendingEntry::OsHandle to avoid confusion

* Rename OsHandle to RawOsHandle

Also, since `RawOsHandle` on *nix doesn't need interior mutability
wrt the inner raw file descriptor, we can safely swap the `RawFd`
for `File` instance.

* Add docs explaining what OsOther is

* Allow for stdio to be non-character-device (e.g., piped)

* Return error on bad preopen rather than panic
This commit is contained in:
Jakub Konka
2020-05-08 01:00:14 +02:00
committed by GitHub
parent 528d3c1355
commit cbf7cbfa39
39 changed files with 1643 additions and 1073 deletions

View File

@@ -1,4 +1,5 @@
use crate::entry::{Entry, EntryHandle, EntryRights};
use crate::entry::{Entry, EntryHandle};
use crate::handle::HandleRights;
use crate::sys::clock;
use crate::wasi::wasi_snapshot_preview1::WasiSnapshotPreview1;
use crate::wasi::{types, AsBytes, Errno, Result};
@@ -91,7 +92,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
len: types::Filesize,
advice: types::Advice,
) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::FD_ADVISE);
let required_rights = HandleRights::from_base(types::Rights::FD_ADVISE);
let entry = self.get_entry(fd)?;
entry
.as_handle(&required_rights)?
@@ -104,7 +105,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
offset: types::Filesize,
len: types::Filesize,
) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::FD_ALLOCATE);
let required_rights = HandleRights::from_base(types::Rights::FD_ALLOCATE);
let entry = self.get_entry(fd)?;
entry.as_handle(&required_rights)?.allocate(offset, len)
}
@@ -121,18 +122,18 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
fn fd_datasync(&self, fd: types::Fd) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::FD_DATASYNC);
let required_rights = HandleRights::from_base(types::Rights::FD_DATASYNC);
let entry = self.get_entry(fd)?;
entry.as_handle(&required_rights)?.datasync()
}
fn fd_fdstat_get(&self, fd: types::Fd) -> Result<types::Fdstat> {
let entry = self.get_entry(fd)?;
let file = entry.as_handle(&EntryRights::empty())?;
let file = entry.as_handle(&HandleRights::empty())?;
let fs_flags = file.fdstat_get()?;
let rights = entry.rights.get();
let rights = entry.get_rights();
let fdstat = types::Fdstat {
fs_filetype: entry.file_type,
fs_filetype: entry.get_file_type(),
fs_rights_base: rights.base,
fs_rights_inheriting: rights.inheriting,
fs_flags,
@@ -141,7 +142,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
fn fd_fdstat_set_flags(&self, fd: types::Fd, flags: types::Fdflags) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::FD_FDSTAT_SET_FLAGS);
let required_rights = HandleRights::from_base(types::Rights::FD_FDSTAT_SET_FLAGS);
let entry = self.get_entry(fd)?;
entry.as_handle(&required_rights)?.fdstat_set_flags(flags)
}
@@ -152,24 +153,24 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
fs_rights_base: types::Rights,
fs_rights_inheriting: types::Rights,
) -> Result<()> {
let rights = EntryRights::new(fs_rights_base, fs_rights_inheriting);
let rights = HandleRights::new(fs_rights_base, fs_rights_inheriting);
let entry = self.get_entry(fd)?;
if !entry.rights.get().contains(&rights) {
if !entry.get_rights().contains(&rights) {
return Err(Errno::Notcapable);
}
entry.rights.set(rights);
entry.set_rights(rights);
Ok(())
}
fn fd_filestat_get(&self, fd: types::Fd) -> Result<types::Filestat> {
let required_rights = EntryRights::from_base(types::Rights::FD_FILESTAT_GET);
let required_rights = HandleRights::from_base(types::Rights::FD_FILESTAT_GET);
let entry = self.get_entry(fd)?;
let host_filestat = entry.as_handle(&required_rights)?.filestat_get()?;
Ok(host_filestat)
}
fn fd_filestat_set_size(&self, fd: types::Fd, size: types::Filesize) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::FD_FILESTAT_SET_SIZE);
let required_rights = HandleRights::from_base(types::Rights::FD_FILESTAT_SET_SIZE);
let entry = self.get_entry(fd)?;
// This check will be unnecessary when rust-lang/rust#63326 is fixed
if size > i64::max_value() as u64 {
@@ -185,7 +186,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
mtim: types::Timestamp,
fst_flags: types::Fstflags,
) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::FD_FILESTAT_SET_TIMES);
let required_rights = HandleRights::from_base(types::Rights::FD_FILESTAT_SET_TIMES);
let entry = self.get_entry(fd)?;
entry
.as_handle(&required_rights)?
@@ -213,7 +214,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
let required_rights =
EntryRights::from_base(types::Rights::FD_READ | types::Rights::FD_SEEK);
HandleRights::from_base(types::Rights::FD_READ | types::Rights::FD_SEEK);
let entry = self.get_entry(fd)?;
if offset > i64::max_value() as u64 {
return Err(Errno::Io);
@@ -227,9 +228,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
fn fd_prestat_get(&self, fd: types::Fd) -> Result<types::Prestat> {
// TODO: should we validate any rights here?
let fe = self.get_entry(fd)?;
let po_path = fe.preopen_path.as_ref().ok_or(Errno::Notsup)?;
if fe.file_type != types::Filetype::Directory {
let entry = self.get_entry(fd)?;
let po_path = entry.preopen_path.as_ref().ok_or(Errno::Notsup)?;
if entry.get_file_type() != types::Filetype::Directory {
return Err(Errno::Notdir);
}
@@ -247,9 +248,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
path_len: types::Size,
) -> Result<()> {
// TODO: should we validate any rights here?
let fe = self.get_entry(fd)?;
let po_path = fe.preopen_path.as_ref().ok_or(Errno::Notsup)?;
if fe.file_type != types::Filetype::Directory {
let entry = self.get_entry(fd)?;
let po_path = entry.preopen_path.as_ref().ok_or(Errno::Notsup)?;
if entry.get_file_type() != types::Filetype::Directory {
return Err(Errno::Notdir);
}
@@ -289,7 +290,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
let required_rights =
EntryRights::from_base(types::Rights::FD_WRITE | types::Rights::FD_SEEK);
HandleRights::from_base(types::Rights::FD_WRITE | types::Rights::FD_SEEK);
let entry = self.get_entry(fd)?;
if offset > i64::max_value() as u64 {
@@ -318,7 +319,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
slices.push(io::IoSliceMut::new(slice));
}
let required_rights = EntryRights::from_base(types::Rights::FD_READ);
let required_rights = HandleRights::from_base(types::Rights::FD_READ);
let entry = self.get_entry(fd)?;
let host_nread = entry
.as_handle(&required_rights)?
@@ -335,7 +336,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
buf_len: types::Size,
cookie: types::Dircookie,
) -> Result<types::Size> {
let required_rights = EntryRights::from_base(types::Rights::FD_READDIR);
let required_rights = HandleRights::from_base(types::Rights::FD_READDIR);
let entry = self.get_entry(fd)?;
let mut bufused = 0;
@@ -395,7 +396,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
} else {
types::Rights::FD_SEEK | types::Rights::FD_TELL
};
let required_rights = EntryRights::from_base(base);
let required_rights = HandleRights::from_base(base);
let entry = self.get_entry(fd)?;
let pos = match whence {
types::Whence::Cur => SeekFrom::Current(offset),
@@ -407,13 +408,13 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
fn fd_sync(&self, fd: types::Fd) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::FD_SYNC);
let required_rights = HandleRights::from_base(types::Rights::FD_SYNC);
let entry = self.get_entry(fd)?;
entry.as_handle(&required_rights)?.sync()
}
fn fd_tell(&self, fd: types::Fd) -> Result<types::Filesize> {
let required_rights = EntryRights::from_base(types::Rights::FD_TELL);
let required_rights = HandleRights::from_base(types::Rights::FD_TELL);
let entry = self.get_entry(fd)?;
let host_offset = entry
.as_handle(&required_rights)?
@@ -435,19 +436,19 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
};
slices.push(io::IoSlice::new(slice));
}
let required_rights = EntryRights::from_base(types::Rights::FD_WRITE);
let required_rights = HandleRights::from_base(types::Rights::FD_WRITE);
let entry = self.get_entry(fd)?;
let isatty = entry.isatty();
let host_nwritten = entry
.as_handle(&required_rights)?
.write_vectored(&slices, isatty)?
.write_vectored(&slices)?
.try_into()?;
Ok(host_nwritten)
}
fn path_create_directory(&self, dirfd: types::Fd, path: &GuestPtr<'_, str>) -> Result<()> {
let required_rights =
EntryRights::from_base(types::Rights::PATH_OPEN | types::Rights::PATH_CREATE_DIRECTORY);
let required_rights = HandleRights::from_base(
types::Rights::PATH_OPEN | types::Rights::PATH_CREATE_DIRECTORY,
);
let entry = self.get_entry(dirfd)?;
let (dirfd, path) = path::get(
&entry,
@@ -465,7 +466,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
flags: types::Lookupflags,
path: &GuestPtr<'_, str>,
) -> Result<types::Filestat> {
let required_rights = EntryRights::from_base(types::Rights::PATH_FILESTAT_GET);
let required_rights = HandleRights::from_base(types::Rights::PATH_FILESTAT_GET);
let entry = self.get_entry(dirfd)?;
let (dirfd, path) = path::get(&entry, &required_rights, flags, path, false)?;
let host_filestat = dirfd
@@ -489,7 +490,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
mtim: types::Timestamp,
fst_flags: types::Fstflags,
) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::PATH_FILESTAT_SET_TIMES);
let required_rights = HandleRights::from_base(types::Rights::PATH_FILESTAT_SET_TIMES);
let entry = self.get_entry(dirfd)?;
let (dirfd, path) = path::get(&entry, &required_rights, flags, path, false)?;
dirfd
@@ -512,7 +513,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
new_fd: types::Fd,
new_path: &GuestPtr<'_, str>,
) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::PATH_LINK_SOURCE);
let required_rights = HandleRights::from_base(types::Rights::PATH_LINK_SOURCE);
let old_entry = self.get_entry(old_fd)?;
let (old_dirfd, old_path) = path::get(
&old_entry,
@@ -521,7 +522,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
old_path,
false,
)?;
let required_rights = EntryRights::from_base(types::Rights::PATH_LINK_TARGET);
let required_rights = HandleRights::from_base(types::Rights::PATH_LINK_TARGET);
let new_entry = self.get_entry(new_fd)?;
let (new_dirfd, new_path) = path::get(
&new_entry,
@@ -549,7 +550,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
fdflags: types::Fdflags,
) -> Result<types::Fd> {
let needed_rights = path::open_rights(
&EntryRights::new(fs_rights_base, fs_rights_inheriting),
&HandleRights::new(fs_rights_base, fs_rights_inheriting),
oflags,
fdflags,
);
@@ -577,14 +578,14 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
write
);
let fd = dirfd.openat(&path, read, write, oflags, fdflags)?;
let fe = Entry::from(EntryHandle::from(fd))?;
let entry = Entry::new(EntryHandle::from(fd));
// We need to manually deny the rights which are not explicitly requested
// because Entry::from will assign maximal consistent rights.
let mut rights = fe.rights.get();
let mut rights = entry.get_rights();
rights.base &= fs_rights_base;
rights.inheriting &= fs_rights_inheriting;
fe.rights.set(rights);
let guest_fd = self.insert_entry(fe)?;
entry.set_rights(rights);
let guest_fd = self.insert_entry(entry)?;
Ok(guest_fd)
}
@@ -595,7 +596,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
buf: &GuestPtr<u8>,
buf_len: types::Size,
) -> Result<types::Size> {
let required_rights = EntryRights::from_base(types::Rights::PATH_READLINK);
let required_rights = HandleRights::from_base(types::Rights::PATH_READLINK);
let entry = self.get_entry(dirfd)?;
let (dirfd, path) = path::get(
&entry,
@@ -615,7 +616,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
fn path_remove_directory(&self, dirfd: types::Fd, path: &GuestPtr<'_, str>) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::PATH_REMOVE_DIRECTORY);
let required_rights = HandleRights::from_base(types::Rights::PATH_REMOVE_DIRECTORY);
let entry = self.get_entry(dirfd)?;
let (dirfd, path) = path::get(
&entry,
@@ -634,7 +635,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
new_fd: types::Fd,
new_path: &GuestPtr<'_, str>,
) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::PATH_RENAME_SOURCE);
let required_rights = HandleRights::from_base(types::Rights::PATH_RENAME_SOURCE);
let entry = self.get_entry(old_fd)?;
let (old_dirfd, old_path) = path::get(
&entry,
@@ -643,7 +644,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
old_path,
true,
)?;
let required_rights = EntryRights::from_base(types::Rights::PATH_RENAME_TARGET);
let required_rights = HandleRights::from_base(types::Rights::PATH_RENAME_TARGET);
let entry = self.get_entry(new_fd)?;
let (new_dirfd, new_path) = path::get(
&entry,
@@ -661,7 +662,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
dirfd: types::Fd,
new_path: &GuestPtr<'_, str>,
) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::PATH_SYMLINK);
let required_rights = HandleRights::from_base(types::Rights::PATH_SYMLINK);
let entry = self.get_entry(dirfd)?;
let (new_fd, new_path) = path::get(
&entry,
@@ -680,7 +681,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
fn path_unlink_file(&self, dirfd: types::Fd, path: &GuestPtr<'_, str>) -> Result<()> {
let required_rights = EntryRights::from_base(types::Rights::PATH_UNLINK_FILE);
let required_rights = HandleRights::from_base(types::Rights::PATH_UNLINK_FILE);
let entry = self.get_entry(dirfd)?;
let (dirfd, path) = path::get(
&entry,
@@ -740,7 +741,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
types::SubscriptionU::FdRead(fd_read) => {
let fd = fd_read.file_descriptor;
let required_rights = EntryRights::from_base(
let required_rights = HandleRights::from_base(
types::Rights::FD_READ | types::Rights::POLL_FD_READWRITE,
);
let entry = match self.get_entry(fd) {
@@ -766,7 +767,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
}
types::SubscriptionU::FdWrite(fd_write) => {
let fd = fd_write.file_descriptor;
let required_rights = EntryRights::from_base(
let required_rights = HandleRights::from_base(
types::Rights::FD_WRITE | types::Rights::POLL_FD_READWRITE,
);
let entry = match self.get_entry(fd) {