From 33163ffb6356cc695ff19c2d785d14abe7ad7412 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sun, 19 Apr 2020 10:34:32 +0200 Subject: [PATCH 01/16] 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. --- crates/wasi-common/src/ctx.rs | 39 +-- crates/wasi-common/src/entry.rs | 107 ++----- crates/wasi-common/src/handle.rs | 63 +++- crates/wasi-common/src/path.rs | 8 +- .../src/snapshots/wasi_snapshot_preview1.rs | 96 +++--- crates/wasi-common/src/sys/mod.rs | 45 ++- crates/wasi-common/src/sys/osdir.rs | 138 +++++++++ crates/wasi-common/src/sys/osfile.rs | 143 +++++++++ crates/wasi-common/src/sys/oshandle.rs | 277 ------------------ crates/wasi-common/src/sys/osother.rs | 85 ++++++ crates/wasi-common/src/sys/stdio.rs | 91 ++++++ crates/wasi-common/src/sys/unix/bsd/mod.rs | 2 +- crates/wasi-common/src/sys/unix/bsd/osfile.rs | 109 ------- .../wasi-common/src/sys/unix/bsd/oshandle.rs | 42 +++ crates/wasi-common/src/sys/unix/bsd/path.rs | 10 +- .../src/sys/unix/emscripten/mod.rs | 4 +- crates/wasi-common/src/sys/unix/fd.rs | 10 +- crates/wasi-common/src/sys/unix/linux/mod.rs | 2 +- .../wasi-common/src/sys/unix/linux/osfile.rs | 82 ------ .../src/sys/unix/linux/oshandle.rs | 44 +++ crates/wasi-common/src/sys/unix/linux/path.rs | 10 +- crates/wasi-common/src/sys/unix/mod.rs | 46 +++ crates/wasi-common/src/sys/unix/osdir.rs | 42 +++ crates/wasi-common/src/sys/unix/osfile.rs | 52 ++++ crates/wasi-common/src/sys/unix/oshandle.rs | 148 +++------- crates/wasi-common/src/sys/unix/osother.rs | 66 +++++ crates/wasi-common/src/sys/unix/path.rs | 33 ++- crates/wasi-common/src/sys/unix/poll.rs | 58 ++-- crates/wasi-common/src/sys/unix/stdio.rs | 66 +++++ crates/wasi-common/src/sys/windows/fd.rs | 14 +- crates/wasi-common/src/sys/windows/mod.rs | 41 +++ crates/wasi-common/src/sys/windows/osdir.rs | 42 +++ crates/wasi-common/src/sys/windows/osfile.rs | 49 ++++ .../wasi-common/src/sys/windows/oshandle.rs | 145 ++------- crates/wasi-common/src/sys/windows/osother.rs | 48 +++ crates/wasi-common/src/sys/windows/path.rs | 43 +-- crates/wasi-common/src/sys/windows/poll.rs | 135 +++++---- crates/wasi-common/src/sys/windows/stdio.rs | 39 +++ crates/wasi-common/src/virtfs.rs | 66 +++-- 39 files changed, 1532 insertions(+), 1008 deletions(-) create mode 100644 crates/wasi-common/src/sys/osdir.rs create mode 100644 crates/wasi-common/src/sys/osfile.rs delete mode 100644 crates/wasi-common/src/sys/oshandle.rs create mode 100644 crates/wasi-common/src/sys/osother.rs create mode 100644 crates/wasi-common/src/sys/stdio.rs delete mode 100644 crates/wasi-common/src/sys/unix/bsd/osfile.rs create mode 100644 crates/wasi-common/src/sys/unix/bsd/oshandle.rs delete mode 100644 crates/wasi-common/src/sys/unix/linux/osfile.rs create mode 100644 crates/wasi-common/src/sys/unix/linux/oshandle.rs create mode 100644 crates/wasi-common/src/sys/unix/osdir.rs create mode 100644 crates/wasi-common/src/sys/unix/osfile.rs create mode 100644 crates/wasi-common/src/sys/unix/osother.rs create mode 100644 crates/wasi-common/src/sys/unix/stdio.rs create mode 100644 crates/wasi-common/src/sys/windows/osdir.rs create mode 100644 crates/wasi-common/src/sys/windows/osfile.rs create mode 100644 crates/wasi-common/src/sys/windows/osother.rs create mode 100644 crates/wasi-common/src/sys/windows/stdio.rs diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index de66d81814c3..5bbe1582615d 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -1,13 +1,15 @@ use crate::entry::{Entry, EntryHandle}; use crate::fdpool::FdPool; use crate::handle::Handle; -use crate::sys::oshandle::{OsHandle, OsHandleExt}; +use crate::sys::osfile::{OsFile, OsFileExt}; +use crate::sys::stdio::{Stdio, StdioExt}; use crate::virtfs::{VirtualDir, VirtualDirEntry}; use crate::wasi::types; use crate::wasi::{Errno, Result}; use std::borrow::Borrow; use std::cell::RefCell; use std::collections::HashMap; +use std::convert::TryFrom; use std::ffi::{self, CString, OsString}; use std::fs::File; use std::path::{Path, PathBuf}; @@ -43,7 +45,7 @@ pub enum WasiCtxBuilderError { type WasiCtxBuilderResult = std::result::Result; enum PendingEntry { - Thunk(fn() -> io::Result), + Thunk(fn() -> io::Result>), File(File), } @@ -53,7 +55,7 @@ impl std::fmt::Debug for PendingEntry { Self::Thunk(f) => write!( fmt, "PendingEntry::Thunk({:p})", - f as *const fn() -> io::Result + f as *const fn() -> io::Result> ), Self::File(f) => write!(fmt, "PendingEntry::File({:?})", f), } @@ -118,9 +120,9 @@ pub struct WasiCtxBuilder { impl WasiCtxBuilder { /// Builder for a new `WasiCtx`. pub fn new() -> Self { - let stdin = Some(PendingEntry::Thunk(OsHandle::from_null)); - let stdout = Some(PendingEntry::Thunk(OsHandle::from_null)); - let stderr = Some(PendingEntry::Thunk(OsHandle::from_null)); + let stdin = Some(PendingEntry::Thunk(OsFile::from_null)); + let stdout = Some(PendingEntry::Thunk(OsFile::from_null)); + let stderr = Some(PendingEntry::Thunk(OsFile::from_null)); Self { stdin, @@ -167,27 +169,27 @@ impl WasiCtxBuilder { /// Inherit stdin from the host process. pub fn inherit_stdin(&mut self) -> &mut Self { - self.stdin = Some(PendingEntry::Thunk(|| Ok(OsHandle::stdin()))); + self.stdin = Some(PendingEntry::Thunk(Stdio::stdin)); self } /// Inherit stdout from the host process. pub fn inherit_stdout(&mut self) -> &mut Self { - self.stdout = Some(PendingEntry::Thunk(|| Ok(OsHandle::stdout()))); + self.stdout = Some(PendingEntry::Thunk(Stdio::stdout)); self } /// Inherit stdout from the host process. pub fn inherit_stderr(&mut self) -> &mut Self { - self.stderr = Some(PendingEntry::Thunk(|| Ok(OsHandle::stderr()))); + self.stderr = Some(PendingEntry::Thunk(Stdio::stderr)); self } /// Inherit the stdin, stdout, and stderr streams from the host process. pub fn inherit_stdio(&mut self) -> &mut Self { - self.stdin = Some(PendingEntry::Thunk(|| Ok(OsHandle::stdin()))); - self.stdout = Some(PendingEntry::Thunk(|| Ok(OsHandle::stdout()))); - self.stderr = Some(PendingEntry::Thunk(|| Ok(OsHandle::stderr()))); + self.stdin = Some(PendingEntry::Thunk(Stdio::stdin)); + self.stdout = Some(PendingEntry::Thunk(Stdio::stdout)); + self.stderr = Some(PendingEntry::Thunk(Stdio::stderr)); self } @@ -251,7 +253,7 @@ impl WasiCtxBuilder { pub fn preopened_dir>(&mut self, dir: File, guest_path: P) -> &mut Self { self.preopens.as_mut().unwrap().push(( guest_path.as_ref().to_owned(), - Box::new(OsHandle::from(dir)), + >::try_from(dir).expect("valid handle"), )); self } @@ -336,15 +338,16 @@ impl WasiCtxBuilder { log::debug!("WasiCtx inserting entry {:?}", pending); let fd = match pending { PendingEntry::Thunk(f) => { - let handle = EntryHandle::new(f()?); - let entry = Entry::from(handle)?; + let handle = EntryHandle::from(f()?); + let entry = Entry::new(handle); entries .insert(entry) .ok_or(WasiCtxBuilderError::TooManyFilesOpen)? } PendingEntry::File(f) => { - let handle = EntryHandle::new(OsHandle::from(f)); - let entry = Entry::from(handle)?; + let handle = >::try_from(f)?; + let handle = EntryHandle::from(handle); + let entry = Entry::new(handle); entries .insert(entry) .ok_or(WasiCtxBuilderError::TooManyFilesOpen)? @@ -359,7 +362,7 @@ impl WasiCtxBuilder { } let handle = EntryHandle::from(dir); - let mut entry = Entry::from(handle)?; + let mut entry = Entry::new(handle); entry.preopen_path = Some(guest_path); let fd = entries .insert(entry) diff --git a/crates/wasi-common/src/entry.rs b/crates/wasi-common/src/entry.rs index debd904617e4..1b219efbedc3 100644 --- a/crates/wasi-common/src/entry.rs +++ b/crates/wasi-common/src/entry.rs @@ -1,19 +1,13 @@ -use crate::handle::Handle; +use crate::handle::{Handle, HandleRights}; use crate::wasi::types::{Filetype, Rights}; use crate::wasi::{Errno, Result}; -use std::cell::Cell; use std::ops::Deref; use std::path::PathBuf; use std::rc::Rc; -use std::{fmt, io}; pub(crate) struct EntryHandle(Rc); impl EntryHandle { - pub(crate) fn new(handle: T) -> Self { - Self(Rc::new(handle)) - } - pub(crate) fn get(&self) -> Self { Self(Rc::clone(&self.0)) } @@ -33,118 +27,79 @@ impl Deref for EntryHandle { } } -/// An abstraction struct serving as a wrapper for a host `Descriptor` object which requires -/// certain rights `rights` in order to be accessed correctly. +/// An abstraction struct serving as a wrapper for a `Handle` object. /// -/// Here, the `descriptor` field stores the host `Descriptor` object (such as a file descriptor, or -/// stdin handle), and accessing it can only be done via the provided `Entry::as_descriptor` method +/// Here, the `handle` field stores an instance of `Handle` type (such as a file descriptor, or +/// stdin handle), and accessing it can only be done via the provided `Entry::as_handle` method /// which require a set of base and inheriting rights to be specified, verifying whether the stored -/// `Descriptor` object is valid for the rights specified. +/// `Handle` object is valid for the rights specified. pub(crate) struct Entry { - pub(crate) file_type: Filetype, handle: EntryHandle, - pub(crate) rights: Cell, pub(crate) preopen_path: Option, // TODO: directories } -/// Represents rights of an `Entry` entity, either already held or -/// required. -#[derive(Debug, Copy, Clone)] -pub(crate) struct EntryRights { - pub(crate) base: Rights, - pub(crate) inheriting: Rights, -} - -impl EntryRights { - pub(crate) fn new(base: Rights, inheriting: Rights) -> Self { - Self { base, inheriting } - } - - /// Create new `EntryRights` instance from `base` rights only, keeping - /// `inheriting` set to none. - pub(crate) fn from_base(base: Rights) -> Self { - Self { - base, - inheriting: Rights::empty(), - } - } - - /// Create new `EntryRights` instance with both `base` and `inheriting` - /// rights set to none. - pub(crate) fn empty() -> Self { +impl Entry { + pub(crate) fn new(handle: EntryHandle) -> Self { + let preopen_path = None; Self { - base: Rights::empty(), - inheriting: Rights::empty(), + handle, + preopen_path, } } - /// Check if `other` is a subset of those rights. - pub(crate) fn contains(&self, other: &Self) -> bool { - self.base.contains(&other.base) && self.inheriting.contains(&other.inheriting) + pub(crate) fn get_file_type(&self) -> Filetype { + self.handle.get_file_type() } -} -impl fmt::Display for EntryRights { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "EntryRights {{ base: {}, inheriting: {} }}", - self.base, self.inheriting - ) + pub(crate) fn get_rights(&self) -> HandleRights { + self.handle.get_rights() } -} -impl Entry { - pub(crate) fn from(handle: EntryHandle) -> io::Result { - let file_type = handle.get_file_type()?; - let rights = handle.get_rights()?; - Ok(Self { - file_type, - handle, - rights: Cell::new(rights), - preopen_path: None, - }) + pub(crate) fn set_rights(&self, rights: HandleRights) { + self.handle.set_rights(rights) } - /// Convert this `Entry` into a host `Descriptor` object provided the specified + /// Convert this `Entry` into a `Handle` object provided the specified /// `rights` rights are set on this `Entry` object. /// - /// The `Entry` can only be converted into a valid `Descriptor` object if + /// The `Entry` can only be converted into a valid `Handle` object if /// the specified set of base rights, and inheriting rights encapsulated within `rights` - /// `EntryRights` structure is a subset of rights attached to this `Entry`. The check is + /// `HandleRights` structure is a subset of rights attached to this `Entry`. The check is /// performed using `Entry::validate_rights` method. If the check fails, `Errno::Notcapable` /// is returned. - pub(crate) fn as_handle(&self, rights: &EntryRights) -> Result { + pub(crate) fn as_handle(&self, rights: &HandleRights) -> Result { self.validate_rights(rights)?; Ok(self.handle.get()) } - /// Check if this `Entry` object satisfies the specified `EntryRights`; i.e., if + /// Check if this `Entry` object satisfies the specified `HandleRights`; i.e., if /// rights attached to this `Entry` object are a superset. /// /// Upon unsuccessful check, `Errno::Notcapable` is returned. - pub(crate) fn validate_rights(&self, rights: &EntryRights) -> Result<()> { - if self.rights.get().contains(rights) { + pub(crate) fn validate_rights(&self, rights: &HandleRights) -> Result<()> { + let this_rights = self.handle.get_rights(); + if this_rights.contains(rights) { Ok(()) } else { log::trace!( " | validate_rights failed: required rights = {}; actual rights = {}", rights, - self.rights.get(), + this_rights, ); Err(Errno::Notcapable) } } + // TODO we should probably have a separate struct representing + // char device /// Test whether this descriptor is considered a tty within WASI. /// Note that since WASI itself lacks an `isatty` syscall and relies /// on a conservative approximation, we use the same approximation here. pub(crate) fn isatty(&self) -> bool { - self.file_type == Filetype::CharacterDevice - && self - .rights - .get() - .contains(&EntryRights::from_base(Rights::FD_SEEK | Rights::FD_TELL)) + let file_type = self.handle.get_file_type(); + let rights = self.handle.get_rights(); + let required_rights = HandleRights::from_base(Rights::FD_SEEK | Rights::FD_TELL); + file_type == Filetype::CharacterDevice && rights.contains(&required_rights) } } diff --git a/crates/wasi-common/src/handle.rs b/crates/wasi-common/src/handle.rs index 671ae2d2a1cc..8d0cf2191fe6 100644 --- a/crates/wasi-common/src/handle.rs +++ b/crates/wasi-common/src/handle.rs @@ -1,20 +1,65 @@ -use crate::entry::EntryRights; -use crate::wasi::{types, Errno, Result}; +use crate::wasi::types::{self, Rights}; +use crate::wasi::{Errno, Result}; use std::any::Any; +use std::fmt; use std::io::{self, SeekFrom}; +/// Represents rights of a `Handle`, either already held or required. +#[derive(Debug, Copy, Clone)] +pub(crate) struct HandleRights { + pub(crate) base: Rights, + pub(crate) inheriting: Rights, +} + +impl HandleRights { + pub(crate) fn new(base: Rights, inheriting: Rights) -> Self { + Self { base, inheriting } + } + + /// Create new `HandleRights` instance from `base` rights only, keeping + /// `inheriting` set to none. + pub(crate) fn from_base(base: Rights) -> Self { + Self { + base, + inheriting: Rights::empty(), + } + } + + /// Create new `HandleRights` instance with both `base` and `inheriting` + /// rights set to none. + pub(crate) fn empty() -> Self { + Self { + base: Rights::empty(), + inheriting: Rights::empty(), + } + } + + /// Check if `other` is a subset of those rights. + pub(crate) fn contains(&self, other: &Self) -> bool { + self.base.contains(&other.base) && self.inheriting.contains(&other.inheriting) + } +} + +impl fmt::Display for HandleRights { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "HandleRights {{ base: {}, inheriting: {} }}", + self.base, self.inheriting + ) + } +} + pub(crate) trait Handle { fn as_any(&self) -> &dyn Any; fn try_clone(&self) -> io::Result>; - fn get_file_type(&self) -> io::Result; - fn get_rights(&self) -> io::Result { - Ok(EntryRights::empty()) + fn get_file_type(&self) -> types::Filetype; + fn get_rights(&self) -> HandleRights { + HandleRights::empty() } + fn set_rights(&self, rights: HandleRights); fn is_directory(&self) -> bool { - if let Ok(ft) = self.get_file_type() { - return ft == types::Filetype::Directory; - } - false + self.get_file_type() == types::Filetype::Directory } // TODO perhaps should be a separate trait? // FdOps diff --git a/crates/wasi-common/src/path.rs b/crates/wasi-common/src/path.rs index 515ed18019d6..f5399a7b7ee4 100644 --- a/crates/wasi-common/src/path.rs +++ b/crates/wasi-common/src/path.rs @@ -1,5 +1,5 @@ -use crate::entry::{Entry, EntryRights}; -use crate::handle::Handle; +use crate::entry::Entry; +use crate::handle::{Handle, HandleRights}; use crate::wasi::{types, Errno, Result}; use std::path::{Component, Path}; use std::str; @@ -12,7 +12,7 @@ pub(crate) use crate::sys::path::{from_host, open_rights}; /// This is a workaround for not having Capsicum support in the OS. pub(crate) fn get( entry: &Entry, - required_rights: &EntryRights, + required_rights: &HandleRights, dirflags: types::Lookupflags, path: &GuestPtr<'_, str>, needs_final_component: bool, @@ -33,7 +33,7 @@ pub(crate) fn get( return Err(Errno::Ilseq); } - if entry.file_type != types::Filetype::Directory { + if entry.get_file_type() != types::Filetype::Directory { // if `dirfd` doesn't refer to a directory, return `Notdir`. return Err(Errno::Notdir); } diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index 23649b20b495..865f8f489b2d 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -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 { 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 { - 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 { // 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 { - 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 { - 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,7 +436,7 @@ 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 @@ -446,8 +447,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } 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 +467,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { flags: types::Lookupflags, path: &GuestPtr<'_, str>, ) -> Result { - 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 +491,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 +514,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 +523,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 +551,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fdflags: types::Fdflags, ) -> Result { 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 +579,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 +597,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { buf: &GuestPtr, buf_len: types::Size, ) -> Result { - 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 +617,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 +636,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 +645,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 +663,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 +682,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 +742,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 +768,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) { diff --git a/crates/wasi-common/src/sys/mod.rs b/crates/wasi-common/src/sys/mod.rs index 61ed66700f4d..c1652a9a59d4 100644 --- a/crates/wasi-common/src/sys/mod.rs +++ b/crates/wasi-common/src/sys/mod.rs @@ -1,6 +1,9 @@ pub(crate) mod clock; pub(crate) mod fd; -pub(crate) mod oshandle; +pub(crate) mod osdir; +pub(crate) mod osfile; +pub(crate) mod osother; +pub(crate) mod stdio; use cfg_if::cfg_if; @@ -20,3 +23,43 @@ cfg_if! { pub(crate) use sys_impl::path; pub(crate) use sys_impl::poll; + +use super::handle::Handle; +use crate::wasi::types; +use osdir::OsDir; +use osfile::OsFile; +use osother::OsOther; +use std::convert::TryFrom; +use std::fs::File; +use std::io; +use std::mem::ManuallyDrop; +use sys_impl::get_file_type; + +pub(crate) trait AsFile { + fn as_file(&self) -> ManuallyDrop; +} + +impl TryFrom for Box { + type Error = io::Error; + + fn try_from(file: File) -> io::Result { + let file_type = get_file_type(&file)?; + match file_type { + types::Filetype::RegularFile => { + let handle = OsFile::try_from(file)?; + log::debug!("Created new instance of OsFile: {:?}", handle); + Ok(Box::new(handle)) + } + types::Filetype::Directory => { + let handle = OsDir::try_from(file)?; + log::debug!("Created new instance of OsDir: {:?}", handle); + Ok(Box::new(handle)) + } + _ => { + let handle = OsOther::try_from(file)?; + log::debug!("Created new instance of OsOther: {:?}", handle); + Ok(Box::new(handle)) + } + } + } +} diff --git a/crates/wasi-common/src/sys/osdir.rs b/crates/wasi-common/src/sys/osdir.rs new file mode 100644 index 000000000000..406b239c2696 --- /dev/null +++ b/crates/wasi-common/src/sys/osdir.rs @@ -0,0 +1,138 @@ +use super::sys_impl::oshandle::OsDirHandle; +use super::{fd, path, AsFile}; +use crate::handle::{Handle, HandleRights}; +use crate::wasi::{types, Errno, Result}; +use log::{debug, error}; +use std::any::Any; +use std::cell::Cell; +use std::io; +use std::ops::Deref; + +#[derive(Debug)] +pub(crate) struct OsDir { + rights: Cell, + handle: OsDirHandle, +} + +impl OsDir { + pub(super) fn new(rights: HandleRights, handle: OsDirHandle) -> Self { + let rights = Cell::new(rights); + Self { rights, handle } + } +} + +impl Deref for OsDir { + type Target = OsDirHandle; + + fn deref(&self) -> &Self::Target { + &self.handle + } +} + +impl Handle for OsDir { + fn as_any(&self) -> &dyn Any { + self + } + fn try_clone(&self) -> io::Result> { + let handle = self.handle.try_clone()?; + let rights = self.rights.clone(); + Ok(Box::new(Self { rights, handle })) + } + fn get_file_type(&self) -> types::Filetype { + types::Filetype::Directory + } + fn get_rights(&self) -> HandleRights { + self.rights.get() + } + fn set_rights(&self, rights: HandleRights) { + self.rights.set(rights) + } + // FdOps + fn fdstat_get(&self) -> Result { + fd::fdstat_get(&self.as_file()) + } + fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { + if let Some(new_file) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { + self.handle.update_from(new_file); + } + Ok(()) + } + fn filestat_get(&self) -> Result { + fd::filestat_get(&self.as_file()) + } + fn filestat_set_times( + &self, + atim: types::Timestamp, + mtim: types::Timestamp, + fst_flags: types::Fstflags, + ) -> Result<()> { + fd::filestat_set_times(&self.as_file(), atim, mtim, fst_flags) + } + fn readdir<'a>( + &'a self, + cookie: types::Dircookie, + ) -> Result> + 'a>> { + fd::readdir(self, cookie) + } + // PathOps + fn create_directory(&self, path: &str) -> Result<()> { + path::create_directory(self, path) + } + fn openat( + &self, + path: &str, + read: bool, + write: bool, + oflags: types::Oflags, + fd_flags: types::Fdflags, + ) -> Result> { + path::open(self, path, read, write, oflags, fd_flags) + } + fn link( + &self, + old_path: &str, + new_handle: Box, + new_path: &str, + follow: bool, + ) -> Result<()> { + let new_handle = match new_handle.as_any().downcast_ref::() { + None => { + error!("Tried to link OS resource with Virtual"); + return Err(Errno::Badf); + } + Some(handle) => handle, + }; + path::link(self, old_path, new_handle, new_path, follow) + } + fn symlink(&self, old_path: &str, new_path: &str) -> Result<()> { + path::symlink(old_path, self, new_path) + } + fn readlink(&self, path: &str, buf: &mut [u8]) -> Result { + path::readlink(self, path, buf) + } + fn readlinkat(&self, path: &str) -> Result { + path::readlinkat(self, path) + } + fn rename(&self, old_path: &str, new_handle: Box, new_path: &str) -> Result<()> { + let new_handle = match new_handle.as_any().downcast_ref::() { + None => { + error!("Tried to link OS resource with Virtual"); + return Err(Errno::Badf); + } + Some(handle) => handle, + }; + debug!("rename (old_dirfd, old_path)=({:?}, {:?})", self, old_path); + debug!( + "rename (new_dirfd, new_path)=({:?}, {:?})", + new_handle, new_path + ); + path::rename(self, old_path, new_handle, new_path) + } + fn remove_directory(&self, path: &str) -> Result<()> { + debug!("remove_directory (dirfd, path)=({:?}, {:?})", self, path); + path::remove_directory(self, path) + } + fn unlink_file(&self, path: &str) -> Result<()> { + path::unlink_file(self, path) + } +} diff --git a/crates/wasi-common/src/sys/osfile.rs b/crates/wasi-common/src/sys/osfile.rs new file mode 100644 index 000000000000..ed255389f563 --- /dev/null +++ b/crates/wasi-common/src/sys/osfile.rs @@ -0,0 +1,143 @@ +use super::sys_impl::oshandle::OsHandle; +use super::{fd, AsFile}; +use crate::handle::{Handle, HandleRights}; +use crate::sandboxed_tty_writer::SandboxedTTYWriter; +use crate::wasi::{types, Errno, Result}; +use std::any::Any; +use std::cell::Cell; +use std::fs::File; +use std::io::{self, Read, Seek, SeekFrom, Write}; +use std::ops::Deref; + +pub(crate) trait OsFileExt { + fn from_null() -> io::Result>; +} + +#[derive(Debug)] +pub(crate) struct OsFile { + rights: Cell, + handle: OsHandle, +} + +impl OsFile { + pub(super) fn new(rights: HandleRights, handle: OsHandle) -> Self { + let rights = Cell::new(rights); + Self { rights, handle } + } +} + +impl Deref for OsFile { + type Target = OsHandle; + + fn deref(&self) -> &Self::Target { + &self.handle + } +} + +impl Handle for OsFile { + fn as_any(&self) -> &dyn Any { + self + } + fn try_clone(&self) -> io::Result> { + let handle = self.handle.try_clone()?; + let rights = self.rights.clone(); + Ok(Box::new(Self { rights, handle })) + } + fn get_file_type(&self) -> types::Filetype { + types::Filetype::RegularFile + } + fn get_rights(&self) -> HandleRights { + self.rights.get() + } + fn set_rights(&self, rights: HandleRights) { + self.rights.set(rights) + } + // FdOps + fn advise( + &self, + advice: types::Advice, + offset: types::Filesize, + len: types::Filesize, + ) -> Result<()> { + fd::advise(self, advice, offset, len) + } + fn allocate(&self, offset: types::Filesize, len: types::Filesize) -> Result<()> { + let fd = self.as_file(); + let metadata = fd.metadata()?; + let current_size = metadata.len(); + let wanted_size = offset.checked_add(len).ok_or(Errno::TooBig)?; + // This check will be unnecessary when rust-lang/rust#63326 is fixed + if wanted_size > i64::max_value() as u64 { + return Err(Errno::TooBig); + } + if wanted_size > current_size { + fd.set_len(wanted_size)?; + } + Ok(()) + } + fn datasync(&self) -> Result<()> { + self.as_file().sync_data()?; + Ok(()) + } + fn fdstat_get(&self) -> Result { + fd::fdstat_get(&self.as_file()) + } + fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { + if let Some(new_handle) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { + self.handle.update_from(new_handle); + } + Ok(()) + } + fn filestat_get(&self) -> Result { + fd::filestat_get(&self.as_file()) + } + fn filestat_set_size(&self, size: types::Filesize) -> Result<()> { + self.as_file().set_len(size)?; + Ok(()) + } + fn filestat_set_times( + &self, + atim: types::Timestamp, + mtim: types::Timestamp, + fst_flags: types::Fstflags, + ) -> Result<()> { + fd::filestat_set_times(&self.as_file(), atim, mtim, fst_flags) + } + fn preadv(&self, buf: &mut [io::IoSliceMut], offset: u64) -> Result { + let mut fd: &File = &self.as_file(); + let cur_pos = fd.seek(SeekFrom::Current(0))?; + fd.seek(SeekFrom::Start(offset))?; + let nread = self.read_vectored(buf)?; + fd.seek(SeekFrom::Start(cur_pos))?; + Ok(nread) + } + fn pwritev(&self, buf: &[io::IoSlice], offset: u64) -> Result { + let mut fd: &File = &self.as_file(); + let cur_pos = fd.seek(SeekFrom::Current(0))?; + fd.seek(SeekFrom::Start(offset))?; + let nwritten = self.write_vectored(&buf, false)?; + fd.seek(SeekFrom::Start(cur_pos))?; + Ok(nwritten) + } + fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result { + let nread = self.as_file().read_vectored(iovs)?; + Ok(nread) + } + fn seek(&self, offset: SeekFrom) -> Result { + let pos = self.as_file().seek(offset)?; + Ok(pos) + } + fn sync(&self) -> Result<()> { + self.as_file().sync_all()?; + Ok(()) + } + fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result { + let mut file: &File = &self.as_file(); + let nwritten = if isatty { + SandboxedTTYWriter::new(&mut file).write_vectored(&iovs)? + } else { + file.write_vectored(&iovs)? + }; + Ok(nwritten) + } +} diff --git a/crates/wasi-common/src/sys/oshandle.rs b/crates/wasi-common/src/sys/oshandle.rs deleted file mode 100644 index eeff049cce8f..000000000000 --- a/crates/wasi-common/src/sys/oshandle.rs +++ /dev/null @@ -1,277 +0,0 @@ -use super::{fd, path}; -use crate::entry::EntryRights; -use crate::handle::Handle; -use crate::sandboxed_tty_writer::SandboxedTTYWriter; -use crate::wasi::{types, Errno, Result}; -use log::{debug, error}; -use std::any::Any; -use std::fs::File; -use std::io::{self, Read, Seek, SeekFrom, Write}; -use std::mem::ManuallyDrop; - -pub(crate) use super::sys_impl::oshandle::*; - -#[derive(Debug)] -pub(crate) enum OsHandle { - OsFile(OsFile), - Stdin, - Stdout, - Stderr, -} - -impl OsHandle { - pub(crate) fn as_os_file(&self) -> Result<&OsFile> { - match self { - Self::OsFile(fd) => Ok(fd), - _ => Err(Errno::Badf), - } - } - - pub(crate) fn stdin() -> Self { - Self::Stdin - } - - pub(crate) fn stdout() -> Self { - Self::Stdout - } - - pub(crate) fn stderr() -> Self { - Self::Stderr - } -} - -pub(crate) trait AsFile { - fn as_file(&self) -> ManuallyDrop; -} - -pub(crate) trait OsHandleExt: Sized { - /// Returns the file type. - fn get_file_type(&self) -> io::Result; - /// Returns the set of all possible rights that are both relevant for the file - /// type and consistent with the open mode. - fn get_rights(&self, filetype: types::Filetype) -> io::Result; - fn from_null() -> io::Result; -} - -impl From for OsHandle { - fn from(file: OsFile) -> Self { - Self::OsFile(file) - } -} - -impl Handle for OsHandle { - fn as_any(&self) -> &dyn Any { - self - } - fn try_clone(&self) -> io::Result> { - let new_handle = match self { - Self::OsFile(file) => Self::OsFile(file.try_clone()?), - Self::Stdin => Self::Stdin, - Self::Stdout => Self::Stdout, - Self::Stderr => Self::Stderr, - }; - Ok(Box::new(new_handle)) - } - fn get_file_type(&self) -> io::Result { - ::get_file_type(self) - } - - fn get_rights(&self) -> io::Result { - ::get_rights(self, ::get_file_type(self)?) - } - // FdOps - fn advise( - &self, - advice: types::Advice, - offset: types::Filesize, - len: types::Filesize, - ) -> Result<()> { - fd::advise(self.as_os_file()?, advice, offset, len) - } - fn allocate(&self, offset: types::Filesize, len: types::Filesize) -> Result<()> { - let fd = self.as_file(); - let metadata = fd.metadata()?; - let current_size = metadata.len(); - let wanted_size = offset.checked_add(len).ok_or(Errno::TooBig)?; - // This check will be unnecessary when rust-lang/rust#63326 is fixed - if wanted_size > i64::max_value() as u64 { - return Err(Errno::TooBig); - } - if wanted_size > current_size { - fd.set_len(wanted_size)?; - } - Ok(()) - } - fn datasync(&self) -> Result<()> { - self.as_file().sync_data()?; - Ok(()) - } - fn fdstat_get(&self) -> Result { - fd::fdstat_get(&self.as_file()) - } - fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { - if let Some(new_file) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { - // If we don't deal with OsFile, then something went wrong, and we - // should fail. On the other hand, is that even possible? - self.as_os_file()?.update_from(new_file); - } - Ok(()) - } - fn filestat_get(&self) -> Result { - fd::filestat_get(&self.as_file()) - } - fn filestat_set_size(&self, size: types::Filesize) -> Result<()> { - self.as_os_file()?.as_file().set_len(size)?; - Ok(()) - } - fn filestat_set_times( - &self, - atim: types::Timestamp, - mtim: types::Timestamp, - fst_flags: types::Fstflags, - ) -> Result<()> { - fd::filestat_set_times(&self.as_file(), atim, mtim, fst_flags) - } - fn preadv(&self, buf: &mut [io::IoSliceMut], offset: u64) -> Result { - let mut fd: &File = &self.as_os_file()?.as_file(); - let cur_pos = fd.seek(SeekFrom::Current(0))?; - fd.seek(SeekFrom::Start(offset))?; - let nread = self.read_vectored(buf)?; - fd.seek(SeekFrom::Start(cur_pos))?; - Ok(nread) - } - fn pwritev(&self, buf: &[io::IoSlice], offset: u64) -> Result { - let mut fd: &File = &self.as_os_file()?.as_file(); - let cur_pos = fd.seek(SeekFrom::Current(0))?; - fd.seek(SeekFrom::Start(offset))?; - let nwritten = self.write_vectored(&buf, false)?; - fd.seek(SeekFrom::Start(cur_pos))?; - Ok(nwritten) - } - fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result { - let nread = match self { - Self::OsFile(file) => file.as_file().read_vectored(iovs)?, - Self::Stdin => io::stdin().read_vectored(iovs)?, - _ => return Err(Errno::Badf), - }; - Ok(nread) - } - fn readdir<'a>( - &'a self, - cookie: types::Dircookie, - ) -> Result> + 'a>> { - fd::readdir(self.as_os_file()?, cookie) - } - fn seek(&self, offset: SeekFrom) -> Result { - let pos = self.as_os_file()?.as_file().seek(offset)?; - Ok(pos) - } - fn sync(&self) -> Result<()> { - self.as_os_file()?.as_file().sync_all()?; - Ok(()) - } - fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result { - let nwritten = match self { - Self::OsFile(file) => { - let mut file: &File = &file.as_file(); - if isatty { - SandboxedTTYWriter::new(&mut file).write_vectored(&iovs)? - } else { - file.write_vectored(&iovs)? - } - } - Self::Stdin => return Err(Errno::Badf), - Self::Stdout => { - // lock for the duration of the scope - let stdout = io::stdout(); - let mut stdout = stdout.lock(); - let nwritten = if isatty { - SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)? - } else { - stdout.write_vectored(&iovs)? - }; - stdout.flush()?; - nwritten - } - // Always sanitize stderr, even if it's not directly connected to a tty, - // because stderr is meant for diagnostics rather than binary output, - // and may be redirected to a file which could end up being displayed - // on a tty later. - Self::Stderr => SandboxedTTYWriter::new(&mut io::stderr()).write_vectored(&iovs)?, - }; - Ok(nwritten) - } - // PathOps - fn create_directory(&self, path: &str) -> Result<()> { - path::create_directory(self.as_os_file()?, path) - } - fn openat( - &self, - path: &str, - read: bool, - write: bool, - oflags: types::Oflags, - fd_flags: types::Fdflags, - ) -> Result> { - let handle = path::open(self.as_os_file()?, path, read, write, oflags, fd_flags)?; - Ok(Box::new(handle)) - } - fn link( - &self, - old_path: &str, - new_handle: Box, - new_path: &str, - follow: bool, - ) -> Result<()> { - let new_handle = match new_handle.as_any().downcast_ref::() { - None => { - error!("Tried to link OS resource with Virtual"); - return Err(Errno::Badf); - } - Some(handle) => handle, - }; - path::link( - self.as_os_file()?, - old_path, - new_handle.as_os_file()?, - new_path, - follow, - ) - } - fn symlink(&self, old_path: &str, new_path: &str) -> Result<()> { - path::symlink(old_path, self.as_os_file()?, new_path) - } - fn readlink(&self, path: &str, buf: &mut [u8]) -> Result { - path::readlink(self.as_os_file()?, path, buf) - } - fn readlinkat(&self, path: &str) -> Result { - path::readlinkat(self.as_os_file()?, path) - } - fn rename(&self, old_path: &str, new_handle: Box, new_path: &str) -> Result<()> { - let new_handle = match new_handle.as_any().downcast_ref::() { - None => { - error!("Tried to link OS resource with Virtual"); - return Err(Errno::Badf); - } - Some(handle) => handle, - }; - debug!("rename (old_dirfd, old_path)=({:?}, {:?})", self, old_path); - debug!( - "rename (new_dirfd, new_path)=({:?}, {:?})", - new_handle, new_path - ); - path::rename( - self.as_os_file()?, - old_path, - new_handle.as_os_file()?, - new_path, - ) - } - fn remove_directory(&self, path: &str) -> Result<()> { - debug!("remove_directory (dirfd, path)=({:?}, {:?})", self, path); - path::remove_directory(self.as_os_file()?, path) - } - fn unlink_file(&self, path: &str) -> Result<()> { - path::unlink_file(self.as_os_file()?, path) - } -} diff --git a/crates/wasi-common/src/sys/osother.rs b/crates/wasi-common/src/sys/osother.rs new file mode 100644 index 000000000000..47016f75d5bd --- /dev/null +++ b/crates/wasi-common/src/sys/osother.rs @@ -0,0 +1,85 @@ +use super::sys_impl::oshandle::OsHandle; +use super::{fd, AsFile}; +use crate::handle::{Handle, HandleRights}; +use crate::sandboxed_tty_writer::SandboxedTTYWriter; +use crate::wasi::types::{self, Filetype}; +use crate::wasi::Result; +use std::any::Any; +use std::cell::Cell; +use std::fs::File; +use std::io::{self, Read, Write}; +use std::ops::Deref; + +#[derive(Debug)] +pub(crate) struct OsOther { + file_type: Filetype, + rights: Cell, + handle: OsHandle, +} + +impl OsOther { + pub(super) fn new(file_type: Filetype, rights: HandleRights, handle: OsHandle) -> Self { + let rights = Cell::new(rights); + Self { + file_type, + rights, + handle, + } + } +} + +impl Deref for OsOther { + type Target = OsHandle; + + fn deref(&self) -> &Self::Target { + &self.handle + } +} + +impl Handle for OsOther { + fn as_any(&self) -> &dyn Any { + self + } + fn try_clone(&self) -> io::Result> { + let file_type = self.file_type; + let handle = self.handle.try_clone()?; + let rights = self.rights.clone(); + Ok(Box::new(Self { + file_type, + rights, + handle, + })) + } + fn get_file_type(&self) -> Filetype { + self.file_type + } + fn get_rights(&self) -> HandleRights { + self.rights.get() + } + fn set_rights(&self, new_rights: HandleRights) { + self.rights.set(new_rights) + } + // FdOps + fn fdstat_get(&self) -> Result { + fd::fdstat_get(&self.as_file()) + } + fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { + if let Some(handle) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { + self.handle.update_from(handle); + } + Ok(()) + } + fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result { + let nread = self.as_file().read_vectored(iovs)?; + Ok(nread) + } + fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result { + let mut fd: &File = &self.as_file(); + let nwritten = if isatty { + SandboxedTTYWriter::new(&mut fd).write_vectored(&iovs)? + } else { + fd.write_vectored(iovs)? + }; + Ok(nwritten) + } +} diff --git a/crates/wasi-common/src/sys/stdio.rs b/crates/wasi-common/src/sys/stdio.rs new file mode 100644 index 000000000000..45d81c7ee645 --- /dev/null +++ b/crates/wasi-common/src/sys/stdio.rs @@ -0,0 +1,91 @@ +use super::{fd, AsFile}; +use crate::handle::{Handle, HandleRights}; +use crate::sandboxed_tty_writer::SandboxedTTYWriter; +use crate::wasi::types::{self, Filetype}; +use crate::wasi::{Errno, Result}; +use std::any::Any; +use std::cell::Cell; +use std::io::{self, Read, Write}; + +pub(crate) trait StdioExt: Sized { + fn stdin() -> io::Result>; + fn stdout() -> io::Result>; + fn stderr() -> io::Result>; +} + +#[derive(Debug, Clone)] +#[allow(dead_code)] +pub(crate) enum Stdio { + In { rights: Cell }, + Out { rights: Cell }, + Err { rights: Cell }, +} + +impl Handle for Stdio { + fn as_any(&self) -> &dyn Any { + self + } + fn try_clone(&self) -> io::Result> { + Ok(Box::new(self.clone())) + } + fn get_file_type(&self) -> Filetype { + Filetype::CharacterDevice + } + fn get_rights(&self) -> HandleRights { + match self { + Self::In { rights } => rights.get(), + Self::Out { rights } => rights.get(), + Self::Err { rights } => rights.get(), + } + } + fn set_rights(&self, new_rights: HandleRights) { + match self { + Self::In { rights } => rights.set(new_rights), + Self::Out { rights } => rights.set(new_rights), + Self::Err { rights } => rights.set(new_rights), + } + } + // FdOps + fn fdstat_get(&self) -> Result { + fd::fdstat_get(&self.as_file()) + } + fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { + if let Some(_) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { + // OK, this means we should somehow update the underlying os handle, + // and we can't do that with `std::io::std{in, out, err}`, so we'll + // panic for now. + panic!("Tried updating Fdflags on Stdio handle by re-opening as file!"); + } + Ok(()) + } + fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result { + let nread = match self { + Self::In { .. } => io::stdin().read_vectored(iovs)?, + _ => return Err(Errno::Badf), + }; + Ok(nread) + } + fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result { + let nwritten = match self { + Self::In { .. } => return Err(Errno::Badf), + Self::Out { .. } => { + // lock for the duration of the scope + let stdout = io::stdout(); + let mut stdout = stdout.lock(); + let nwritten = if isatty { + SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)? + } else { + stdout.write_vectored(&iovs)? + }; + stdout.flush()?; + nwritten + } + // Always sanitize stderr, even if it's not directly connected to a tty, + // because stderr is meant for diagnostics rather than binary output, + // and may be redirected to a file which could end up being displayed + // on a tty later. + Self::Err { .. } => SandboxedTTYWriter::new(&mut io::stderr()).write_vectored(&iovs)?, + }; + Ok(nwritten) + } +} diff --git a/crates/wasi-common/src/sys/unix/bsd/mod.rs b/crates/wasi-common/src/sys/unix/bsd/mod.rs index a4a18824539f..a58b813e4aa5 100644 --- a/crates/wasi-common/src/sys/unix/bsd/mod.rs +++ b/crates/wasi-common/src/sys/unix/bsd/mod.rs @@ -1,4 +1,4 @@ -pub(crate) mod osfile; +pub(crate) mod oshandle; pub(crate) mod path; pub(crate) const O_RSYNC: yanix::file::OFlag = yanix::file::OFlag::SYNC; diff --git a/crates/wasi-common/src/sys/unix/bsd/osfile.rs b/crates/wasi-common/src/sys/unix/bsd/osfile.rs deleted file mode 100644 index 0ead556838ad..000000000000 --- a/crates/wasi-common/src/sys/unix/bsd/osfile.rs +++ /dev/null @@ -1,109 +0,0 @@ -use crate::sys::oshandle::AsFile; -use crate::wasi::Result; -use std::cell::{Cell, RefCell, RefMut}; -use std::fs::File; -use std::io; -use std::mem::ManuallyDrop; -use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; -use yanix::dir::Dir; - -#[derive(Debug)] -pub(crate) struct OsFile { - fd: Cell, - // In case that this `OsHandle` actually refers to a directory, - // when the client makes a `fd_readdir` syscall on this descriptor, - // we will need to cache the `libc::DIR` pointer manually in order - // to be able to seek on it later. While on Linux, this is handled - // by the OS, BSD Unixes require the client to do this caching. - // - // This comes directly from the BSD man pages on `readdir`: - // > Values returned by telldir() are good only for the lifetime - // > of the DIR pointer, dirp, from which they are derived. - // > If the directory is closed and then reopened, prior values - // > returned by telldir() will no longer be valid. - dir: RefCell>, -} - -impl OsFile { - /// Consumes `other` taking the ownership of the underlying - /// `RawFd` file descriptor. - /// - /// Note that the state of `Dir` stream pointer *will* not be carried - /// across from `other` to `self`. - pub(crate) fn update_from(&self, other: Self) { - let new_fd = other.into_raw_fd(); - let old_fd = self.fd.get(); - self.fd.set(new_fd); - // We need to remember to close the old_fd. - unsafe { - File::from_raw_fd(old_fd); - } - } - /// Clones `self` uninitializing the `Dir` stream pointer - /// (if any). - pub(crate) fn try_clone(&self) -> io::Result { - let fd = self.as_file().try_clone()?; - Ok(Self { - fd: Cell::new(fd.into_raw_fd()), - dir: RefCell::new(None), - }) - } - /// Returns the `Dir` stream pointer associated with - /// this instance. - /// - /// Initializes the `Dir` stream pointer if `None`. - pub(crate) fn dir_stream(&self) -> Result> { - if self.dir.borrow().is_none() { - // We need to duplicate the fd, because `opendir(3)`: - // Upon successful return from fdopendir(), the file descriptor is under - // control of the system, and if any attempt is made to close the file - // descriptor, or to modify the state of the associated description other - // than by means of closedir(), readdir(), readdir_r(), or rewinddir(), - // the behaviour is undefined. - let file = self.try_clone()?; - let d = Dir::from(file)?; - *self.dir.borrow_mut() = Some(d); - } - Ok(RefMut::map(self.dir.borrow_mut(), |dir| { - dir.as_mut().unwrap() - })) - } -} - -impl Drop for OsFile { - fn drop(&mut self) { - unsafe { - File::from_raw_fd(self.as_raw_fd()); - } - } -} - -impl AsRawFd for OsFile { - fn as_raw_fd(&self) -> RawFd { - self.fd.get() - } -} - -impl FromRawFd for OsFile { - unsafe fn from_raw_fd(fd: RawFd) -> Self { - Self { - fd: Cell::new(fd), - dir: RefCell::new(None), - } - } -} - -impl IntoRawFd for OsFile { - fn into_raw_fd(self) -> RawFd { - // We need to prevent dropping of the OsFile - let wrapped = ManuallyDrop::new(self); - wrapped.fd.get() - } -} - -impl AsFile for OsFile { - fn as_file(&self) -> ManuallyDrop { - let file = unsafe { File::from_raw_fd(self.fd.get()) }; - ManuallyDrop::new(file) - } -} diff --git a/crates/wasi-common/src/sys/unix/bsd/oshandle.rs b/crates/wasi-common/src/sys/unix/bsd/oshandle.rs new file mode 100644 index 000000000000..5a926d224382 --- /dev/null +++ b/crates/wasi-common/src/sys/unix/bsd/oshandle.rs @@ -0,0 +1,42 @@ +use crate::sys::sys_impl::oshandle::OsHandle; +use crate::wasi::Result; +use std::cell::{RefCell, RefMut}; +use std::io; +use std::ops::Deref; +use yanix::dir::Dir; + +#[derive(Debug)] +pub(crate) struct OsDirHandle { + handle: OsHandle, + dir: RefCell, +} + +impl OsDirHandle { + /// Consumes the spcified `OsHandle` and initialises the + /// underlying `Dir` stream pointer. + pub(crate) fn new(handle: OsHandle) -> io::Result { + let dir = Dir::from(handle.try_clone()?)?; + let dir = RefCell::new(dir); + Ok(Self { handle, dir }) + } + /// Tries clone `self`. + /// + /// Note that the `Dir` stream pointer will be reset + /// to start. + pub(crate) fn try_clone(&self) -> io::Result { + let handle = self.handle.try_clone()?; + Self::new(handle) + } + /// Gets mutable reference to the current dir stream pointer. + pub(crate) fn dir_stream(&self) -> Result> { + Ok(self.dir.borrow_mut()) + } +} + +impl Deref for OsDirHandle { + type Target = OsHandle; + + fn deref(&self) -> &Self::Target { + &self.handle + } +} diff --git a/crates/wasi-common/src/sys/unix/bsd/path.rs b/crates/wasi-common/src/sys/unix/bsd/path.rs index 872d3f25cd9a..8dff951cb998 100644 --- a/crates/wasi-common/src/sys/unix/bsd/path.rs +++ b/crates/wasi-common/src/sys/unix/bsd/path.rs @@ -1,8 +1,8 @@ -use super::osfile::OsFile; +use crate::sys::osdir::OsDir; use crate::wasi::{Errno, Result}; use std::os::unix::prelude::AsRawFd; -pub(crate) fn unlink_file(dirfd: &OsFile, path: &str) -> Result<()> { +pub(crate) fn unlink_file(dirfd: &OsDir, path: &str) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; match unsafe { unlinkat(dirfd.as_raw_fd(), path, AtFlag::empty()) } { Err(err) => { @@ -35,7 +35,7 @@ pub(crate) fn unlink_file(dirfd: &OsFile, path: &str) -> Result<()> { } } -pub(crate) fn symlink(old_path: &str, new_dirfd: &OsFile, new_path: &str) -> Result<()> { +pub(crate) fn symlink(old_path: &str, new_dirfd: &OsDir, new_path: &str) -> Result<()> { use yanix::file::{fstatat, symlinkat, AtFlag}; log::debug!("path_symlink old_path = {:?}", old_path); @@ -69,9 +69,9 @@ pub(crate) fn symlink(old_path: &str, new_dirfd: &OsFile, new_path: &str) -> Res } pub(crate) fn rename( - old_dirfd: &OsFile, + old_dirfd: &OsDir, old_path: &str, - new_dirfd: &OsFile, + new_dirfd: &OsDir, new_path: &str, ) -> Result<()> { use yanix::file::{fstatat, renameat, AtFlag}; diff --git a/crates/wasi-common/src/sys/unix/emscripten/mod.rs b/crates/wasi-common/src/sys/unix/emscripten/mod.rs index 151d7350259b..8f233de8fe4e 100644 --- a/crates/wasi-common/src/sys/unix/emscripten/mod.rs +++ b/crates/wasi-common/src/sys/unix/emscripten/mod.rs @@ -1,5 +1,5 @@ -#[path = "../linux/osfile.rs"] -pub(crate) mod osfile; +#[path = "../linux/oshandle.rs"] +pub(crate) mod oshandle; #[path = "../linux/path.rs"] pub(crate) mod path; diff --git a/crates/wasi-common/src/sys/unix/fd.rs b/crates/wasi-common/src/sys/unix/fd.rs index e192bd44d85d..b218e0d1e766 100644 --- a/crates/wasi-common/src/sys/unix/fd.rs +++ b/crates/wasi-common/src/sys/unix/fd.rs @@ -1,4 +1,6 @@ -use super::oshandle::OsFile; +use super::oshandle::OsHandle; +use crate::sys::osdir::OsDir; +use crate::sys::osfile::OsFile; use crate::wasi::{self, types, Result}; use std::convert::TryInto; use std::fs::File; @@ -9,7 +11,7 @@ pub(crate) fn fdstat_get(fd: &File) -> Result { Ok(fdflags.into()) } -pub(crate) fn fdstat_set_flags(fd: &File, fdflags: types::Fdflags) -> Result> { +pub(crate) fn fdstat_set_flags(fd: &File, fdflags: types::Fdflags) -> Result> { unsafe { yanix::fcntl::set_status_flags(fd.as_raw_fd(), fdflags.into())? }; // We return None here to signal that the operation succeeded on the original // file descriptor and mutating the original WASI Descriptor is thus unnecessary. @@ -45,14 +47,14 @@ pub(crate) fn filestat_get(file: &File) -> Result { } pub(crate) fn readdir<'a>( - file: &'a OsFile, + dirfd: &'a OsDir, cookie: types::Dircookie, ) -> Result> + 'a>> { use yanix::dir::{DirIter, Entry, EntryExt, SeekLoc}; // Get an instance of `Dir`; this is host-specific due to intricasies // of managing a dir stream between Linux and BSD *nixes - let mut dir = file.dir_stream()?; + let mut dir = dirfd.dir_stream()?; // Seek if needed. Unless cookie is wasi::__WASI_DIRCOOKIE_START, // new items may not be returned to the caller. diff --git a/crates/wasi-common/src/sys/unix/linux/mod.rs b/crates/wasi-common/src/sys/unix/linux/mod.rs index b587ed6b6fbd..9d7cc60191cc 100644 --- a/crates/wasi-common/src/sys/unix/linux/mod.rs +++ b/crates/wasi-common/src/sys/unix/linux/mod.rs @@ -1,4 +1,4 @@ -pub(crate) mod osfile; +pub(crate) mod oshandle; pub(crate) mod path; pub(crate) const O_RSYNC: yanix::file::OFlag = yanix::file::OFlag::RSYNC; diff --git a/crates/wasi-common/src/sys/unix/linux/osfile.rs b/crates/wasi-common/src/sys/unix/linux/osfile.rs deleted file mode 100644 index 388f245dfa07..000000000000 --- a/crates/wasi-common/src/sys/unix/linux/osfile.rs +++ /dev/null @@ -1,82 +0,0 @@ -use crate::sys::oshandle::AsFile; -use crate::wasi::Result; -use std::cell::Cell; -use std::fs::File; -use std::io; -use std::mem::ManuallyDrop; -use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; -use yanix::dir::Dir; - -#[derive(Debug)] -pub(crate) struct OsFile(Cell); - -impl OsFile { - /// Consumes `other` taking the ownership of the underlying - /// `RawFd` file descriptor. - pub(crate) fn update_from(&self, other: Self) { - let new_fd = other.into_raw_fd(); - let old_fd = self.0.get(); - self.0.set(new_fd); - // We need to remember to close the old_fd. - unsafe { - File::from_raw_fd(old_fd); - } - } - /// Clones `self`. - pub(crate) fn try_clone(&self) -> io::Result { - let fd = self.as_file().try_clone()?; - Ok(Self(Cell::new(fd.into_raw_fd()))) - } - /// Returns the `Dir` stream pointer associated with - /// this instance. - pub(crate) fn dir_stream(&self) -> Result> { - // We need to duplicate the fd, because `opendir(3)`: - // After a successful call to fdopendir(), fd is used internally by the implementation, - // and should not otherwise be used by the application. - // `opendir(3p)` also says that it's undefined behavior to - // modify the state of the fd in a different way than by accessing DIR*. - // - // Still, rewinddir will be needed because the two file descriptors - // share progress. But we can safely execute closedir now. - let file = self.try_clone()?; - // TODO This doesn't look very clean. Can we do something about it? - // Boxing is needed here in order to satisfy `yanix`'s trait requirement for the `DirIter` - // where `T: Deref`. - Ok(Box::new(Dir::from(file)?)) - } -} - -impl Drop for OsFile { - fn drop(&mut self) { - unsafe { - File::from_raw_fd(self.as_raw_fd()); - } - } -} - -impl AsRawFd for OsFile { - fn as_raw_fd(&self) -> RawFd { - self.0.get() - } -} - -impl FromRawFd for OsFile { - unsafe fn from_raw_fd(fd: RawFd) -> Self { - Self(Cell::new(fd)) - } -} - -impl IntoRawFd for OsFile { - fn into_raw_fd(self) -> RawFd { - // We need to prevent dropping of the OsFile - let wrapped = ManuallyDrop::new(self); - wrapped.0.get() - } -} - -impl AsFile for OsFile { - fn as_file(&self) -> ManuallyDrop { - let file = unsafe { File::from_raw_fd(self.0.get()) }; - ManuallyDrop::new(file) - } -} diff --git a/crates/wasi-common/src/sys/unix/linux/oshandle.rs b/crates/wasi-common/src/sys/unix/linux/oshandle.rs new file mode 100644 index 000000000000..e5306c934634 --- /dev/null +++ b/crates/wasi-common/src/sys/unix/linux/oshandle.rs @@ -0,0 +1,44 @@ +use crate::sys::sys_impl::oshandle::OsHandle; +use crate::wasi::Result; +use std::io; +use std::ops::Deref; +use yanix::dir::Dir; + +#[derive(Debug)] +pub(crate) struct OsDirHandle(OsHandle); + +impl OsDirHandle { + /// Consumes the spcified `OsHandle`. + pub(crate) fn new(handle: OsHandle) -> io::Result { + Ok(Self(handle)) + } + /// Tries clone `self`. + pub(crate) fn try_clone(&self) -> io::Result { + let handle = self.0.try_clone()?; + Self::new(handle) + } + /// Gets current dir stream pointer. + pub(crate) fn dir_stream(&self) -> Result> { + // We need to duplicate the fd, because `opendir(3)`: + // After a successful call to fdopendir(), fd is used internally by the implementation, + // and should not otherwise be used by the application. + // `opendir(3p)` also says that it's undefined behavior to + // modify the state of the fd in a different way than by accessing DIR*. + // + // Still, rewinddir will be needed because the two file descriptors + // share progress. But we can safely execute closedir now. + let file = self.0.try_clone()?; + // TODO This doesn't look very clean. Can we do something about it? + // Boxing is needed here in order to satisfy `yanix`'s trait requirement for the `DirIter` + // where `T: Deref`. + Ok(Box::new(Dir::from(file)?)) + } +} + +impl Deref for OsDirHandle { + type Target = OsHandle; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} diff --git a/crates/wasi-common/src/sys/unix/linux/path.rs b/crates/wasi-common/src/sys/unix/linux/path.rs index 02ee2b3fddc0..611c57d22e51 100644 --- a/crates/wasi-common/src/sys/unix/linux/path.rs +++ b/crates/wasi-common/src/sys/unix/linux/path.rs @@ -1,14 +1,14 @@ -use super::osfile::OsFile; +use crate::sys::osdir::OsDir; use crate::wasi::Result; use std::os::unix::prelude::AsRawFd; -pub(crate) fn unlink_file(dirfd: &OsFile, path: &str) -> Result<()> { +pub(crate) fn unlink_file(dirfd: &OsDir, path: &str) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; unsafe { unlinkat(dirfd.as_raw_fd(), path, AtFlag::empty())? }; Ok(()) } -pub(crate) fn symlink(old_path: &str, new_dirfd: &OsFile, new_path: &str) -> Result<()> { +pub(crate) fn symlink(old_path: &str, new_dirfd: &OsDir, new_path: &str) -> Result<()> { use yanix::file::symlinkat; log::debug!("path_symlink old_path = {:?}", old_path); @@ -23,9 +23,9 @@ pub(crate) fn symlink(old_path: &str, new_dirfd: &OsFile, new_path: &str) -> Res } pub(crate) fn rename( - old_dirfd: &OsFile, + old_dirfd: &OsDir, old_path: &str, - new_dirfd: &OsFile, + new_dirfd: &OsDir, new_path: &str, ) -> Result<()> { use yanix::file::renameat; diff --git a/crates/wasi-common/src/sys/unix/mod.rs b/crates/wasi-common/src/sys/unix/mod.rs index 8bad50fc795e..8768cd1e8368 100644 --- a/crates/wasi-common/src/sys/unix/mod.rs +++ b/crates/wasi-common/src/sys/unix/mod.rs @@ -1,8 +1,12 @@ pub(crate) mod clock; pub(crate) mod fd; +pub(crate) mod osdir; +pub(crate) mod osfile; pub(crate) mod oshandle; +pub(crate) mod osother; pub(crate) mod path; pub(crate) mod poll; +pub(crate) mod stdio; cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { @@ -22,16 +26,58 @@ cfg_if::cfg_if! { } } +use crate::sys::AsFile; use crate::wasi::{types, Errno, Result}; use std::convert::{TryFrom, TryInto}; use std::fs::File; use std::io; +use std::mem::ManuallyDrop; +use std::os::unix::prelude::{AsRawFd, FileTypeExt, FromRawFd}; use std::path::Path; use yanix::clock::ClockId; use yanix::file::{AtFlag, OFlag}; pub(crate) use sys_impl::*; +impl AsFile for T { + fn as_file(&self) -> ManuallyDrop { + let file = unsafe { File::from_raw_fd(self.as_raw_fd()) }; + ManuallyDrop::new(file) + } +} + +pub(super) fn get_file_type(file: &File) -> io::Result { + let ft = file.metadata()?.file_type(); + let file_type = if ft.is_block_device() { + log::debug!("Host fd {:?} is a block device", file.as_raw_fd()); + types::Filetype::BlockDevice + } else if ft.is_char_device() { + log::debug!("Host fd {:?} is a char device", file.as_raw_fd()); + types::Filetype::CharacterDevice + } else if ft.is_dir() { + log::debug!("Host fd {:?} is a directory", file.as_raw_fd()); + types::Filetype::Directory + } else if ft.is_file() { + log::debug!("Host fd {:?} is a file", file.as_raw_fd()); + types::Filetype::RegularFile + } else if ft.is_socket() { + log::debug!("Host fd {:?} is a socket", file.as_raw_fd()); + use yanix::socket::{get_socket_type, SockType}; + match unsafe { get_socket_type(file.as_raw_fd())? } { + SockType::Datagram => types::Filetype::SocketDgram, + SockType::Stream => types::Filetype::SocketStream, + _ => return Err(io::Error::from_raw_os_error(libc::EINVAL)), + } + } else if ft.is_fifo() { + log::debug!("Host fd {:?} is a fifo", file.as_raw_fd()); + types::Filetype::Unknown + } else { + log::debug!("Host fd {:?} is unknown", file.as_raw_fd()); + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + }; + Ok(file_type) +} + pub fn preopen_dir>(path: P) -> io::Result { File::open(path) } diff --git a/crates/wasi-common/src/sys/unix/osdir.rs b/crates/wasi-common/src/sys/unix/osdir.rs new file mode 100644 index 000000000000..aab45df306e1 --- /dev/null +++ b/crates/wasi-common/src/sys/unix/osdir.rs @@ -0,0 +1,42 @@ +use super::oshandle::{OsDirHandle, OsHandle}; +use crate::handle::HandleRights; +use crate::sys::osdir::OsDir; +use crate::wasi::{types, RightsExt}; +use std::convert::TryFrom; +use std::fs::File; +use std::io; +use std::ops::Deref; +use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; + +impl AsRawFd for OsDir { + fn as_raw_fd(&self) -> RawFd { + self.deref().as_raw_fd() + } +} + +impl TryFrom for OsDir { + type Error = io::Error; + + fn try_from(file: File) -> io::Result { + let rights = get_rights(&file)?; + let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; + let handle = OsDirHandle::new(handle)?; + Ok(Self::new(rights, handle)) + } +} + +fn get_rights(file: &File) -> io::Result { + use yanix::{fcntl, file::OFlag}; + let mut rights = HandleRights::new( + types::Rights::directory_base(), + types::Rights::directory_inheriting(), + ); + let flags = unsafe { fcntl::get_status_flags(file.as_raw_fd())? }; + let accmode = flags & OFlag::ACCMODE; + if accmode == OFlag::RDONLY { + rights.base &= !types::Rights::FD_WRITE; + } else if accmode == OFlag::WRONLY { + rights.base &= !types::Rights::FD_READ; + } + Ok(rights) +} diff --git a/crates/wasi-common/src/sys/unix/osfile.rs b/crates/wasi-common/src/sys/unix/osfile.rs new file mode 100644 index 000000000000..de888a338a82 --- /dev/null +++ b/crates/wasi-common/src/sys/unix/osfile.rs @@ -0,0 +1,52 @@ +use super::oshandle::OsHandle; +use crate::handle::{Handle, HandleRights}; +use crate::sys::osfile::{OsFile, OsFileExt}; +use crate::wasi::{types, RightsExt}; +use std::convert::TryFrom; +use std::fs::{File, OpenOptions}; +use std::io; +use std::ops::Deref; +use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; + +impl AsRawFd for OsFile { + fn as_raw_fd(&self) -> RawFd { + self.deref().as_raw_fd() + } +} + +impl TryFrom for OsFile { + type Error = io::Error; + + fn try_from(file: File) -> io::Result { + let rights = get_rights(&file)?; + let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; + Ok(Self::new(rights, handle)) + } +} + +fn get_rights(file: &File) -> io::Result { + use yanix::{fcntl, file::OFlag}; + let mut rights = HandleRights::new( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ); + let flags = unsafe { fcntl::get_status_flags(file.as_raw_fd())? }; + let accmode = flags & OFlag::ACCMODE; + if accmode == OFlag::RDONLY { + rights.base &= !types::Rights::FD_WRITE; + } else if accmode == OFlag::WRONLY { + rights.base &= !types::Rights::FD_READ; + } + Ok(rights) +} + +impl OsFileExt for OsFile { + fn from_null() -> io::Result> { + let file = OpenOptions::new() + .read(true) + .write(true) + .open("/dev/null")?; + let file = Self::try_from(file)?; + Ok(Box::new(file)) + } +} diff --git a/crates/wasi-common/src/sys/unix/oshandle.rs b/crates/wasi-common/src/sys/unix/oshandle.rs index 236ba07884e0..ea55431f4fe9 100644 --- a/crates/wasi-common/src/sys/unix/oshandle.rs +++ b/crates/wasi-common/src/sys/unix/oshandle.rs @@ -1,123 +1,61 @@ -use crate::entry::EntryRights; -use crate::sys::oshandle::{AsFile, OsHandle, OsHandleExt}; -use crate::wasi::{types, RightsExt}; -use std::fs::{File, OpenOptions}; +use crate::sys::AsFile; +use std::cell::Cell; +use std::fs::File; use std::io; use std::mem::ManuallyDrop; -use std::os::unix::prelude::{AsRawFd, FileTypeExt, FromRawFd, IntoRawFd, RawFd}; +use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; -pub(crate) use super::sys_impl::osfile::*; +pub(crate) use super::sys_impl::oshandle::*; -impl AsRawFd for OsHandle { - fn as_raw_fd(&self) -> RawFd { - match self { - Self::OsFile(file) => file.as_raw_fd(), - Self::Stdin => io::stdin().as_raw_fd(), - Self::Stdout => io::stdout().as_raw_fd(), - Self::Stderr => io::stderr().as_raw_fd(), +#[derive(Debug)] +pub(crate) struct OsHandle(Cell); + +impl OsHandle { + /// Tries clone `self`. + pub(crate) fn try_clone(&self) -> io::Result { + let fd = self.as_file().try_clone()?; + Ok(Self(Cell::new(fd.into_raw_fd()))) + } + /// Consumes `other` taking the ownership of the underlying + /// `RawFd` file descriptor. + /// + /// Note that the state of `Dir` stream pointer *will* not be carried + /// across from `other` to `self`. + pub(crate) fn update_from(&self, other: Self) { + let new_fd = other.into_raw_fd(); + let old_fd = self.0.get(); + self.0.set(new_fd); + // We need to remember to close the old_fd. + unsafe { + File::from_raw_fd(old_fd); } } } -impl AsFile for OsHandle { - fn as_file(&self) -> ManuallyDrop { - let file = unsafe { File::from_raw_fd(self.as_raw_fd()) }; - ManuallyDrop::new(file) +impl Drop for OsHandle { + fn drop(&mut self) { + unsafe { + File::from_raw_fd(self.as_raw_fd()); + } } } -impl From for OsHandle { - fn from(file: File) -> Self { - Self::from(unsafe { OsFile::from_raw_fd(file.into_raw_fd()) }) +impl AsRawFd for OsHandle { + fn as_raw_fd(&self) -> RawFd { + self.0.get() } } -impl OsHandleExt for OsHandle { - fn get_file_type(&self) -> io::Result { - let file = self.as_file(); - let ft = file.metadata()?.file_type(); - let file_type = if ft.is_block_device() { - log::debug!("Host fd {:?} is a block device", self.as_raw_fd()); - types::Filetype::BlockDevice - } else if ft.is_char_device() { - log::debug!("Host fd {:?} is a char device", self.as_raw_fd()); - types::Filetype::CharacterDevice - } else if ft.is_dir() { - log::debug!("Host fd {:?} is a directory", self.as_raw_fd()); - types::Filetype::Directory - } else if ft.is_file() { - log::debug!("Host fd {:?} is a file", self.as_raw_fd()); - types::Filetype::RegularFile - } else if ft.is_socket() { - log::debug!("Host fd {:?} is a socket", self.as_raw_fd()); - use yanix::socket::{get_socket_type, SockType}; - match unsafe { get_socket_type(self.as_raw_fd())? } { - SockType::Datagram => types::Filetype::SocketDgram, - SockType::Stream => types::Filetype::SocketStream, - _ => return Err(io::Error::from_raw_os_error(libc::EINVAL)), - } - } else if ft.is_fifo() { - log::debug!("Host fd {:?} is a fifo", self.as_raw_fd()); - types::Filetype::Unknown - } else { - log::debug!("Host fd {:?} is unknown", self.as_raw_fd()); - return Err(io::Error::from_raw_os_error(libc::EINVAL)); - }; - - Ok(file_type) - } - - fn get_rights(&self, file_type: types::Filetype) -> io::Result { - use yanix::{fcntl, file::OFlag}; - let (base, inheriting) = match file_type { - types::Filetype::BlockDevice => ( - types::Rights::block_device_base(), - types::Rights::block_device_inheriting(), - ), - types::Filetype::CharacterDevice => { - use yanix::file::isatty; - if unsafe { isatty(self.as_raw_fd())? } { - (types::Rights::tty_base(), types::Rights::tty_base()) - } else { - ( - types::Rights::character_device_base(), - types::Rights::character_device_inheriting(), - ) - } - } - types::Filetype::Directory => ( - types::Rights::directory_base(), - types::Rights::directory_inheriting(), - ), - types::Filetype::RegularFile => ( - types::Rights::regular_file_base(), - types::Rights::regular_file_inheriting(), - ), - types::Filetype::SocketDgram | types::Filetype::SocketStream => ( - types::Rights::socket_base(), - types::Rights::socket_inheriting(), - ), - types::Filetype::SymbolicLink | types::Filetype::Unknown => ( - types::Rights::regular_file_base(), - types::Rights::regular_file_inheriting(), - ), - }; - let mut rights = EntryRights::new(base, inheriting); - let flags = unsafe { fcntl::get_status_flags(self.as_raw_fd())? }; - let accmode = flags & OFlag::ACCMODE; - if accmode == OFlag::RDONLY { - rights.base &= !types::Rights::FD_WRITE; - } else if accmode == OFlag::WRONLY { - rights.base &= !types::Rights::FD_READ; - } - Ok(rights) +impl FromRawFd for OsHandle { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + Self(Cell::new(fd)) } +} - fn from_null() -> io::Result { - let file = OpenOptions::new() - .read(true) - .write(true) - .open("/dev/null")?; - Ok(Self::from(file)) +impl IntoRawFd for OsHandle { + fn into_raw_fd(self) -> RawFd { + // We need to prevent dropping of self + let wrapped = ManuallyDrop::new(self); + wrapped.0.get() } } diff --git a/crates/wasi-common/src/sys/unix/osother.rs b/crates/wasi-common/src/sys/unix/osother.rs new file mode 100644 index 000000000000..ce9d189bd317 --- /dev/null +++ b/crates/wasi-common/src/sys/unix/osother.rs @@ -0,0 +1,66 @@ +use super::get_file_type; +use super::oshandle::OsHandle; +use crate::handle::HandleRights; +use crate::sys::osother::OsOther; +use crate::wasi::{types, RightsExt}; +use std::convert::TryFrom; +use std::fs::File; +use std::io; +use std::ops::Deref; +use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; + +impl AsRawFd for OsOther { + fn as_raw_fd(&self) -> RawFd { + self.deref().as_raw_fd() + } +} + +impl TryFrom for OsOther { + type Error = io::Error; + + fn try_from(file: File) -> io::Result { + let file_type = get_file_type(&file)?; + let rights = get_rights(&file, &file_type)?; + let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; + Ok(Self::new(file_type, rights, handle)) + } +} + +fn get_rights(file: &File, file_type: &types::Filetype) -> io::Result { + use yanix::{fcntl, file::OFlag}; + let (base, inheriting) = match file_type { + types::Filetype::BlockDevice => ( + types::Rights::block_device_base(), + types::Rights::block_device_inheriting(), + ), + types::Filetype::CharacterDevice => { + use yanix::file::isatty; + if unsafe { isatty(file.as_raw_fd())? } { + (types::Rights::tty_base(), types::Rights::tty_base()) + } else { + ( + types::Rights::character_device_base(), + types::Rights::character_device_inheriting(), + ) + } + } + types::Filetype::SocketDgram | types::Filetype::SocketStream => ( + types::Rights::socket_base(), + types::Rights::socket_inheriting(), + ), + types::Filetype::SymbolicLink | types::Filetype::Unknown => ( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ), + _ => unreachable!("these should have been handled already!"), + }; + let mut rights = HandleRights::new(base, inheriting); + let flags = unsafe { fcntl::get_status_flags(file.as_raw_fd())? }; + let accmode = flags & OFlag::ACCMODE; + if accmode == OFlag::RDONLY { + rights.base &= !types::Rights::FD_WRITE; + } else if accmode == OFlag::WRONLY { + rights.base &= !types::Rights::FD_READ; + } + Ok(rights) +} diff --git a/crates/wasi-common/src/sys/unix/path.rs b/crates/wasi-common/src/sys/unix/path.rs index e52aa0f67f6a..9ffa5f7b60bb 100644 --- a/crates/wasi-common/src/sys/unix/path.rs +++ b/crates/wasi-common/src/sys/unix/path.rs @@ -1,8 +1,9 @@ -use super::oshandle::OsFile; -use crate::entry::EntryRights; -use crate::sys::oshandle::OsHandle; +use crate::handle::{Handle, HandleRights}; +use crate::sys::osdir::OsDir; use crate::wasi::{types, Errno, Result}; +use std::convert::TryFrom; use std::ffi::OsStr; +use std::fs::File; use std::os::unix::prelude::{AsRawFd, FromRawFd, OsStrExt}; use std::str; use yanix::file::OFlag; @@ -19,10 +20,10 @@ pub(crate) fn from_host>(s: S) -> Result { } pub(crate) fn open_rights( - input_rights: &EntryRights, + input_rights: &HandleRights, oflags: types::Oflags, fs_flags: types::Fdflags, -) -> EntryRights { +) -> HandleRights { // which rights are needed on the dirfd? let mut needed_base = types::Rights::PATH_OPEN; let mut needed_inheriting = input_rights.base | input_rights.inheriting; @@ -45,10 +46,10 @@ pub(crate) fn open_rights( needed_inheriting |= types::Rights::FD_SYNC; } - EntryRights::new(needed_base, needed_inheriting) + HandleRights::new(needed_base, needed_inheriting) } -pub(crate) fn readlinkat(dirfd: &OsFile, path: &str) -> Result { +pub(crate) fn readlinkat(dirfd: &OsDir, path: &str) -> Result { use std::os::unix::prelude::AsRawFd; use yanix::file::readlinkat; @@ -59,16 +60,16 @@ pub(crate) fn readlinkat(dirfd: &OsFile, path: &str) -> Result { Ok(path) } -pub(crate) fn create_directory(base: &OsFile, path: &str) -> Result<()> { +pub(crate) fn create_directory(base: &OsDir, path: &str) -> Result<()> { use yanix::file::{mkdirat, Mode}; unsafe { mkdirat(base.as_raw_fd(), path, Mode::from_bits_truncate(0o777))? }; Ok(()) } pub(crate) fn link( - old_dirfd: &OsFile, + old_dirfd: &OsDir, old_path: &str, - new_dirfd: &OsFile, + new_dirfd: &OsDir, new_path: &str, follow_symlinks: bool, ) -> Result<()> { @@ -91,13 +92,13 @@ pub(crate) fn link( } pub(crate) fn open( - dirfd: &OsFile, + dirfd: &OsDir, path: &str, read: bool, write: bool, oflags: types::Oflags, fs_flags: types::Fdflags, -) -> Result { +) -> Result> { use yanix::file::{fstatat, openat, AtFlag, FileType, Mode, OFlag}; let mut nix_all_oflags = if read && write { @@ -181,10 +182,12 @@ pub(crate) fn open( log::debug!("path_open (host) new_fd = {:?}", new_fd); // Determine the type of the new file descriptor and which rights contradict with this type - Ok(OsHandle::from(unsafe { OsFile::from_raw_fd(new_fd) })) + let file = unsafe { File::from_raw_fd(new_fd) }; + let handle = >::try_from(file)?; + Ok(handle) } -pub(crate) fn readlink(dirfd: &OsFile, path: &str, buf: &mut [u8]) -> Result { +pub(crate) fn readlink(dirfd: &OsDir, path: &str, buf: &mut [u8]) -> Result { use std::cmp::min; use yanix::file::readlinkat; let read_link = unsafe { readlinkat(dirfd.as_raw_fd(), path)? }; @@ -196,7 +199,7 @@ pub(crate) fn readlink(dirfd: &OsFile, path: &str, buf: &mut [u8]) -> Result Result<()> { +pub(crate) fn remove_directory(dirfd: &OsDir, path: &str) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; unsafe { unlinkat(dirfd.as_raw_fd(), path, AtFlag::REMOVEDIR)? }; Ok(()) diff --git a/crates/wasi-common/src/sys/unix/poll.rs b/crates/wasi-common/src/sys/unix/poll.rs index 7b5ffddded71..da932007a9fc 100644 --- a/crates/wasi-common/src/sys/unix/poll.rs +++ b/crates/wasi-common/src/sys/unix/poll.rs @@ -1,6 +1,10 @@ -use super::super::oshandle::OsHandle; +use crate::entry::EntryHandle; use crate::poll::{ClockEventData, FdEventData}; -use crate::sys::oshandle::AsFile; +use crate::sys::osdir::OsDir; +use crate::sys::osfile::OsFile; +use crate::sys::osother::OsOther; +use crate::sys::stdio::Stdio; +use crate::sys::AsFile; use crate::wasi::{types, Errno, Result}; use std::io; use std::{convert::TryInto, os::unix::prelude::AsRawFd}; @@ -28,12 +32,18 @@ pub(crate) fn oneoff( // events we filtered before. If we get something else here, the code has a serious bug. _ => unreachable!(), }; - let handle = event - .handle - .as_any() - .downcast_ref::() - .expect("can poll FdEvent for OS resources only"); - unsafe { PollFd::new(handle.as_raw_fd(), flags) } + let fd = if let Some(file) = event.handle.as_any().downcast_ref::() { + file.as_raw_fd() + } else if let Some(dir) = event.handle.as_any().downcast_ref::() { + dir.as_raw_fd() + } else if let Some(stdio) = event.handle.as_any().downcast_ref::() { + stdio.as_raw_fd() + } else if let Some(other) = event.handle.as_any().downcast_ref::() { + other.as_raw_fd() + } else { + panic!("can poll FdEvent for OS resources only") + }; + unsafe { PollFd::new(fd, flags) } }) .collect(); @@ -80,18 +90,23 @@ fn handle_fd_event( ready_events: impl Iterator, events: &mut Vec, ) -> Result<()> { - fn query_nbytes(handle: &OsHandle) -> Result { - // fionread may overflow for large files, so use another way for regular files. - if let OsHandle::OsFile(file) = handle { + fn query_nbytes(handle: EntryHandle) -> Result { + if let Some(file) = handle.as_any().downcast_ref::() { + // fionread may overflow for large files, so use another way for regular files. + use yanix::file::tell; let meta = file.as_file().metadata()?; - if meta.file_type().is_file() { - use yanix::file::tell; - let len = meta.len(); - let host_offset = unsafe { tell(file.as_raw_fd())? }; - return Ok(len - host_offset); - } + let len = meta.len(); + let host_offset = unsafe { tell(file.as_raw_fd())? }; + Ok(len - host_offset) + } else if let Some(dir) = handle.as_any().downcast_ref::() { + unsafe { Ok(fionread(dir.as_raw_fd())?.into()) } + } else if let Some(stdio) = handle.as_any().downcast_ref::() { + unsafe { Ok(fionread(stdio.as_raw_fd())?.into()) } + } else if let Some(other) = handle.as_any().downcast_ref::() { + unsafe { Ok(fionread(other.as_raw_fd())?.into()) } + } else { + panic!("can poll FdEvent for OS resources only") } - unsafe { Ok(fionread(handle.as_raw_fd())?.into()) } } for (fd_event, poll_fd) in ready_events { @@ -106,12 +121,7 @@ fn handle_fd_event( log::debug!("poll_oneoff_handle_fd_event revents = {:?}", revents); let nbytes = if fd_event.r#type == types::Eventtype::FdRead { - let handle = fd_event - .handle - .as_any() - .downcast_ref::() - .expect("can poll FdEvent for OS resources only"); - query_nbytes(handle)? + query_nbytes(fd_event.handle)? } else { 0 }; diff --git a/crates/wasi-common/src/sys/unix/stdio.rs b/crates/wasi-common/src/sys/unix/stdio.rs new file mode 100644 index 000000000000..5d55f5848f70 --- /dev/null +++ b/crates/wasi-common/src/sys/unix/stdio.rs @@ -0,0 +1,66 @@ +use crate::handle::{Handle, HandleRights}; +use crate::sys::stdio::{Stdio, StdioExt}; +use crate::wasi::{types, RightsExt}; +use std::cell::Cell; +use std::fs::File; +use std::io; +use std::mem::ManuallyDrop; +use std::os::unix::prelude::{AsRawFd, FromRawFd, RawFd}; + +impl AsRawFd for Stdio { + fn as_raw_fd(&self) -> RawFd { + match self { + Self::In { .. } => io::stdin().as_raw_fd(), + Self::Out { .. } => io::stdout().as_raw_fd(), + Self::Err { .. } => io::stderr().as_raw_fd(), + } + } +} + +impl StdioExt for Stdio { + fn stdin() -> io::Result> { + let file = unsafe { File::from_raw_fd(io::stdin().as_raw_fd()) }; + let file = ManuallyDrop::new(file); + let rights = get_rights(&file)?; + let rights = Cell::new(rights); + Ok(Box::new(Self::In { rights })) + } + fn stdout() -> io::Result> { + let file = unsafe { File::from_raw_fd(io::stdin().as_raw_fd()) }; + let file = ManuallyDrop::new(file); + let rights = get_rights(&file)?; + let rights = Cell::new(rights); + Ok(Box::new(Self::Out { rights })) + } + fn stderr() -> io::Result> { + let file = unsafe { File::from_raw_fd(io::stdin().as_raw_fd()) }; + let file = ManuallyDrop::new(file); + let rights = get_rights(&file)?; + let rights = Cell::new(rights); + Ok(Box::new(Self::Err { rights })) + } +} + +fn get_rights(file: &File) -> io::Result { + use yanix::fcntl; + use yanix::file::{isatty, OFlag}; + let (base, inheriting) = { + if unsafe { isatty(file.as_raw_fd())? } { + (types::Rights::tty_base(), types::Rights::tty_base()) + } else { + ( + types::Rights::character_device_base(), + types::Rights::character_device_inheriting(), + ) + } + }; + let mut rights = HandleRights::new(base, inheriting); + let flags = unsafe { fcntl::get_status_flags(file.as_raw_fd())? }; + let accmode = flags & OFlag::ACCMODE; + if accmode == OFlag::RDONLY { + rights.base &= !types::Rights::FD_WRITE; + } else if accmode == OFlag::WRONLY { + rights.base &= !types::Rights::FD_READ; + } + Ok(rights) +} diff --git a/crates/wasi-common/src/sys/windows/fd.rs b/crates/wasi-common/src/sys/windows/fd.rs index 0cc20a79bb41..690079b8533e 100644 --- a/crates/wasi-common/src/sys/windows/fd.rs +++ b/crates/wasi-common/src/sys/windows/fd.rs @@ -1,7 +1,9 @@ use super::file_serial_no; -use super::oshandle::OsFile; +use super::oshandle::OsHandle; use crate::path; -use crate::sys::oshandle::AsFile; +use crate::sys::osdir::OsDir; +use crate::sys::osfile::OsFile; +use crate::sys::AsFile; use crate::wasi::{types, Result}; use log::trace; use std::convert::TryInto; @@ -39,7 +41,7 @@ pub(crate) fn fdstat_get(file: &File) -> Result { // handle came from `CreateFile`, but the Rust's libstd will use `GetStdHandle` // rather than `CreateFile`. Relevant discussion can be found in: // https://github.com/rust-lang/rust/issues/40490 -pub(crate) fn fdstat_set_flags(file: &File, fdflags: types::Fdflags) -> Result> { +pub(crate) fn fdstat_set_flags(file: &File, fdflags: types::Fdflags) -> Result> { let handle = file.as_raw_handle(); let access_mode = winx::file::query_access_information(handle)?; let new_access_mode = file_access_mode_from_fdflags( @@ -49,7 +51,7 @@ pub(crate) fn fdstat_set_flags(file: &File, fdflags: types::Fdflags) -> Result Result>>> { use winx::file::get_file_path; let cookie = cookie.try_into()?; - let path = get_file_path(&file.as_file())?; + let path = get_file_path(&dirfd.as_file())?; // std::fs::ReadDir doesn't return . and .., so we need to emulate it let path = Path::new(&path); // The directory /.. is the same as / on Unix (at least on ext4), so emulate this behavior too diff --git a/crates/wasi-common/src/sys/windows/mod.rs b/crates/wasi-common/src/sys/windows/mod.rs index c493115ebefe..a7757cb5a18a 100644 --- a/crates/wasi-common/src/sys/windows/mod.rs +++ b/crates/wasi-common/src/sys/windows/mod.rs @@ -1,18 +1,59 @@ pub(crate) mod clock; pub(crate) mod fd; +pub(crate) mod osdir; +pub(crate) mod osfile; pub(crate) mod oshandle; +pub(crate) mod osother; pub(crate) mod path; pub(crate) mod poll; +pub(crate) mod stdio; +use crate::sys::AsFile; use crate::wasi::{types, Errno, Result}; use std::convert::{TryFrom, TryInto}; use std::fs::File; +use std::mem::ManuallyDrop; +use std::os::windows::prelude::{AsRawHandle, FromRawHandle}; use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; use std::{io, string}; use winapi::shared::winerror; use winx::file::{CreationDisposition, Flags}; +impl AsFile for T { + fn as_file(&self) -> ManuallyDrop { + let file = unsafe { File::from_raw_handle(self.as_raw_handle()) }; + ManuallyDrop::new(file) + } +} + +pub(crate) fn get_file_type(file: &File) -> io::Result { + let file_type = unsafe { winx::file::get_file_type(file.as_raw_handle())? }; + let file_type = if file_type.is_char() { + // character file: LPT device or console + // TODO: rule out LPT device + types::Filetype::CharacterDevice + } else if file_type.is_disk() { + // disk file: file, dir or disk device + let file = file.as_file(); + let meta = file.metadata()?; + if meta.is_dir() { + types::Filetype::Directory + } else if meta.is_file() { + types::Filetype::RegularFile + } else { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } + } else if file_type.is_pipe() { + // pipe object: socket, named pipe or anonymous pipe + // TODO: what about pipes, etc? + types::Filetype::SocketStream + } else { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + }; + Ok(file_type) +} + pub fn preopen_dir>(path: P) -> io::Result { use std::fs::OpenOptions; use std::os::windows::fs::OpenOptionsExt; diff --git a/crates/wasi-common/src/sys/windows/osdir.rs b/crates/wasi-common/src/sys/windows/osdir.rs new file mode 100644 index 000000000000..88f4afd45c45 --- /dev/null +++ b/crates/wasi-common/src/sys/windows/osdir.rs @@ -0,0 +1,42 @@ +use super::oshandle::{OsDirHandle, OsHandle}; +use crate::handle::HandleRights; +use crate::sys::osdir::OsDir; +use crate::wasi::{types, RightsExt}; +use std::convert::TryFrom; +use std::fs::File; +use std::io; +use std::ops::Deref; +use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; + +impl AsRawHandle for OsDir { + fn as_raw_handle(&self) -> RawHandle { + self.deref().as_raw_handle() + } +} + +impl TryFrom for OsDir { + type Error = io::Error; + + fn try_from(file: File) -> io::Result { + let rights = get_rights(&file)?; + let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; + let handle = OsDirHandle::new(handle)?; + Ok(Self::new(rights, handle)) + } +} + +fn get_rights(file: &File) -> io::Result { + use winx::file::{query_access_information, AccessMode}; + let mut rights = HandleRights::new( + types::Rights::directory_base(), + types::Rights::directory_inheriting(), + ); + let mode = query_access_information(file.as_raw_handle())?; + if mode.contains(AccessMode::FILE_GENERIC_READ) { + rights.base |= types::Rights::FD_READ; + } + if mode.contains(AccessMode::FILE_GENERIC_WRITE) { + rights.base |= types::Rights::FD_WRITE; + } + Ok(rights) +} diff --git a/crates/wasi-common/src/sys/windows/osfile.rs b/crates/wasi-common/src/sys/windows/osfile.rs new file mode 100644 index 000000000000..7efc45519f8f --- /dev/null +++ b/crates/wasi-common/src/sys/windows/osfile.rs @@ -0,0 +1,49 @@ +use super::oshandle::OsHandle; +use crate::handle::{Handle, HandleRights}; +use crate::sys::osfile::{OsFile, OsFileExt}; +use crate::wasi::{types, RightsExt}; +use std::convert::TryFrom; +use std::fs::{File, OpenOptions}; +use std::io; +use std::ops::Deref; +use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; + +impl AsRawHandle for OsFile { + fn as_raw_handle(&self) -> RawHandle { + self.deref().as_raw_handle() + } +} + +impl TryFrom for OsFile { + type Error = io::Error; + + fn try_from(file: File) -> io::Result { + let rights = get_rights(&file)?; + let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; + Ok(Self::new(rights, handle)) + } +} + +fn get_rights(file: &File) -> io::Result { + use winx::file::{query_access_information, AccessMode}; + let mut rights = HandleRights::new( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ); + let mode = query_access_information(file.as_raw_handle())?; + if mode.contains(AccessMode::FILE_GENERIC_READ) { + rights.base |= types::Rights::FD_READ; + } + if mode.contains(AccessMode::FILE_GENERIC_WRITE) { + rights.base |= types::Rights::FD_WRITE; + } + Ok(rights) +} + +impl OsFileExt for OsFile { + fn from_null() -> io::Result> { + let file = OpenOptions::new().read(true).write(true).open("NUL")?; + let file = Self::try_from(file)?; + Ok(Box::new(file)) + } +} diff --git a/crates/wasi-common/src/sys/windows/oshandle.rs b/crates/wasi-common/src/sys/windows/oshandle.rs index a313416a2abd..2fbdc133dec9 100644 --- a/crates/wasi-common/src/sys/windows/oshandle.rs +++ b/crates/wasi-common/src/sys/windows/oshandle.rs @@ -1,16 +1,20 @@ -use crate::entry::EntryRights; -use crate::sys::oshandle::{AsFile, OsHandle, OsHandleExt}; -use crate::wasi::{types, RightsExt}; +use crate::sys::AsFile; use std::cell::Cell; -use std::fs::{File, OpenOptions}; +use std::fs::File; use std::io; use std::mem::ManuallyDrop; +use std::ops::Deref; use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; #[derive(Debug)] -pub(crate) struct OsFile(Cell); +pub(crate) struct OsHandle(Cell); -impl OsFile { +impl OsHandle { + /// Tries clone `self`. + pub(crate) fn try_clone(&self) -> io::Result { + let handle = self.as_file().try_clone()?; + Ok(Self(Cell::new(handle.into_raw_handle()))) + } /// Consumes `other` taking the ownership of the underlying /// `RawHandle` file handle. pub(crate) fn update_from(&self, other: Self) { @@ -22,14 +26,9 @@ impl OsFile { File::from_raw_handle(old_handle); } } - /// Clones `self`. - pub(crate) fn try_clone(&self) -> io::Result { - let handle = self.as_file().try_clone()?; - Ok(Self(Cell::new(handle.into_raw_handle()))) - } } -impl Drop for OsFile { +impl Drop for OsHandle { fn drop(&mut self) { unsafe { File::from_raw_handle(self.as_raw_handle()); @@ -37,19 +36,19 @@ impl Drop for OsFile { } } -impl AsRawHandle for OsFile { +impl AsRawHandle for OsHandle { fn as_raw_handle(&self) -> RawHandle { self.0.get() } } -impl FromRawHandle for OsFile { +impl FromRawHandle for OsHandle { unsafe fn from_raw_handle(handle: RawHandle) -> Self { Self(Cell::new(handle)) } } -impl IntoRawHandle for OsFile { +impl IntoRawHandle for OsHandle { fn into_raw_handle(self) -> RawHandle { // We need to prevent dropping of the OsFile let wrapped = ManuallyDrop::new(self); @@ -57,113 +56,25 @@ impl IntoRawHandle for OsFile { } } -impl AsFile for OsFile { - fn as_file(&self) -> ManuallyDrop { - let file = unsafe { File::from_raw_handle(self.0.get()) }; - ManuallyDrop::new(file) - } -} - -impl AsRawHandle for OsHandle { - fn as_raw_handle(&self) -> RawHandle { - match self { - Self::OsFile(file) => file.as_raw_handle(), - Self::Stdin => io::stdin().as_raw_handle(), - Self::Stdout => io::stdout().as_raw_handle(), - Self::Stderr => io::stderr().as_raw_handle(), - } - } -} +#[derive(Debug)] +pub(crate) struct OsDirHandle(OsHandle); -impl AsFile for OsHandle { - fn as_file(&self) -> ManuallyDrop { - let file = unsafe { File::from_raw_handle(self.as_raw_handle()) }; - ManuallyDrop::new(file) +impl OsDirHandle { + /// Consumes the spcified `OsHandle`. + pub(crate) fn new(handle: OsHandle) -> io::Result { + Ok(Self(handle)) } -} - -impl From for OsHandle { - fn from(file: File) -> Self { - Self::from(unsafe { OsFile::from_raw_handle(file.into_raw_handle()) }) + /// Tries clone `self`. + pub(crate) fn try_clone(&self) -> io::Result { + let handle = self.0.try_clone()?; + Self::new(handle) } } -impl OsHandleExt for OsHandle { - fn get_file_type(&self) -> io::Result { - let file_type = unsafe { winx::file::get_file_type(self.as_raw_handle())? }; - let file_type = if file_type.is_char() { - // character file: LPT device or console - // TODO: rule out LPT device - types::Filetype::CharacterDevice - } else if file_type.is_disk() { - // disk file: file, dir or disk device - let file = self.as_file(); - let meta = file.metadata()?; - if meta.is_dir() { - types::Filetype::Directory - } else if meta.is_file() { - types::Filetype::RegularFile - } else { - return Err(io::Error::from_raw_os_error(libc::EINVAL)); - } - } else if file_type.is_pipe() { - // pipe object: socket, named pipe or anonymous pipe - // TODO: what about pipes, etc? - types::Filetype::SocketStream - } else { - return Err(io::Error::from_raw_os_error(libc::EINVAL)); - }; - Ok(file_type) - } - - fn get_rights(&self, file_type: types::Filetype) -> io::Result { - use winx::file::{query_access_information, AccessMode}; - let (base, inheriting) = match file_type { - types::Filetype::BlockDevice => ( - types::Rights::block_device_base(), - types::Rights::block_device_inheriting(), - ), - types::Filetype::CharacterDevice => { - (types::Rights::tty_base(), types::Rights::tty_base()) - } - types::Filetype::Directory => ( - types::Rights::directory_base(), - types::Rights::directory_inheriting(), - ), - types::Filetype::RegularFile => ( - types::Rights::regular_file_base(), - types::Rights::regular_file_inheriting(), - ), - types::Filetype::SocketDgram | types::Filetype::SocketStream => ( - types::Rights::socket_base(), - types::Rights::socket_inheriting(), - ), - types::Filetype::SymbolicLink | types::Filetype::Unknown => ( - types::Rights::regular_file_base(), - types::Rights::regular_file_inheriting(), - ), - }; - let mut rights = EntryRights::new(base, inheriting); - match file_type { - types::Filetype::Directory | types::Filetype::RegularFile => { - let mode = query_access_information(self.as_raw_handle())?; - if mode.contains(AccessMode::FILE_GENERIC_READ) { - rights.base |= types::Rights::FD_READ; - } - if mode.contains(AccessMode::FILE_GENERIC_WRITE) { - rights.base |= types::Rights::FD_WRITE; - } - } - _ => { - // TODO: is there a way around this? On windows, it seems - // we cannot check access rights for anything but dirs and regular files - } - } - Ok(rights) - } +impl Deref for OsDirHandle { + type Target = OsHandle; - fn from_null() -> io::Result { - let file = OpenOptions::new().read(true).write(true).open("NUL")?; - Ok(Self::from(file)) + fn deref(&self) -> &Self::Target { + &self.0 } } diff --git a/crates/wasi-common/src/sys/windows/osother.rs b/crates/wasi-common/src/sys/windows/osother.rs new file mode 100644 index 000000000000..4f7d1a0eb6c3 --- /dev/null +++ b/crates/wasi-common/src/sys/windows/osother.rs @@ -0,0 +1,48 @@ +use super::get_file_type; +use super::oshandle::OsHandle; +use crate::handle::HandleRights; +use crate::sys::osother::OsOther; +use crate::wasi::{types, RightsExt}; +use std::convert::TryFrom; +use std::fs::File; +use std::io; +use std::ops::Deref; +use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; + +impl AsRawHandle for OsOther { + fn as_raw_handle(&self) -> RawHandle { + self.deref().as_raw_handle() + } +} + +impl TryFrom for OsOther { + type Error = io::Error; + + fn try_from(file: File) -> io::Result { + let file_type = get_file_type(&file)?; + let rights = get_rights(&file_type)?; + let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; + Ok(Self::new(file_type, rights, handle)) + } +} + +fn get_rights(file_type: &types::Filetype) -> io::Result { + let (base, inheriting) = match file_type { + types::Filetype::BlockDevice => ( + types::Rights::block_device_base(), + types::Rights::block_device_inheriting(), + ), + types::Filetype::CharacterDevice => (types::Rights::tty_base(), types::Rights::tty_base()), + types::Filetype::SocketDgram | types::Filetype::SocketStream => ( + types::Rights::socket_base(), + types::Rights::socket_inheriting(), + ), + types::Filetype::SymbolicLink | types::Filetype::Unknown => ( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ), + _ => unreachable!("these should have been handled already!"), + }; + let rights = HandleRights::new(base, inheriting); + Ok(rights) +} diff --git a/crates/wasi-common/src/sys/windows/path.rs b/crates/wasi-common/src/sys/windows/path.rs index a33ffc08c67c..e743f47c918d 100644 --- a/crates/wasi-common/src/sys/windows/path.rs +++ b/crates/wasi-common/src/sys/windows/path.rs @@ -1,7 +1,8 @@ -use super::oshandle::OsFile; -use crate::entry::EntryRights; -use crate::sys::oshandle::{AsFile, OsHandle}; +use crate::handle::{Handle, HandleRights}; +use crate::sys::osdir::OsDir; +use crate::sys::AsFile; use crate::wasi::{types, Errno, Result}; +use std::convert::TryFrom; use std::ffi::{OsStr, OsString}; use std::fs::{self, Metadata, OpenOptions}; use std::os::windows::ffi::{OsStrExt, OsStringExt}; @@ -10,7 +11,7 @@ use std::path::{Path, PathBuf}; use winapi::shared::winerror; use winx::file::AccessMode; -fn strip_trailing_slashes_and_concatenate(dirfd: &OsFile, path: &str) -> Result> { +fn strip_trailing_slashes_and_concatenate(dirfd: &OsDir, path: &str) -> Result> { if path.ends_with('/') { let suffix = path.trim_end_matches('/'); concatenate(dirfd, Path::new(suffix)).map(Some) @@ -28,7 +29,7 @@ fn strip_extended_prefix>(path: P) -> OsString { } } -fn concatenate>(file: &OsFile, path: P) -> Result { +fn concatenate>(file: &OsDir, path: P) -> Result { use winx::file::get_file_path; // WASI is not able to deal with absolute paths @@ -89,10 +90,10 @@ pub(crate) fn from_host>(s: S) -> Result { } pub(crate) fn open_rights( - input_rights: &EntryRights, + input_rights: &HandleRights, oflags: types::Oflags, fdflags: types::Fdflags, -) -> EntryRights { +) -> HandleRights { // which rights are needed on the dirfd? let mut needed_base = types::Rights::PATH_OPEN; let mut needed_inheriting = input_rights.base | input_rights.inheriting; @@ -113,10 +114,10 @@ pub(crate) fn open_rights( needed_inheriting |= types::Rights::FD_SYNC; } - EntryRights::new(needed_base, needed_inheriting) + HandleRights::new(needed_base, needed_inheriting) } -pub(crate) fn readlinkat(dirfd: &OsFile, s_path: &str) -> Result { +pub(crate) fn readlinkat(dirfd: &OsDir, s_path: &str) -> Result { use winx::file::get_file_path; let path = concatenate(dirfd, Path::new(s_path))?; @@ -151,16 +152,16 @@ pub(crate) fn readlinkat(dirfd: &OsFile, s_path: &str) -> Result { Err(err.into()) } -pub(crate) fn create_directory(file: &OsFile, path: &str) -> Result<()> { +pub(crate) fn create_directory(file: &OsDir, path: &str) -> Result<()> { let path = concatenate(file, path)?; std::fs::create_dir(&path)?; Ok(()) } pub(crate) fn link( - old_dirfd: &OsFile, + old_dirfd: &OsDir, old_path: &str, - new_dirfd: &OsFile, + new_dirfd: &OsDir, new_path: &str, follow_symlinks: bool, ) -> Result<()> { @@ -197,13 +198,13 @@ pub(crate) fn link( } pub(crate) fn open( - dirfd: &OsFile, + dirfd: &OsDir, path: &str, read: bool, write: bool, oflags: types::Oflags, fdflags: types::Fdflags, -) -> Result { +) -> Result> { use winx::file::{AccessMode, CreationDisposition, Flags}; let is_trunc = oflags.contains(&types::Oflags::TRUNC); @@ -280,11 +281,11 @@ pub(crate) fn open( .access_mode(access_mode.bits()) .custom_flags(flags.bits()) .open(&path)?; - let handle = OsHandle::from(file); + let handle = >::try_from(file)?; Ok(handle) } -pub(crate) fn readlink(dirfd: &OsFile, path: &str, buf: &mut [u8]) -> Result { +pub(crate) fn readlink(dirfd: &OsDir, path: &str, buf: &mut [u8]) -> Result { use winx::file::get_file_path; let path = concatenate(dirfd, path)?; @@ -322,9 +323,9 @@ pub(crate) fn readlink(dirfd: &OsFile, path: &str, buf: &mut [u8]) -> Result Result<()> { use std::fs; @@ -390,7 +391,7 @@ pub(crate) fn rename( } } -pub(crate) fn symlink(old_path: &str, new_dirfd: &OsFile, new_path_: &str) -> Result<()> { +pub(crate) fn symlink(old_path: &str, new_dirfd: &OsDir, new_path_: &str) -> Result<()> { use std::os::windows::fs::{symlink_dir, symlink_file}; let old_path = concatenate(new_dirfd, Path::new(old_path))?; @@ -447,7 +448,7 @@ pub(crate) fn symlink(old_path: &str, new_dirfd: &OsFile, new_path_: &str) -> Re } } -pub(crate) fn unlink_file(dirfd: &OsFile, path: &str) -> Result<()> { +pub(crate) fn unlink_file(dirfd: &OsDir, path: &str) -> Result<()> { use std::fs; let path = concatenate(dirfd, path)?; @@ -489,7 +490,7 @@ pub(crate) fn unlink_file(dirfd: &OsFile, path: &str) -> Result<()> { } } -pub(crate) fn remove_directory(dirfd: &OsFile, path: &str) -> Result<()> { +pub(crate) fn remove_directory(dirfd: &OsDir, path: &str) -> Result<()> { let path = concatenate(dirfd, path)?; std::fs::remove_dir(&path).map_err(Into::into) } diff --git a/crates/wasi-common/src/sys/windows/poll.rs b/crates/wasi-common/src/sys/windows/poll.rs index 7e56c3982971..285820c30940 100644 --- a/crates/wasi-common/src/sys/windows/poll.rs +++ b/crates/wasi-common/src/sys/windows/poll.rs @@ -1,11 +1,14 @@ -use super::super::oshandle::OsHandle; +use crate::handle::Handle; use crate::poll::{ClockEventData, FdEventData}; -use crate::sys::oshandle::AsFile; +use crate::sys::osdir::OsDir; +use crate::sys::osfile::OsFile; +use crate::sys::osother::OsOther; +use crate::sys::stdio::Stdio; +use crate::sys::AsFile; use crate::wasi::{types, Errno, Result}; use lazy_static::lazy_static; use log::{debug, error, trace, warn}; use std::convert::TryInto; -use std::os::windows::io::AsRawHandle; use std::sync::mpsc::{self, Receiver, RecvTimeoutError, Sender, TryRecvError}; use std::sync::Mutex; use std::thread; @@ -141,30 +144,56 @@ fn handle_timeout_event(timeout_event: ClockEventData, events: &mut Vec) { - let handle = event - .handle - .as_any() - .downcast_ref::() - .expect("can poll FdEvent for OS resources only"); - let size = match handle { - OsHandle::OsFile(file) => { - if event.r#type == types::Eventtype::FdRead { - file.as_file() - .metadata() - .map(|m| m.len()) - .map_err(Into::into) - } else { - // The spec is unclear what nbytes should actually be for __WASI_EVENTTYPE_FD_WRITE and - // the implementation on Unix just returns 0 here, so it's probably fine - // to do the same on Windows for now. - // cf. https://github.com/WebAssembly/WASI/issues/148 - Ok(0) - } + let handle = &event.handle; + let size = if let Some(file) = handle.as_any().downcast_ref::() { + if event.r#type == types::Eventtype::FdRead { + file.as_file() + .metadata() + .map(|m| m.len()) + .map_err(Into::into) + } else { + // The spec is unclear what nbytes should actually be for __WASI_EVENTTYPE_FD_WRITE and + // the implementation on Unix just returns 0 here, so it's probably fine + // to do the same on Windows for now. + // cf. https://github.com/WebAssembly/WASI/issues/148 + Ok(0) } - // We return the only universally correct lower bound, see the comment later in the function. - OsHandle::Stdin => Ok(1), - // On Unix, ioctl(FIONREAD) will return 0 for stdout/stderr. Emulate the same behavior on Windows. - OsHandle::Stdout | OsHandle::Stderr => Ok(0), + } else if let Some(dir) = handle.as_any().downcast_ref::() { + if event.r#type == types::Eventtype::FdRead { + dir.as_file() + .metadata() + .map(|m| m.len()) + .map_err(Into::into) + } else { + // The spec is unclear what nbytes should actually be for __WASI_EVENTTYPE_FD_WRITE and + // the implementation on Unix just returns 0 here, so it's probably fine + // to do the same on Windows for now. + // cf. https://github.com/WebAssembly/WASI/issues/148 + Ok(0) + } + } else if let Some(stdio) = handle.as_any().downcast_ref::() { + match stdio { + // We return the only universally correct lower bound, see the comment later in the function. + Stdio::In { .. } => Ok(1), + // On Unix, ioctl(FIONREAD) will return 0 for stdout/stderr. Emulate the same behavior on Windows. + Stdio::Out { .. } | Stdio::Err { .. } => Ok(0), + } + } else if let Some(other) = handle.as_any().downcast_ref::() { + if event.r#type == types::Eventtype::FdRead { + other + .as_file() + .metadata() + .map(|m| m.len()) + .map_err(Into::into) + } else { + // The spec is unclear what nbytes should actually be for __WASI_EVENTTYPE_FD_WRITE and + // the implementation on Unix just returns 0 here, so it's probably fine + // to do the same on Windows for now. + // cf. https://github.com/WebAssembly/WASI/issues/148 + Ok(0) + } + } else { + panic!("can poll FdEvent for OS resources only") }; let new_event = make_rw_event(&event, size); @@ -206,33 +235,37 @@ pub(crate) fn oneoff( let mut pipe_events = vec![]; for event in fd_events { - let handle = event - .handle - .as_any() - .downcast_ref::() - .expect("can poll FdEvent for OS resources only"); - match handle { - OsHandle::Stdin if event.r#type == types::Eventtype::FdRead => stdin_events.push(event), - // stdout/stderr are always considered ready to write because there seems to - // be no way of checking if a write to stdout would block. - // - // If stdin is polled for anything else then reading, then it is also - // considered immediately ready, following the behavior on Linux. - OsHandle::Stdin | OsHandle::Stderr | OsHandle::Stdout => immediate_events.push(event), - OsHandle::OsFile(file) => { - let ftype = unsafe { winx::file::get_file_type(file.as_raw_handle()) }?; - if ftype.is_unknown() || ftype.is_char() { - debug!("poll_oneoff: unsupported file type: {:?}", ftype); - handle_error_event(event, Errno::Notsup, events); - } else if ftype.is_disk() { - immediate_events.push(event); - } else if ftype.is_pipe() { - pipe_events.push(event); - } else { - unreachable!(); + let handle = &event.handle; + if let Some(_) = handle.as_any().downcast_ref::() { + immediate_events.push(event); + } else if let Some(_) = handle.as_any().downcast_ref::() { + immediate_events.push(event); + } else if let Some(stdio) = handle.as_any().downcast_ref::() { + match stdio { + Stdio::In { .. } if event.r#type == types::Eventtype::FdRead => { + stdin_events.push(event) } + // stdout/stderr are always considered ready to write because there seems to + // be no way of checking if a write to stdout would block. + // + // If stdin is polled for anything else then reading, then it is also + // considered immediately ready, following the behavior on Linux. + _ => immediate_events.push(event), + }; + } else if let Some(other) = handle.as_any().downcast_ref::() { + if other.get_file_type() == types::Filetype::SocketStream { + // We map pipe to SocketStream + pipe_events.push(event); + } else { + debug!( + "poll_oneoff: unsupported file type: {}", + other.get_file_type() + ); + handle_error_event(event, Errno::Notsup, events); } - }; + } else { + panic!("can poll FdEvent for OS resources only"); + } } let immediate = !immediate_events.is_empty(); diff --git a/crates/wasi-common/src/sys/windows/stdio.rs b/crates/wasi-common/src/sys/windows/stdio.rs new file mode 100644 index 000000000000..abc100746532 --- /dev/null +++ b/crates/wasi-common/src/sys/windows/stdio.rs @@ -0,0 +1,39 @@ +use crate::handle::{Handle, HandleRights}; +use crate::sys::stdio::{Stdio, StdioExt}; +use crate::wasi::{types, RightsExt}; +use std::cell::Cell; +use std::io; +use std::os::windows::prelude::{AsRawHandle, RawHandle}; + +impl AsRawHandle for Stdio { + fn as_raw_handle(&self) -> RawHandle { + match self { + Self::In { .. } => io::stdin().as_raw_handle(), + Self::Out { .. } => io::stdout().as_raw_handle(), + Self::Err { .. } => io::stderr().as_raw_handle(), + } + } +} + +impl StdioExt for Stdio { + fn stdin() -> io::Result> { + let rights = get_rights()?; + let rights = Cell::new(rights); + Ok(Box::new(Self::In { rights })) + } + fn stdout() -> io::Result> { + let rights = get_rights()?; + let rights = Cell::new(rights); + Ok(Box::new(Self::Out { rights })) + } + fn stderr() -> io::Result> { + let rights = get_rights()?; + let rights = Cell::new(rights); + Ok(Box::new(Self::Err { rights })) + } +} + +fn get_rights() -> io::Result { + let rights = HandleRights::new(types::Rights::tty_base(), types::Rights::tty_base()); + Ok(rights) +} diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index 521119f428d4..ed2f47fcbdf8 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -1,5 +1,4 @@ -use crate::entry::EntryRights; -use crate::handle::Handle; +use crate::handle::{Handle, HandleRights}; use crate::wasi::{self, types, Errno, Result, RightsExt}; use log::trace; use std::any::Any; @@ -134,6 +133,7 @@ impl VecFileContents { /// a filesystem wherein a file descriptor is one view into a possibly-shared underlying collection /// of data and permissions on a filesystem. pub struct InMemoryFile { + rights: Cell, cursor: Cell, parent: Rc>>>, fd_flags: Cell, @@ -142,16 +142,17 @@ pub struct InMemoryFile { impl InMemoryFile { pub fn memory_backed() -> Self { - Self { - cursor: Cell::new(0), - parent: Rc::new(RefCell::new(None)), - fd_flags: Cell::new(types::Fdflags::empty()), - data: Rc::new(RefCell::new(Box::new(VecFileContents::new()))), - } + Self::new(Box::new(VecFileContents::new())) } pub fn new(contents: Box) -> Self { + let rights = HandleRights::new( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ); + let rights = Cell::new(rights); Self { + rights, cursor: Cell::new(0), fd_flags: Cell::new(types::Fdflags::empty()), parent: Rc::new(RefCell::new(None)), @@ -172,20 +173,21 @@ impl Handle for InMemoryFile { } fn try_clone(&self) -> io::Result> { Ok(Box::new(Self { + rights: self.rights.clone(), cursor: Cell::new(0), fd_flags: self.fd_flags.clone(), parent: Rc::clone(&self.parent), data: Rc::clone(&self.data), })) } - fn get_file_type(&self) -> io::Result { - Ok(types::Filetype::RegularFile) + fn get_file_type(&self) -> types::Filetype { + types::Filetype::RegularFile } - fn get_rights(&self) -> io::Result { - Ok(EntryRights::new( - types::Rights::regular_file_base(), - types::Rights::regular_file_inheriting(), - )) + fn get_rights(&self) -> HandleRights { + self.rights.get() + } + fn set_rights(&self, rights: HandleRights) { + self.rights.set(rights) } // FdOps fn advise( @@ -227,7 +229,7 @@ impl Handle for InMemoryFile { atim: 0, ctim: 0, mtim: 0, - filetype: self.get_file_type()?, + filetype: self.get_file_type(), }; Ok(stat) } @@ -402,6 +404,7 @@ impl Handle for InMemoryFile { /// A clonable read/write directory. pub struct VirtualDir { + rights: Cell, writable: bool, // All copies of this `VirtualDir` must share `parent`, and changes in one copy's `parent` // must be reflected in all handles, so they share `Rc` of an underlying `parent`. @@ -411,7 +414,13 @@ pub struct VirtualDir { impl VirtualDir { pub fn new(writable: bool) -> Self { + let rights = HandleRights::new( + types::Rights::directory_base(), + types::Rights::directory_inheriting(), + ); + let rights = Cell::new(rights); Self { + rights, writable, parent: Rc::new(RefCell::new(None)), entries: Rc::new(RefCell::new(HashMap::new())), @@ -468,19 +477,20 @@ impl Handle for VirtualDir { } fn try_clone(&self) -> io::Result> { Ok(Box::new(Self { + rights: self.rights.clone(), writable: self.writable, parent: Rc::clone(&self.parent), entries: Rc::clone(&self.entries), })) } - fn get_file_type(&self) -> io::Result { - Ok(types::Filetype::Directory) + fn get_file_type(&self) -> types::Filetype { + types::Filetype::Directory } - fn get_rights(&self) -> io::Result { - Ok(EntryRights::new( - types::Rights::directory_base(), - types::Rights::directory_inheriting(), - )) + fn get_rights(&self) -> HandleRights { + self.rights.get() + } + fn set_rights(&self, rights: HandleRights) { + self.rights.set(rights) } // FdOps fn filestat_get(&self) -> Result { @@ -492,7 +502,7 @@ impl Handle for VirtualDir { atim: 0, ctim: 0, mtim: 0, - filetype: self.get_file_type()?, + filetype: self.get_file_type(), }; Ok(stat) } @@ -555,7 +565,7 @@ impl Handle for VirtualDir { let dirent = || -> Result { let dirent = types::Dirent { d_namlen: name.len().try_into()?, - d_type: file.get_file_type()?, + d_type: file.get_file_type(), d_ino: 0, d_next: self.start as u64, }; @@ -639,7 +649,7 @@ impl Handle for VirtualDir { } if oflags.contains(&types::Oflags::DIRECTORY) - && e.get().get_file_type()? != types::Filetype::Directory + && e.get().get_file_type() != types::Filetype::Directory { log::trace!( "VirtualDir::openat was passed oflags DIRECTORY, but {:?} is a file.", @@ -683,7 +693,7 @@ impl Handle for VirtualDir { match entries.entry(Path::new(trimmed_path).to_path_buf()) { Entry::Occupied(e) => { // first, does this name a directory? - if e.get().get_file_type()? != types::Filetype::Directory { + if e.get().get_file_type() != types::Filetype::Directory { return Err(Errno::Notdir); } @@ -731,7 +741,7 @@ impl Handle for VirtualDir { match entries.entry(Path::new(trimmed_path).to_path_buf()) { Entry::Occupied(e) => { // Directories must be removed through `remove_directory`, not `unlink_file`. - if e.get().get_file_type()? == types::Filetype::Directory { + if e.get().get_file_type() == types::Filetype::Directory { return Err(Errno::Isdir); } From 0a82c46648443f3dd9740dbddd37de48cb249c83 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 20 Apr 2020 21:55:40 +0200 Subject: [PATCH 02/16] Do not adjust rights on Stdio --- crates/wasi-common/src/sys/unix/stdio.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/crates/wasi-common/src/sys/unix/stdio.rs b/crates/wasi-common/src/sys/unix/stdio.rs index 5d55f5848f70..8e8dc847107a 100644 --- a/crates/wasi-common/src/sys/unix/stdio.rs +++ b/crates/wasi-common/src/sys/unix/stdio.rs @@ -42,8 +42,7 @@ impl StdioExt for Stdio { } fn get_rights(file: &File) -> io::Result { - use yanix::fcntl; - use yanix::file::{isatty, OFlag}; + use yanix::file::isatty; let (base, inheriting) = { if unsafe { isatty(file.as_raw_fd())? } { (types::Rights::tty_base(), types::Rights::tty_base()) @@ -54,13 +53,5 @@ fn get_rights(file: &File) -> io::Result { ) } }; - let mut rights = HandleRights::new(base, inheriting); - let flags = unsafe { fcntl::get_status_flags(file.as_raw_fd())? }; - let accmode = flags & OFlag::ACCMODE; - if accmode == OFlag::RDONLY { - rights.base &= !types::Rights::FD_WRITE; - } else if accmode == OFlag::WRONLY { - rights.base &= !types::Rights::FD_READ; - } - Ok(rights) + Ok(HandleRights::new(base, inheriting)) } From c2590344757c21abf95160e1ab62aa0f5fc6aaba Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 20 Apr 2020 23:57:18 +0200 Subject: [PATCH 03/16] Clean up testing for TTY and escaping writes --- crates/wasi-common/src/entry.rs | 14 +------------- crates/wasi-common/src/handle.rs | 11 ++++++++++- .../src/snapshots/wasi_snapshot_preview1.rs | 3 +-- crates/wasi-common/src/sys/osfile.rs | 12 +++--------- crates/wasi-common/src/sys/osother.rs | 4 ++-- crates/wasi-common/src/sys/stdio.rs | 4 ++-- crates/wasi-common/src/virtfs.rs | 6 +----- 7 files changed, 20 insertions(+), 34 deletions(-) diff --git a/crates/wasi-common/src/entry.rs b/crates/wasi-common/src/entry.rs index 1b219efbedc3..e8e8e18b546d 100644 --- a/crates/wasi-common/src/entry.rs +++ b/crates/wasi-common/src/entry.rs @@ -1,5 +1,5 @@ use crate::handle::{Handle, HandleRights}; -use crate::wasi::types::{Filetype, Rights}; +use crate::wasi::types::Filetype; use crate::wasi::{Errno, Result}; use std::ops::Deref; use std::path::PathBuf; @@ -90,16 +90,4 @@ impl Entry { Err(Errno::Notcapable) } } - - // TODO we should probably have a separate struct representing - // char device - /// Test whether this descriptor is considered a tty within WASI. - /// Note that since WASI itself lacks an `isatty` syscall and relies - /// on a conservative approximation, we use the same approximation here. - pub(crate) fn isatty(&self) -> bool { - let file_type = self.handle.get_file_type(); - let rights = self.handle.get_rights(); - let required_rights = HandleRights::from_base(Rights::FD_SEEK | Rights::FD_TELL); - file_type == Filetype::CharacterDevice && rights.contains(&required_rights) - } } diff --git a/crates/wasi-common/src/handle.rs b/crates/wasi-common/src/handle.rs index 8d0cf2191fe6..21a0ab4f2c3e 100644 --- a/crates/wasi-common/src/handle.rs +++ b/crates/wasi-common/src/handle.rs @@ -61,6 +61,15 @@ pub(crate) trait Handle { fn is_directory(&self) -> bool { self.get_file_type() == types::Filetype::Directory } + /// Test whether this descriptor is considered a tty within WASI. + /// Note that since WASI itself lacks an `isatty` syscall and relies + /// on a conservative approximation, we use the same approximation here. + fn is_tty(&self) -> bool { + let file_type = self.get_file_type(); + let rights = self.get_rights(); + let required_rights = HandleRights::from_base(Rights::FD_SEEK | Rights::FD_TELL); + file_type == types::Filetype::CharacterDevice && rights.contains(&required_rights) + } // TODO perhaps should be a separate trait? // FdOps fn advise( @@ -118,7 +127,7 @@ pub(crate) trait Handle { fn sync(&self) -> Result<()> { Ok(()) } - fn write_vectored(&self, _iovs: &[io::IoSlice], _isatty: bool) -> Result { + fn write_vectored(&self, _iovs: &[io::IoSlice]) -> Result { Err(Errno::Badf) } // TODO perhaps should be a separate trait? diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index 865f8f489b2d..4ab53f5d6ef8 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -438,10 +438,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } 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) } diff --git a/crates/wasi-common/src/sys/osfile.rs b/crates/wasi-common/src/sys/osfile.rs index ed255389f563..1d6974f2c3b2 100644 --- a/crates/wasi-common/src/sys/osfile.rs +++ b/crates/wasi-common/src/sys/osfile.rs @@ -1,7 +1,6 @@ use super::sys_impl::oshandle::OsHandle; use super::{fd, AsFile}; use crate::handle::{Handle, HandleRights}; -use crate::sandboxed_tty_writer::SandboxedTTYWriter; use crate::wasi::{types, Errno, Result}; use std::any::Any; use std::cell::Cell; @@ -115,7 +114,7 @@ impl Handle for OsFile { let mut fd: &File = &self.as_file(); let cur_pos = fd.seek(SeekFrom::Current(0))?; fd.seek(SeekFrom::Start(offset))?; - let nwritten = self.write_vectored(&buf, false)?; + let nwritten = self.write_vectored(&buf)?; fd.seek(SeekFrom::Start(cur_pos))?; Ok(nwritten) } @@ -131,13 +130,8 @@ impl Handle for OsFile { self.as_file().sync_all()?; Ok(()) } - fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result { - let mut file: &File = &self.as_file(); - let nwritten = if isatty { - SandboxedTTYWriter::new(&mut file).write_vectored(&iovs)? - } else { - file.write_vectored(&iovs)? - }; + fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { + let nwritten = self.as_file().write_vectored(&iovs)?; Ok(nwritten) } } diff --git a/crates/wasi-common/src/sys/osother.rs b/crates/wasi-common/src/sys/osother.rs index 47016f75d5bd..d63c11d0a91f 100644 --- a/crates/wasi-common/src/sys/osother.rs +++ b/crates/wasi-common/src/sys/osother.rs @@ -73,9 +73,9 @@ impl Handle for OsOther { let nread = self.as_file().read_vectored(iovs)?; Ok(nread) } - fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result { + fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { let mut fd: &File = &self.as_file(); - let nwritten = if isatty { + let nwritten = if self.is_tty() { SandboxedTTYWriter::new(&mut fd).write_vectored(&iovs)? } else { fd.write_vectored(iovs)? diff --git a/crates/wasi-common/src/sys/stdio.rs b/crates/wasi-common/src/sys/stdio.rs index 45d81c7ee645..bdbb4fcd233b 100644 --- a/crates/wasi-common/src/sys/stdio.rs +++ b/crates/wasi-common/src/sys/stdio.rs @@ -65,14 +65,14 @@ impl Handle for Stdio { }; Ok(nread) } - fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result { + fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { let nwritten = match self { Self::In { .. } => return Err(Errno::Badf), Self::Out { .. } => { // lock for the duration of the scope let stdout = io::stdout(); let mut stdout = stdout.lock(); - let nwritten = if isatty { + let nwritten = if self.is_tty() { SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)? } else { stdout.write_vectored(&iovs)? diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index ed2f47fcbdf8..c868a279555d 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -282,11 +282,7 @@ impl Handle for InMemoryFile { Ok(self.cursor.get()) } - fn write_vectored(&self, iovs: &[io::IoSlice], isatty: bool) -> Result { - if isatty { - unimplemented!("writes to virtual tty"); - } - + fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { trace!("write_vectored(iovs={:?})", iovs); let mut data = self.data.borrow_mut(); From de881c7fa1576cbe6143c53864ce031346f86321 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 21 Apr 2020 22:32:07 +0200 Subject: [PATCH 04/16] Implement AsFile for dyn Handle This cleans up a lot of repeating boilerplate code todo with dynamic dispatch. --- crates/wasi-common/src/sys/mod.rs | 17 +++++++++ crates/wasi-common/src/sys/osdir.rs | 4 +-- crates/wasi-common/src/sys/unix/osdir.rs | 9 +---- crates/wasi-common/src/sys/unix/osfile.rs | 9 +---- crates/wasi-common/src/sys/unix/osother.rs | 9 +---- crates/wasi-common/src/sys/unix/poll.rs | 27 ++------------ crates/wasi-common/src/sys/windows/osdir.rs | 9 +---- crates/wasi-common/src/sys/windows/osfile.rs | 9 +---- crates/wasi-common/src/sys/windows/osother.rs | 9 +---- crates/wasi-common/src/sys/windows/poll.rs | 35 ++----------------- 10 files changed, 31 insertions(+), 106 deletions(-) diff --git a/crates/wasi-common/src/sys/mod.rs b/crates/wasi-common/src/sys/mod.rs index c1652a9a59d4..4286cc8b5faa 100644 --- a/crates/wasi-common/src/sys/mod.rs +++ b/crates/wasi-common/src/sys/mod.rs @@ -33,12 +33,29 @@ use std::convert::TryFrom; use std::fs::File; use std::io; use std::mem::ManuallyDrop; +use stdio::Stdio; use sys_impl::get_file_type; pub(crate) trait AsFile { fn as_file(&self) -> ManuallyDrop; } +impl AsFile for dyn Handle + 'static { + fn as_file(&self) -> ManuallyDrop { + if let Some(file) = self.as_any().downcast_ref::() { + file.as_file() + } else if let Some(dir) = self.as_any().downcast_ref::() { + dir.as_file() + } else if let Some(stdio) = self.as_any().downcast_ref::() { + stdio.as_file() + } else if let Some(other) = self.as_any().downcast_ref::() { + other.as_file() + } else { + panic!("non-OS resource cannot be made into a File") + } + } +} + impl TryFrom for Box { type Error = io::Error; diff --git a/crates/wasi-common/src/sys/osdir.rs b/crates/wasi-common/src/sys/osdir.rs index 406b239c2696..f53e94937d62 100644 --- a/crates/wasi-common/src/sys/osdir.rs +++ b/crates/wasi-common/src/sys/osdir.rs @@ -97,7 +97,7 @@ impl Handle for OsDir { ) -> Result<()> { let new_handle = match new_handle.as_any().downcast_ref::() { None => { - error!("Tried to link OS resource with Virtual"); + error!("Tried to link with handle that's not an OsDir"); return Err(Errno::Badf); } Some(handle) => handle, @@ -116,7 +116,7 @@ impl Handle for OsDir { fn rename(&self, old_path: &str, new_handle: Box, new_path: &str) -> Result<()> { let new_handle = match new_handle.as_any().downcast_ref::() { None => { - error!("Tried to link OS resource with Virtual"); + error!("Tried to rename with handle that's not an OsDir"); return Err(Errno::Badf); } Some(handle) => handle, diff --git a/crates/wasi-common/src/sys/unix/osdir.rs b/crates/wasi-common/src/sys/unix/osdir.rs index aab45df306e1..c85707eeefab 100644 --- a/crates/wasi-common/src/sys/unix/osdir.rs +++ b/crates/wasi-common/src/sys/unix/osdir.rs @@ -5,14 +5,7 @@ use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; use std::fs::File; use std::io; -use std::ops::Deref; -use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; - -impl AsRawFd for OsDir { - fn as_raw_fd(&self) -> RawFd { - self.deref().as_raw_fd() - } -} +use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd}; impl TryFrom for OsDir { type Error = io::Error; diff --git a/crates/wasi-common/src/sys/unix/osfile.rs b/crates/wasi-common/src/sys/unix/osfile.rs index de888a338a82..8c72dc75a9bb 100644 --- a/crates/wasi-common/src/sys/unix/osfile.rs +++ b/crates/wasi-common/src/sys/unix/osfile.rs @@ -5,14 +5,7 @@ use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; use std::fs::{File, OpenOptions}; use std::io; -use std::ops::Deref; -use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; - -impl AsRawFd for OsFile { - fn as_raw_fd(&self) -> RawFd { - self.deref().as_raw_fd() - } -} +use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd}; impl TryFrom for OsFile { type Error = io::Error; diff --git a/crates/wasi-common/src/sys/unix/osother.rs b/crates/wasi-common/src/sys/unix/osother.rs index ce9d189bd317..17e24d9dc223 100644 --- a/crates/wasi-common/src/sys/unix/osother.rs +++ b/crates/wasi-common/src/sys/unix/osother.rs @@ -6,14 +6,7 @@ use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; use std::fs::File; use std::io; -use std::ops::Deref; -use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; - -impl AsRawFd for OsOther { - fn as_raw_fd(&self) -> RawFd { - self.deref().as_raw_fd() - } -} +use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd}; impl TryFrom for OsOther { type Error = io::Error; diff --git a/crates/wasi-common/src/sys/unix/poll.rs b/crates/wasi-common/src/sys/unix/poll.rs index da932007a9fc..7dfb0d1d366f 100644 --- a/crates/wasi-common/src/sys/unix/poll.rs +++ b/crates/wasi-common/src/sys/unix/poll.rs @@ -1,9 +1,6 @@ use crate::entry::EntryHandle; use crate::poll::{ClockEventData, FdEventData}; -use crate::sys::osdir::OsDir; use crate::sys::osfile::OsFile; -use crate::sys::osother::OsOther; -use crate::sys::stdio::Stdio; use crate::sys::AsFile; use crate::wasi::{types, Errno, Result}; use std::io; @@ -32,18 +29,7 @@ pub(crate) fn oneoff( // events we filtered before. If we get something else here, the code has a serious bug. _ => unreachable!(), }; - let fd = if let Some(file) = event.handle.as_any().downcast_ref::() { - file.as_raw_fd() - } else if let Some(dir) = event.handle.as_any().downcast_ref::() { - dir.as_raw_fd() - } else if let Some(stdio) = event.handle.as_any().downcast_ref::() { - stdio.as_raw_fd() - } else if let Some(other) = event.handle.as_any().downcast_ref::() { - other.as_raw_fd() - } else { - panic!("can poll FdEvent for OS resources only") - }; - unsafe { PollFd::new(fd, flags) } + unsafe { PollFd::new(event.handle.as_file().as_raw_fd(), flags) } }) .collect(); @@ -97,16 +83,9 @@ fn handle_fd_event( let meta = file.as_file().metadata()?; let len = meta.len(); let host_offset = unsafe { tell(file.as_raw_fd())? }; - Ok(len - host_offset) - } else if let Some(dir) = handle.as_any().downcast_ref::() { - unsafe { Ok(fionread(dir.as_raw_fd())?.into()) } - } else if let Some(stdio) = handle.as_any().downcast_ref::() { - unsafe { Ok(fionread(stdio.as_raw_fd())?.into()) } - } else if let Some(other) = handle.as_any().downcast_ref::() { - unsafe { Ok(fionread(other.as_raw_fd())?.into()) } - } else { - panic!("can poll FdEvent for OS resources only") + return Ok(len - host_offset); } + Ok(unsafe { fionread(handle.as_file().as_raw_fd())?.into() }) } for (fd_event, poll_fd) in ready_events { diff --git a/crates/wasi-common/src/sys/windows/osdir.rs b/crates/wasi-common/src/sys/windows/osdir.rs index 88f4afd45c45..c3bd56d9c283 100644 --- a/crates/wasi-common/src/sys/windows/osdir.rs +++ b/crates/wasi-common/src/sys/windows/osdir.rs @@ -5,14 +5,7 @@ use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; use std::fs::File; use std::io; -use std::ops::Deref; -use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; - -impl AsRawHandle for OsDir { - fn as_raw_handle(&self) -> RawHandle { - self.deref().as_raw_handle() - } -} +use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle}; impl TryFrom for OsDir { type Error = io::Error; diff --git a/crates/wasi-common/src/sys/windows/osfile.rs b/crates/wasi-common/src/sys/windows/osfile.rs index 7efc45519f8f..3559a580c148 100644 --- a/crates/wasi-common/src/sys/windows/osfile.rs +++ b/crates/wasi-common/src/sys/windows/osfile.rs @@ -5,14 +5,7 @@ use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; use std::fs::{File, OpenOptions}; use std::io; -use std::ops::Deref; -use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; - -impl AsRawHandle for OsFile { - fn as_raw_handle(&self) -> RawHandle { - self.deref().as_raw_handle() - } -} +use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle}; impl TryFrom for OsFile { type Error = io::Error; diff --git a/crates/wasi-common/src/sys/windows/osother.rs b/crates/wasi-common/src/sys/windows/osother.rs index 4f7d1a0eb6c3..16b0b8657c79 100644 --- a/crates/wasi-common/src/sys/windows/osother.rs +++ b/crates/wasi-common/src/sys/windows/osother.rs @@ -6,14 +6,7 @@ use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; use std::fs::File; use std::io; -use std::ops::Deref; -use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; - -impl AsRawHandle for OsOther { - fn as_raw_handle(&self) -> RawHandle { - self.deref().as_raw_handle() - } -} +use std::os::windows::prelude::{FromRawHandle, IntoRawHandle}; impl TryFrom for OsOther { type Error = io::Error; diff --git a/crates/wasi-common/src/sys/windows/poll.rs b/crates/wasi-common/src/sys/windows/poll.rs index 285820c30940..ed847869564b 100644 --- a/crates/wasi-common/src/sys/windows/poll.rs +++ b/crates/wasi-common/src/sys/windows/poll.rs @@ -145,42 +145,16 @@ fn handle_timeout_event(timeout_event: ClockEventData, events: &mut Vec) { let handle = &event.handle; - let size = if let Some(file) = handle.as_any().downcast_ref::() { - if event.r#type == types::Eventtype::FdRead { - file.as_file() - .metadata() - .map(|m| m.len()) - .map_err(Into::into) - } else { - // The spec is unclear what nbytes should actually be for __WASI_EVENTTYPE_FD_WRITE and - // the implementation on Unix just returns 0 here, so it's probably fine - // to do the same on Windows for now. - // cf. https://github.com/WebAssembly/WASI/issues/148 - Ok(0) - } - } else if let Some(dir) = handle.as_any().downcast_ref::() { - if event.r#type == types::Eventtype::FdRead { - dir.as_file() - .metadata() - .map(|m| m.len()) - .map_err(Into::into) - } else { - // The spec is unclear what nbytes should actually be for __WASI_EVENTTYPE_FD_WRITE and - // the implementation on Unix just returns 0 here, so it's probably fine - // to do the same on Windows for now. - // cf. https://github.com/WebAssembly/WASI/issues/148 - Ok(0) - } - } else if let Some(stdio) = handle.as_any().downcast_ref::() { + let size = if let Some(stdio) = handle.as_any().downcast_ref::() { match stdio { // We return the only universally correct lower bound, see the comment later in the function. Stdio::In { .. } => Ok(1), // On Unix, ioctl(FIONREAD) will return 0 for stdout/stderr. Emulate the same behavior on Windows. Stdio::Out { .. } | Stdio::Err { .. } => Ok(0), } - } else if let Some(other) = handle.as_any().downcast_ref::() { + } else { if event.r#type == types::Eventtype::FdRead { - other + handle .as_file() .metadata() .map(|m| m.len()) @@ -192,10 +166,7 @@ fn handle_rw_event(event: FdEventData, out_events: &mut Vec) { // cf. https://github.com/WebAssembly/WASI/issues/148 Ok(0) } - } else { - panic!("can poll FdEvent for OS resources only") }; - let new_event = make_rw_event(&event, size); out_events.push(new_event); } From 18cd22319cff89a1f848f93b36aff5cfb6cd1629 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 22 Apr 2020 11:34:07 +0200 Subject: [PATCH 05/16] 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? --- crates/wasi-common/src/sys/osdir.rs | 25 ++++------ crates/wasi-common/src/sys/osfile.rs | 1 + crates/wasi-common/src/sys/stdio.rs | 3 ++ crates/wasi-common/src/sys/unix/bsd/mod.rs | 2 +- crates/wasi-common/src/sys/unix/bsd/osdir.rs | 46 +++++++++++++++++++ .../wasi-common/src/sys/unix/bsd/oshandle.rs | 42 ----------------- .../src/sys/unix/emscripten/mod.rs | 4 +- crates/wasi-common/src/sys/unix/fd.rs | 2 +- crates/wasi-common/src/sys/unix/linux/mod.rs | 2 +- .../sys/unix/linux/{oshandle.rs => osdir.rs} | 37 ++++++--------- crates/wasi-common/src/sys/unix/osdir.rs | 8 ++-- crates/wasi-common/src/sys/unix/oshandle.rs | 2 - crates/wasi-common/src/sys/windows/osdir.rs | 20 ++++++-- .../wasi-common/src/sys/windows/oshandle.rs | 26 +---------- 14 files changed, 98 insertions(+), 122 deletions(-) create mode 100644 crates/wasi-common/src/sys/unix/bsd/osdir.rs delete mode 100644 crates/wasi-common/src/sys/unix/bsd/oshandle.rs rename crates/wasi-common/src/sys/unix/linux/{oshandle.rs => osdir.rs} (55%) diff --git a/crates/wasi-common/src/sys/osdir.rs b/crates/wasi-common/src/sys/osdir.rs index f53e94937d62..1def35f39f52 100644 --- a/crates/wasi-common/src/sys/osdir.rs +++ b/crates/wasi-common/src/sys/osdir.rs @@ -1,28 +1,19 @@ -use super::sys_impl::oshandle::OsDirHandle; +use super::sys_impl::oshandle::OsHandle; use super::{fd, path, AsFile}; use crate::handle::{Handle, HandleRights}; use crate::wasi::{types, Errno, Result}; use log::{debug, error}; use std::any::Any; -use std::cell::Cell; use std::io; use std::ops::Deref; -#[derive(Debug)] -pub(crate) struct OsDir { - rights: Cell, - handle: OsDirHandle, -} - -impl OsDir { - pub(super) fn new(rights: HandleRights, handle: OsDirHandle) -> Self { - let rights = Cell::new(rights); - Self { rights, handle } - } -} +// TODO could this be cleaned up? +// The actual `OsDir` struct is OS-dependent, therefore we delegate +// its definition to OS-specific modules. +pub(crate) use super::sys_impl::osdir::OsDir; impl Deref for OsDir { - type Target = OsDirHandle; + type Target = OsHandle; fn deref(&self) -> &Self::Target { &self.handle @@ -35,8 +26,8 @@ impl Handle for OsDir { } fn try_clone(&self) -> io::Result> { let handle = self.handle.try_clone()?; - let rights = self.rights.clone(); - Ok(Box::new(Self { rights, handle })) + let new = Self::new(self.rights.get(), handle)?; + Ok(Box::new(new)) } fn get_file_type(&self) -> types::Filetype { types::Filetype::Directory diff --git a/crates/wasi-common/src/sys/osfile.rs b/crates/wasi-common/src/sys/osfile.rs index 1d6974f2c3b2..b44c015af021 100644 --- a/crates/wasi-common/src/sys/osfile.rs +++ b/crates/wasi-common/src/sys/osfile.rs @@ -9,6 +9,7 @@ use std::io::{self, Read, Seek, SeekFrom, Write}; use std::ops::Deref; pub(crate) trait OsFileExt { + /// Create `OsFile` as `dyn Handle` from null device. fn from_null() -> io::Result>; } diff --git a/crates/wasi-common/src/sys/stdio.rs b/crates/wasi-common/src/sys/stdio.rs index bdbb4fcd233b..ec10e1ea98cc 100644 --- a/crates/wasi-common/src/sys/stdio.rs +++ b/crates/wasi-common/src/sys/stdio.rs @@ -8,8 +8,11 @@ use std::cell::Cell; use std::io::{self, Read, Write}; pub(crate) trait StdioExt: Sized { + /// Create `Stdio` from `io::stdin`. fn stdin() -> io::Result>; + /// Create `Stdio` from `io::stdout`. fn stdout() -> io::Result>; + /// Create `Stdio` from `io::stderr`. fn stderr() -> io::Result>; } diff --git a/crates/wasi-common/src/sys/unix/bsd/mod.rs b/crates/wasi-common/src/sys/unix/bsd/mod.rs index a58b813e4aa5..a521318cdf13 100644 --- a/crates/wasi-common/src/sys/unix/bsd/mod.rs +++ b/crates/wasi-common/src/sys/unix/bsd/mod.rs @@ -1,4 +1,4 @@ -pub(crate) mod oshandle; +pub(crate) mod osdir; pub(crate) mod path; pub(crate) const O_RSYNC: yanix::file::OFlag = yanix::file::OFlag::SYNC; diff --git a/crates/wasi-common/src/sys/unix/bsd/osdir.rs b/crates/wasi-common/src/sys/unix/bsd/osdir.rs new file mode 100644 index 000000000000..03d5682a86d6 --- /dev/null +++ b/crates/wasi-common/src/sys/unix/bsd/osdir.rs @@ -0,0 +1,46 @@ +use crate::handle::HandleRights; +use crate::sys::sys_impl::oshandle::OsHandle; +use crate::wasi::Result; +use std::cell::{Cell, RefCell, RefMut}; +use std::io; +use yanix::dir::Dir; + +#[derive(Debug)] +pub(crate) struct OsDir { + pub(crate) rights: Cell, + pub(crate) handle: OsHandle, + // When the client makes a `fd_readdir` syscall on this descriptor, + // we will need to cache the `libc::DIR` pointer manually in order + // to be able to seek on it later. While on Linux, this is handled + // by the OS, BSD Unixes require the client to do this caching. + // + // This comes directly from the BSD man pages on `readdir`: + // > Values returned by telldir() are good only for the lifetime + // > of the DIR pointer, dirp, from which they are derived. + // > If the directory is closed and then reopened, prior values + // > returned by telldir() will no longer be valid. + stream_ptr: RefCell, +} + +impl OsDir { + pub(crate) fn new(rights: HandleRights, handle: OsHandle) -> io::Result { + let rights = Cell::new(rights); + // We need to duplicate the handle, because `opendir(3)`: + // Upon successful return from fdopendir(), the file descriptor is under + // control of the system, and if any attempt is made to close the file + // descriptor, or to modify the state of the associated description other + // than by means of closedir(), readdir(), readdir_r(), or rewinddir(), + // the behaviour is undefined. + let stream_ptr = Dir::from(handle.try_clone()?)?; + let stream_ptr = RefCell::new(stream_ptr); + Ok(Self { + rights, + handle, + stream_ptr, + }) + } + /// Returns the `Dir` stream pointer associated with this `OsDir`. + pub(crate) fn stream_ptr(&self) -> Result> { + Ok(self.stream_ptr.borrow_mut()) + } +} diff --git a/crates/wasi-common/src/sys/unix/bsd/oshandle.rs b/crates/wasi-common/src/sys/unix/bsd/oshandle.rs deleted file mode 100644 index 5a926d224382..000000000000 --- a/crates/wasi-common/src/sys/unix/bsd/oshandle.rs +++ /dev/null @@ -1,42 +0,0 @@ -use crate::sys::sys_impl::oshandle::OsHandle; -use crate::wasi::Result; -use std::cell::{RefCell, RefMut}; -use std::io; -use std::ops::Deref; -use yanix::dir::Dir; - -#[derive(Debug)] -pub(crate) struct OsDirHandle { - handle: OsHandle, - dir: RefCell, -} - -impl OsDirHandle { - /// Consumes the spcified `OsHandle` and initialises the - /// underlying `Dir` stream pointer. - pub(crate) fn new(handle: OsHandle) -> io::Result { - let dir = Dir::from(handle.try_clone()?)?; - let dir = RefCell::new(dir); - Ok(Self { handle, dir }) - } - /// Tries clone `self`. - /// - /// Note that the `Dir` stream pointer will be reset - /// to start. - pub(crate) fn try_clone(&self) -> io::Result { - let handle = self.handle.try_clone()?; - Self::new(handle) - } - /// Gets mutable reference to the current dir stream pointer. - pub(crate) fn dir_stream(&self) -> Result> { - Ok(self.dir.borrow_mut()) - } -} - -impl Deref for OsDirHandle { - type Target = OsHandle; - - fn deref(&self) -> &Self::Target { - &self.handle - } -} diff --git a/crates/wasi-common/src/sys/unix/emscripten/mod.rs b/crates/wasi-common/src/sys/unix/emscripten/mod.rs index 8f233de8fe4e..ed0472e45ddc 100644 --- a/crates/wasi-common/src/sys/unix/emscripten/mod.rs +++ b/crates/wasi-common/src/sys/unix/emscripten/mod.rs @@ -1,5 +1,5 @@ -#[path = "../linux/oshandle.rs"] -pub(crate) mod oshandle; +#[path = "../linux/osdir.rs"] +pub(crate) mod osdir; #[path = "../linux/path.rs"] pub(crate) mod path; diff --git a/crates/wasi-common/src/sys/unix/fd.rs b/crates/wasi-common/src/sys/unix/fd.rs index b218e0d1e766..02d00c136dd1 100644 --- a/crates/wasi-common/src/sys/unix/fd.rs +++ b/crates/wasi-common/src/sys/unix/fd.rs @@ -54,7 +54,7 @@ pub(crate) fn readdir<'a>( // Get an instance of `Dir`; this is host-specific due to intricasies // of managing a dir stream between Linux and BSD *nixes - let mut dir = dirfd.dir_stream()?; + let mut dir = dirfd.stream_ptr()?; // Seek if needed. Unless cookie is wasi::__WASI_DIRCOOKIE_START, // new items may not be returned to the caller. diff --git a/crates/wasi-common/src/sys/unix/linux/mod.rs b/crates/wasi-common/src/sys/unix/linux/mod.rs index 9d7cc60191cc..d8bd3e75015c 100644 --- a/crates/wasi-common/src/sys/unix/linux/mod.rs +++ b/crates/wasi-common/src/sys/unix/linux/mod.rs @@ -1,4 +1,4 @@ -pub(crate) mod oshandle; +pub(crate) mod osdir; pub(crate) mod path; pub(crate) const O_RSYNC: yanix::file::OFlag = yanix::file::OFlag::RSYNC; diff --git a/crates/wasi-common/src/sys/unix/linux/oshandle.rs b/crates/wasi-common/src/sys/unix/linux/osdir.rs similarity index 55% rename from crates/wasi-common/src/sys/unix/linux/oshandle.rs rename to crates/wasi-common/src/sys/unix/linux/osdir.rs index e5306c934634..9f6ef2bc6d75 100644 --- a/crates/wasi-common/src/sys/unix/linux/oshandle.rs +++ b/crates/wasi-common/src/sys/unix/linux/osdir.rs @@ -1,25 +1,24 @@ +use crate::handle::HandleRights; use crate::sys::sys_impl::oshandle::OsHandle; use crate::wasi::Result; +use std::cell::Cell; use std::io; -use std::ops::Deref; use yanix::dir::Dir; #[derive(Debug)] -pub(crate) struct OsDirHandle(OsHandle); +pub(crate) struct OsDir { + pub(crate) rights: Cell, + pub(crate) handle: OsHandle, +} -impl OsDirHandle { - /// Consumes the spcified `OsHandle`. - pub(crate) fn new(handle: OsHandle) -> io::Result { - Ok(Self(handle)) - } - /// Tries clone `self`. - pub(crate) fn try_clone(&self) -> io::Result { - let handle = self.0.try_clone()?; - Self::new(handle) +impl OsDir { + pub(crate) fn new(rights: HandleRights, handle: OsHandle) -> io::Result { + let rights = Cell::new(rights); + Ok(Self { rights, handle }) } - /// Gets current dir stream pointer. - pub(crate) fn dir_stream(&self) -> Result> { - // We need to duplicate the fd, because `opendir(3)`: + /// Returns the `Dir` stream pointer associated with this `OsDir`. + pub(crate) fn stream_ptr(&self) -> Result> { + // We need to duplicate the handle, because `opendir(3)`: // After a successful call to fdopendir(), fd is used internally by the implementation, // and should not otherwise be used by the application. // `opendir(3p)` also says that it's undefined behavior to @@ -27,18 +26,10 @@ impl OsDirHandle { // // Still, rewinddir will be needed because the two file descriptors // share progress. But we can safely execute closedir now. - let file = self.0.try_clone()?; + let file = self.handle.try_clone()?; // TODO This doesn't look very clean. Can we do something about it? // Boxing is needed here in order to satisfy `yanix`'s trait requirement for the `DirIter` // where `T: Deref`. Ok(Box::new(Dir::from(file)?)) } } - -impl Deref for OsDirHandle { - type Target = OsHandle; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} diff --git a/crates/wasi-common/src/sys/unix/osdir.rs b/crates/wasi-common/src/sys/unix/osdir.rs index c85707eeefab..0e7377c5284a 100644 --- a/crates/wasi-common/src/sys/unix/osdir.rs +++ b/crates/wasi-common/src/sys/unix/osdir.rs @@ -1,20 +1,20 @@ -use super::oshandle::{OsDirHandle, OsHandle}; +use super::oshandle::OsHandle; use crate::handle::HandleRights; -use crate::sys::osdir::OsDir; use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; use std::fs::File; use std::io; use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd}; +pub(crate) use super::sys_impl::osdir::OsDir; + impl TryFrom for OsDir { type Error = io::Error; fn try_from(file: File) -> io::Result { let rights = get_rights(&file)?; let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; - let handle = OsDirHandle::new(handle)?; - Ok(Self::new(rights, handle)) + Self::new(rights, handle) } } diff --git a/crates/wasi-common/src/sys/unix/oshandle.rs b/crates/wasi-common/src/sys/unix/oshandle.rs index ea55431f4fe9..10d9f96b1855 100644 --- a/crates/wasi-common/src/sys/unix/oshandle.rs +++ b/crates/wasi-common/src/sys/unix/oshandle.rs @@ -5,8 +5,6 @@ use std::io; use std::mem::ManuallyDrop; use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; -pub(crate) use super::sys_impl::oshandle::*; - #[derive(Debug)] pub(crate) struct OsHandle(Cell); diff --git a/crates/wasi-common/src/sys/windows/osdir.rs b/crates/wasi-common/src/sys/windows/osdir.rs index c3bd56d9c283..88c03dafc022 100644 --- a/crates/wasi-common/src/sys/windows/osdir.rs +++ b/crates/wasi-common/src/sys/windows/osdir.rs @@ -1,20 +1,32 @@ -use super::oshandle::{OsDirHandle, OsHandle}; +use super::oshandle::OsHandle; use crate::handle::HandleRights; -use crate::sys::osdir::OsDir; use crate::wasi::{types, RightsExt}; +use std::cell::Cell; use std::convert::TryFrom; use std::fs::File; use std::io; use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle}; +#[derive(Debug)] +pub(crate) struct OsDir { + pub(crate) rights: Cell, + pub(crate) handle: OsHandle, +} + +impl OsDir { + pub(crate) fn new(rights: HandleRights, handle: OsHandle) -> io::Result { + let rights = Cell::new(rights); + Ok(Self { rights, handle }) + } +} + impl TryFrom for OsDir { type Error = io::Error; fn try_from(file: File) -> io::Result { let rights = get_rights(&file)?; let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; - let handle = OsDirHandle::new(handle)?; - Ok(Self::new(rights, handle)) + Self::new(rights, handle) } } diff --git a/crates/wasi-common/src/sys/windows/oshandle.rs b/crates/wasi-common/src/sys/windows/oshandle.rs index 2fbdc133dec9..a502328b37eb 100644 --- a/crates/wasi-common/src/sys/windows/oshandle.rs +++ b/crates/wasi-common/src/sys/windows/oshandle.rs @@ -3,14 +3,13 @@ use std::cell::Cell; use std::fs::File; use std::io; use std::mem::ManuallyDrop; -use std::ops::Deref; use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; #[derive(Debug)] pub(crate) struct OsHandle(Cell); impl OsHandle { - /// Tries clone `self`. + /// Tries cloning `self`. pub(crate) fn try_clone(&self) -> io::Result { let handle = self.as_file().try_clone()?; Ok(Self(Cell::new(handle.into_raw_handle()))) @@ -55,26 +54,3 @@ impl IntoRawHandle for OsHandle { wrapped.0.get() } } - -#[derive(Debug)] -pub(crate) struct OsDirHandle(OsHandle); - -impl OsDirHandle { - /// Consumes the spcified `OsHandle`. - pub(crate) fn new(handle: OsHandle) -> io::Result { - Ok(Self(handle)) - } - /// Tries clone `self`. - pub(crate) fn try_clone(&self) -> io::Result { - let handle = self.0.try_clone()?; - Self::new(handle) - } -} - -impl Deref for OsDirHandle { - type Target = OsHandle; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} From fa3fc43d9f07ac099937e0d772a29598064d2d29 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sun, 26 Apr 2020 09:48:25 +0200 Subject: [PATCH 06/16] 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. --- crates/wasi-common/src/ctx.rs | 15 ++++++------- crates/wasi-common/src/entry.rs | 4 ++++ crates/wasi-common/src/sys/osfile.rs | 5 ----- crates/wasi-common/src/sys/osother.rs | 5 +++++ crates/wasi-common/src/sys/unix/osdir.rs | 4 ++++ crates/wasi-common/src/sys/unix/osfile.rs | 21 +++++++------------ crates/wasi-common/src/sys/unix/osother.rs | 20 +++++++++++++++--- crates/wasi-common/src/sys/windows/osdir.rs | 4 ++++ crates/wasi-common/src/sys/windows/osfile.rs | 18 +++++++--------- crates/wasi-common/src/sys/windows/osother.rs | 17 ++++++++++++--- 10 files changed, 70 insertions(+), 43 deletions(-) diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 5bbe1582615d..3fb135e9959c 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -1,7 +1,8 @@ use crate::entry::{Entry, EntryHandle}; use crate::fdpool::FdPool; use crate::handle::Handle; -use crate::sys::osfile::{OsFile, OsFileExt}; +use crate::sys::osdir::OsDir; +use crate::sys::osother::{OsOther, OsOtherExt}; use crate::sys::stdio::{Stdio, StdioExt}; use crate::virtfs::{VirtualDir, VirtualDirEntry}; use crate::wasi::types; @@ -120,9 +121,9 @@ pub struct WasiCtxBuilder { impl WasiCtxBuilder { /// Builder for a new `WasiCtx`. pub fn new() -> Self { - let stdin = Some(PendingEntry::Thunk(OsFile::from_null)); - let stdout = Some(PendingEntry::Thunk(OsFile::from_null)); - let stderr = Some(PendingEntry::Thunk(OsFile::from_null)); + let stdin = Some(PendingEntry::Thunk(OsOther::from_null)); + let stdout = Some(PendingEntry::Thunk(OsOther::from_null)); + let stderr = Some(PendingEntry::Thunk(OsOther::from_null)); Self { stdin, @@ -253,7 +254,7 @@ impl WasiCtxBuilder { pub fn preopened_dir>(&mut self, dir: File, guest_path: P) -> &mut Self { self.preopens.as_mut().unwrap().push(( guest_path.as_ref().to_owned(), - >::try_from(dir).expect("valid handle"), + Box::new(OsDir::try_from(dir).expect("valid OsDir handle")), )); self } @@ -345,8 +346,8 @@ impl WasiCtxBuilder { .ok_or(WasiCtxBuilderError::TooManyFilesOpen)? } PendingEntry::File(f) => { - let handle = >::try_from(f)?; - let handle = EntryHandle::from(handle); + let handle = OsOther::try_from(f)?; + let handle = EntryHandle::new(handle); let entry = Entry::new(handle); entries .insert(entry) diff --git a/crates/wasi-common/src/entry.rs b/crates/wasi-common/src/entry.rs index e8e8e18b546d..eb311a7b10b2 100644 --- a/crates/wasi-common/src/entry.rs +++ b/crates/wasi-common/src/entry.rs @@ -8,6 +8,10 @@ use std::rc::Rc; pub(crate) struct EntryHandle(Rc); impl EntryHandle { + pub(crate) fn new(handle: T) -> Self { + Self(Rc::new(handle)) + } + pub(crate) fn get(&self) -> Self { Self(Rc::clone(&self.0)) } diff --git a/crates/wasi-common/src/sys/osfile.rs b/crates/wasi-common/src/sys/osfile.rs index b44c015af021..eafa6506f1ec 100644 --- a/crates/wasi-common/src/sys/osfile.rs +++ b/crates/wasi-common/src/sys/osfile.rs @@ -8,11 +8,6 @@ use std::fs::File; use std::io::{self, Read, Seek, SeekFrom, Write}; use std::ops::Deref; -pub(crate) trait OsFileExt { - /// Create `OsFile` as `dyn Handle` from null device. - fn from_null() -> io::Result>; -} - #[derive(Debug)] pub(crate) struct OsFile { rights: Cell, diff --git a/crates/wasi-common/src/sys/osother.rs b/crates/wasi-common/src/sys/osother.rs index d63c11d0a91f..e572510fa283 100644 --- a/crates/wasi-common/src/sys/osother.rs +++ b/crates/wasi-common/src/sys/osother.rs @@ -10,6 +10,11 @@ use std::fs::File; use std::io::{self, Read, Write}; use std::ops::Deref; +pub(crate) trait OsOtherExt { + /// Create `OsOther` as `dyn Handle` from null device. + fn from_null() -> io::Result>; +} + #[derive(Debug)] pub(crate) struct OsOther { file_type: Filetype, diff --git a/crates/wasi-common/src/sys/unix/osdir.rs b/crates/wasi-common/src/sys/unix/osdir.rs index 0e7377c5284a..e339c30b33f3 100644 --- a/crates/wasi-common/src/sys/unix/osdir.rs +++ b/crates/wasi-common/src/sys/unix/osdir.rs @@ -12,6 +12,10 @@ impl TryFrom for OsDir { type Error = io::Error; fn try_from(file: File) -> io::Result { + let ft = file.metadata()?.file_type(); + if !ft.is_dir() { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } let rights = get_rights(&file)?; let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; Self::new(rights, handle) diff --git a/crates/wasi-common/src/sys/unix/osfile.rs b/crates/wasi-common/src/sys/unix/osfile.rs index 8c72dc75a9bb..a3259b587d85 100644 --- a/crates/wasi-common/src/sys/unix/osfile.rs +++ b/crates/wasi-common/src/sys/unix/osfile.rs @@ -1,9 +1,9 @@ use super::oshandle::OsHandle; -use crate::handle::{Handle, HandleRights}; -use crate::sys::osfile::{OsFile, OsFileExt}; +use crate::handle::HandleRights; +use crate::sys::osfile::OsFile; use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; -use std::fs::{File, OpenOptions}; +use std::fs::File; use std::io; use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd}; @@ -11,6 +11,10 @@ impl TryFrom for OsFile { type Error = io::Error; fn try_from(file: File) -> io::Result { + let ft = file.metadata()?.file_type(); + if !ft.is_file() { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } let rights = get_rights(&file)?; let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; Ok(Self::new(rights, handle)) @@ -32,14 +36,3 @@ fn get_rights(file: &File) -> io::Result { } Ok(rights) } - -impl OsFileExt for OsFile { - fn from_null() -> io::Result> { - let file = OpenOptions::new() - .read(true) - .write(true) - .open("/dev/null")?; - let file = Self::try_from(file)?; - Ok(Box::new(file)) - } -} diff --git a/crates/wasi-common/src/sys/unix/osother.rs b/crates/wasi-common/src/sys/unix/osother.rs index 17e24d9dc223..266f888405c3 100644 --- a/crates/wasi-common/src/sys/unix/osother.rs +++ b/crates/wasi-common/src/sys/unix/osother.rs @@ -1,10 +1,10 @@ use super::get_file_type; use super::oshandle::OsHandle; -use crate::handle::HandleRights; -use crate::sys::osother::OsOther; +use crate::handle::{Handle, HandleRights}; +use crate::sys::osother::{OsOther, OsOtherExt}; use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; -use std::fs::File; +use std::fs::{File, OpenOptions}; use std::io; use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd}; @@ -13,6 +13,9 @@ impl TryFrom for OsOther { fn try_from(file: File) -> io::Result { let file_type = get_file_type(&file)?; + if file_type == types::Filetype::RegularFile || file_type == types::Filetype::Directory { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } let rights = get_rights(&file, &file_type)?; let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; Ok(Self::new(file_type, rights, handle)) @@ -57,3 +60,14 @@ fn get_rights(file: &File, file_type: &types::Filetype) -> io::Result io::Result> { + let file = OpenOptions::new() + .read(true) + .write(true) + .open("/dev/null")?; + let file = Self::try_from(file)?; + Ok(Box::new(file)) + } +} diff --git a/crates/wasi-common/src/sys/windows/osdir.rs b/crates/wasi-common/src/sys/windows/osdir.rs index 88c03dafc022..f7017c88802f 100644 --- a/crates/wasi-common/src/sys/windows/osdir.rs +++ b/crates/wasi-common/src/sys/windows/osdir.rs @@ -24,6 +24,10 @@ impl TryFrom for OsDir { type Error = io::Error; fn try_from(file: File) -> io::Result { + let ft = file.metadata()?.file_type(); + if !ft.is_dir() { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } let rights = get_rights(&file)?; let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; Self::new(rights, handle) diff --git a/crates/wasi-common/src/sys/windows/osfile.rs b/crates/wasi-common/src/sys/windows/osfile.rs index 3559a580c148..77f0fb3d16b2 100644 --- a/crates/wasi-common/src/sys/windows/osfile.rs +++ b/crates/wasi-common/src/sys/windows/osfile.rs @@ -1,9 +1,9 @@ use super::oshandle::OsHandle; -use crate::handle::{Handle, HandleRights}; -use crate::sys::osfile::{OsFile, OsFileExt}; +use crate::handle::HandleRights; +use crate::sys::osfile::OsFile; use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; -use std::fs::{File, OpenOptions}; +use std::fs::File; use std::io; use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle}; @@ -11,6 +11,10 @@ impl TryFrom for OsFile { type Error = io::Error; fn try_from(file: File) -> io::Result { + let ft = file.metadata()?.file_type(); + if !ft.is_file() { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } let rights = get_rights(&file)?; let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; Ok(Self::new(rights, handle)) @@ -32,11 +36,3 @@ fn get_rights(file: &File) -> io::Result { } Ok(rights) } - -impl OsFileExt for OsFile { - fn from_null() -> io::Result> { - let file = OpenOptions::new().read(true).write(true).open("NUL")?; - let file = Self::try_from(file)?; - Ok(Box::new(file)) - } -} diff --git a/crates/wasi-common/src/sys/windows/osother.rs b/crates/wasi-common/src/sys/windows/osother.rs index 16b0b8657c79..3ec5fd4f84be 100644 --- a/crates/wasi-common/src/sys/windows/osother.rs +++ b/crates/wasi-common/src/sys/windows/osother.rs @@ -1,10 +1,10 @@ use super::get_file_type; use super::oshandle::OsHandle; -use crate::handle::HandleRights; -use crate::sys::osother::OsOther; +use crate::handle::{Handle, HandleRights}; +use crate::sys::osother::{OsOther, OsOtherExt}; use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; -use std::fs::File; +use std::fs::{File, OpenOptions}; use std::io; use std::os::windows::prelude::{FromRawHandle, IntoRawHandle}; @@ -13,6 +13,9 @@ impl TryFrom for OsOther { fn try_from(file: File) -> io::Result { let file_type = get_file_type(&file)?; + if file_type == types::Filetype::RegularFile || file_type == types::Filetype::Directory { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } let rights = get_rights(&file_type)?; let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; Ok(Self::new(file_type, rights, handle)) @@ -39,3 +42,11 @@ fn get_rights(file_type: &types::Filetype) -> io::Result { let rights = HandleRights::new(base, inheriting); Ok(rights) } + +impl OsOtherExt for OsOther { + fn from_null() -> io::Result> { + let file = OpenOptions::new().read(true).write(true).open("NUL")?; + let file = Self::try_from(file)?; + Ok(Box::new(file)) + } +} From c0574b7aa6b714737708e9ef0722dba5fdac794f Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 28 Apr 2020 17:55:00 +0200 Subject: [PATCH 07/16] Make AsFile fallible Return `EBADF` in `AsFile` in case a `Handle` cannot be made into a `std::fs::File`. --- crates/wasi-common/src/sys/mod.rs | 7 ++--- crates/wasi-common/src/sys/osdir.rs | 8 +++--- crates/wasi-common/src/sys/osfile.rs | 26 +++++++++---------- crates/wasi-common/src/sys/osother.rs | 8 +++--- crates/wasi-common/src/sys/stdio.rs | 4 +-- crates/wasi-common/src/sys/unix/mod.rs | 4 +-- crates/wasi-common/src/sys/unix/oshandle.rs | 2 +- crates/wasi-common/src/sys/unix/poll.rs | 14 +++++----- crates/wasi-common/src/sys/windows/fd.rs | 2 +- crates/wasi-common/src/sys/windows/mod.rs | 6 ++--- .../wasi-common/src/sys/windows/oshandle.rs | 2 +- crates/wasi-common/src/sys/windows/path.rs | 6 ++--- crates/wasi-common/src/sys/windows/poll.rs | 5 ++-- 13 files changed, 49 insertions(+), 45 deletions(-) diff --git a/crates/wasi-common/src/sys/mod.rs b/crates/wasi-common/src/sys/mod.rs index 4286cc8b5faa..279a1002616e 100644 --- a/crates/wasi-common/src/sys/mod.rs +++ b/crates/wasi-common/src/sys/mod.rs @@ -37,11 +37,11 @@ use stdio::Stdio; use sys_impl::get_file_type; pub(crate) trait AsFile { - fn as_file(&self) -> ManuallyDrop; + fn as_file(&self) -> io::Result>; } impl AsFile for dyn Handle + 'static { - fn as_file(&self) -> ManuallyDrop { + fn as_file(&self) -> io::Result> { if let Some(file) = self.as_any().downcast_ref::() { file.as_file() } else if let Some(dir) = self.as_any().downcast_ref::() { @@ -51,7 +51,8 @@ impl AsFile for dyn Handle + 'static { } else if let Some(other) = self.as_any().downcast_ref::() { other.as_file() } else { - panic!("non-OS resource cannot be made into a File") + log::error!("tried to make std::fs::File from non-OS handle"); + Err(io::Error::from_raw_os_error(libc::EBADF)) } } } diff --git a/crates/wasi-common/src/sys/osdir.rs b/crates/wasi-common/src/sys/osdir.rs index 1def35f39f52..27cc845ea232 100644 --- a/crates/wasi-common/src/sys/osdir.rs +++ b/crates/wasi-common/src/sys/osdir.rs @@ -40,16 +40,16 @@ impl Handle for OsDir { } // FdOps fn fdstat_get(&self) -> Result { - fd::fdstat_get(&self.as_file()) + fd::fdstat_get(&*self.as_file()?) } fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { - if let Some(new_file) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { + if let Some(new_file) = fd::fdstat_set_flags(&*self.as_file()?, fdflags)? { self.handle.update_from(new_file); } Ok(()) } fn filestat_get(&self) -> Result { - fd::filestat_get(&self.as_file()) + fd::filestat_get(&*self.as_file()?) } fn filestat_set_times( &self, @@ -57,7 +57,7 @@ impl Handle for OsDir { mtim: types::Timestamp, fst_flags: types::Fstflags, ) -> Result<()> { - fd::filestat_set_times(&self.as_file(), atim, mtim, fst_flags) + fd::filestat_set_times(&*self.as_file()?, atim, mtim, fst_flags) } fn readdir<'a>( &'a self, diff --git a/crates/wasi-common/src/sys/osfile.rs b/crates/wasi-common/src/sys/osfile.rs index eafa6506f1ec..5d1bfb21fcaa 100644 --- a/crates/wasi-common/src/sys/osfile.rs +++ b/crates/wasi-common/src/sys/osfile.rs @@ -57,7 +57,7 @@ impl Handle for OsFile { fd::advise(self, advice, offset, len) } fn allocate(&self, offset: types::Filesize, len: types::Filesize) -> Result<()> { - let fd = self.as_file(); + let fd = self.as_file()?; let metadata = fd.metadata()?; let current_size = metadata.len(); let wanted_size = offset.checked_add(len).ok_or(Errno::TooBig)?; @@ -71,23 +71,23 @@ impl Handle for OsFile { Ok(()) } fn datasync(&self) -> Result<()> { - self.as_file().sync_data()?; + self.as_file()?.sync_data()?; Ok(()) } fn fdstat_get(&self) -> Result { - fd::fdstat_get(&self.as_file()) + fd::fdstat_get(&*self.as_file()?) } fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { - if let Some(new_handle) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { + if let Some(new_handle) = fd::fdstat_set_flags(&*self.as_file()?, fdflags)? { self.handle.update_from(new_handle); } Ok(()) } fn filestat_get(&self) -> Result { - fd::filestat_get(&self.as_file()) + fd::filestat_get(&*self.as_file()?) } fn filestat_set_size(&self, size: types::Filesize) -> Result<()> { - self.as_file().set_len(size)?; + self.as_file()?.set_len(size)?; Ok(()) } fn filestat_set_times( @@ -96,10 +96,10 @@ impl Handle for OsFile { mtim: types::Timestamp, fst_flags: types::Fstflags, ) -> Result<()> { - fd::filestat_set_times(&self.as_file(), atim, mtim, fst_flags) + fd::filestat_set_times(&*self.as_file()?, atim, mtim, fst_flags) } fn preadv(&self, buf: &mut [io::IoSliceMut], offset: u64) -> Result { - let mut fd: &File = &self.as_file(); + let mut fd: &File = &*self.as_file()?; let cur_pos = fd.seek(SeekFrom::Current(0))?; fd.seek(SeekFrom::Start(offset))?; let nread = self.read_vectored(buf)?; @@ -107,7 +107,7 @@ impl Handle for OsFile { Ok(nread) } fn pwritev(&self, buf: &[io::IoSlice], offset: u64) -> Result { - let mut fd: &File = &self.as_file(); + let mut fd: &File = &*self.as_file()?; let cur_pos = fd.seek(SeekFrom::Current(0))?; fd.seek(SeekFrom::Start(offset))?; let nwritten = self.write_vectored(&buf)?; @@ -115,19 +115,19 @@ impl Handle for OsFile { Ok(nwritten) } fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result { - let nread = self.as_file().read_vectored(iovs)?; + let nread = self.as_file()?.read_vectored(iovs)?; Ok(nread) } fn seek(&self, offset: SeekFrom) -> Result { - let pos = self.as_file().seek(offset)?; + let pos = self.as_file()?.seek(offset)?; Ok(pos) } fn sync(&self) -> Result<()> { - self.as_file().sync_all()?; + self.as_file()?.sync_all()?; Ok(()) } fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { - let nwritten = self.as_file().write_vectored(&iovs)?; + let nwritten = self.as_file()?.write_vectored(&iovs)?; Ok(nwritten) } } diff --git a/crates/wasi-common/src/sys/osother.rs b/crates/wasi-common/src/sys/osother.rs index e572510fa283..bf5c32c51cfd 100644 --- a/crates/wasi-common/src/sys/osother.rs +++ b/crates/wasi-common/src/sys/osother.rs @@ -66,20 +66,20 @@ impl Handle for OsOther { } // FdOps fn fdstat_get(&self) -> Result { - fd::fdstat_get(&self.as_file()) + fd::fdstat_get(&*self.as_file()?) } fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { - if let Some(handle) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { + if let Some(handle) = fd::fdstat_set_flags(&*self.as_file()?, fdflags)? { self.handle.update_from(handle); } Ok(()) } fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result { - let nread = self.as_file().read_vectored(iovs)?; + let nread = self.as_file()?.read_vectored(iovs)?; Ok(nread) } fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { - let mut fd: &File = &self.as_file(); + let mut fd: &File = &*self.as_file()?; let nwritten = if self.is_tty() { SandboxedTTYWriter::new(&mut fd).write_vectored(&iovs)? } else { diff --git a/crates/wasi-common/src/sys/stdio.rs b/crates/wasi-common/src/sys/stdio.rs index ec10e1ea98cc..90aec2b05583 100644 --- a/crates/wasi-common/src/sys/stdio.rs +++ b/crates/wasi-common/src/sys/stdio.rs @@ -50,10 +50,10 @@ impl Handle for Stdio { } // FdOps fn fdstat_get(&self) -> Result { - fd::fdstat_get(&self.as_file()) + fd::fdstat_get(&*self.as_file()?) } fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { - if let Some(_) = fd::fdstat_set_flags(&self.as_file(), fdflags)? { + if let Some(_) = fd::fdstat_set_flags(&*self.as_file()?, fdflags)? { // OK, this means we should somehow update the underlying os handle, // and we can't do that with `std::io::std{in, out, err}`, so we'll // panic for now. diff --git a/crates/wasi-common/src/sys/unix/mod.rs b/crates/wasi-common/src/sys/unix/mod.rs index 8768cd1e8368..c1fac592b0e4 100644 --- a/crates/wasi-common/src/sys/unix/mod.rs +++ b/crates/wasi-common/src/sys/unix/mod.rs @@ -40,9 +40,9 @@ use yanix::file::{AtFlag, OFlag}; pub(crate) use sys_impl::*; impl AsFile for T { - fn as_file(&self) -> ManuallyDrop { + fn as_file(&self) -> io::Result> { let file = unsafe { File::from_raw_fd(self.as_raw_fd()) }; - ManuallyDrop::new(file) + Ok(ManuallyDrop::new(file)) } } diff --git a/crates/wasi-common/src/sys/unix/oshandle.rs b/crates/wasi-common/src/sys/unix/oshandle.rs index 10d9f96b1855..8419839f08c5 100644 --- a/crates/wasi-common/src/sys/unix/oshandle.rs +++ b/crates/wasi-common/src/sys/unix/oshandle.rs @@ -11,7 +11,7 @@ pub(crate) struct OsHandle(Cell); impl OsHandle { /// Tries clone `self`. pub(crate) fn try_clone(&self) -> io::Result { - let fd = self.as_file().try_clone()?; + let fd = self.as_file()?.try_clone()?; Ok(Self(Cell::new(fd.into_raw_fd()))) } /// Consumes `other` taking the ownership of the underlying diff --git a/crates/wasi-common/src/sys/unix/poll.rs b/crates/wasi-common/src/sys/unix/poll.rs index 7dfb0d1d366f..b185bdb93c29 100644 --- a/crates/wasi-common/src/sys/unix/poll.rs +++ b/crates/wasi-common/src/sys/unix/poll.rs @@ -1,6 +1,5 @@ use crate::entry::EntryHandle; use crate::poll::{ClockEventData, FdEventData}; -use crate::sys::osfile::OsFile; use crate::sys::AsFile; use crate::wasi::{types, Errno, Result}; use std::io; @@ -17,7 +16,7 @@ pub(crate) fn oneoff( return Ok(()); } - let mut poll_fds: Vec<_> = fd_events + let poll_fds: Result> = fd_events .iter() .map(|event| { let mut flags = PollFlags::empty(); @@ -29,9 +28,11 @@ pub(crate) fn oneoff( // events we filtered before. If we get something else here, the code has a serious bug. _ => unreachable!(), }; - unsafe { PollFd::new(event.handle.as_file().as_raw_fd(), flags) } + let file = event.handle.as_file()?; + unsafe { Ok(PollFd::new(file.as_raw_fd(), flags)) } }) .collect(); + let mut poll_fds = poll_fds?; let poll_timeout = timeout.map_or(-1, |timeout| { let delay = timeout.delay / 1_000_000; // poll syscall requires delay to expressed in milliseconds @@ -77,15 +78,16 @@ fn handle_fd_event( events: &mut Vec, ) -> Result<()> { fn query_nbytes(handle: EntryHandle) -> Result { - if let Some(file) = handle.as_any().downcast_ref::() { + let file = handle.as_file()?; + if handle.get_file_type() == types::Filetype::RegularFile { // fionread may overflow for large files, so use another way for regular files. use yanix::file::tell; - let meta = file.as_file().metadata()?; + let meta = file.metadata()?; let len = meta.len(); let host_offset = unsafe { tell(file.as_raw_fd())? }; return Ok(len - host_offset); } - Ok(unsafe { fionread(handle.as_file().as_raw_fd())?.into() }) + Ok(unsafe { fionread(file.as_raw_fd())?.into() }) } for (fd_event, poll_fd) in ready_events { diff --git a/crates/wasi-common/src/sys/windows/fd.rs b/crates/wasi-common/src/sys/windows/fd.rs index 690079b8533e..1f89ff75a838 100644 --- a/crates/wasi-common/src/sys/windows/fd.rs +++ b/crates/wasi-common/src/sys/windows/fd.rs @@ -128,7 +128,7 @@ pub(crate) fn readdir( use winx::file::get_file_path; let cookie = cookie.try_into()?; - let path = get_file_path(&dirfd.as_file())?; + let path = get_file_path(&*dirfd.as_file()?)?; // std::fs::ReadDir doesn't return . and .., so we need to emulate it let path = Path::new(&path); // The directory /.. is the same as / on Unix (at least on ext4), so emulate this behavior too diff --git a/crates/wasi-common/src/sys/windows/mod.rs b/crates/wasi-common/src/sys/windows/mod.rs index a7757cb5a18a..40cb32103457 100644 --- a/crates/wasi-common/src/sys/windows/mod.rs +++ b/crates/wasi-common/src/sys/windows/mod.rs @@ -21,9 +21,9 @@ use winapi::shared::winerror; use winx::file::{CreationDisposition, Flags}; impl AsFile for T { - fn as_file(&self) -> ManuallyDrop { + fn as_file(&self) -> io::Result> { let file = unsafe { File::from_raw_handle(self.as_raw_handle()) }; - ManuallyDrop::new(file) + Ok(ManuallyDrop::new(file)) } } @@ -35,7 +35,7 @@ pub(crate) fn get_file_type(file: &File) -> io::Result { types::Filetype::CharacterDevice } else if file_type.is_disk() { // disk file: file, dir or disk device - let file = file.as_file(); + let file = file.as_file()?; let meta = file.metadata()?; if meta.is_dir() { types::Filetype::Directory diff --git a/crates/wasi-common/src/sys/windows/oshandle.rs b/crates/wasi-common/src/sys/windows/oshandle.rs index a502328b37eb..b46ce37cdfea 100644 --- a/crates/wasi-common/src/sys/windows/oshandle.rs +++ b/crates/wasi-common/src/sys/windows/oshandle.rs @@ -11,7 +11,7 @@ pub(crate) struct OsHandle(Cell); impl OsHandle { /// Tries cloning `self`. pub(crate) fn try_clone(&self) -> io::Result { - let handle = self.as_file().try_clone()?; + let handle = self.as_file()?.try_clone()?; Ok(Self(Cell::new(handle.into_raw_handle()))) } /// Consumes `other` taking the ownership of the underlying diff --git a/crates/wasi-common/src/sys/windows/path.rs b/crates/wasi-common/src/sys/windows/path.rs index e743f47c918d..d410866718da 100644 --- a/crates/wasi-common/src/sys/windows/path.rs +++ b/crates/wasi-common/src/sys/windows/path.rs @@ -38,7 +38,7 @@ fn concatenate>(file: &OsDir, path: P) -> Result { return Err(Errno::Notcapable); } - let dir_path = get_file_path(&file.as_file())?; + let dir_path = get_file_path(&*file.as_file()?)?; // concatenate paths let mut out_path = PathBuf::from(dir_path); out_path.push(path.as_ref()); @@ -127,7 +127,7 @@ pub(crate) fn readlinkat(dirfd: &OsDir, s_path: &str) -> Result { // we need to strip the prefix from the absolute path // as otherwise we will error out since WASI is not capable // of dealing with absolute paths - let dir_path = get_file_path(&dirfd.as_file())?; + let dir_path = get_file_path(&*dirfd.as_file()?)?; let dir_path = PathBuf::from(strip_extended_prefix(dir_path)); let target_path = target_path .strip_prefix(dir_path) @@ -295,7 +295,7 @@ pub(crate) fn readlink(dirfd: &OsDir, path: &str, buf: &mut [u8]) -> Result) { if event.r#type == types::Eventtype::FdRead { handle .as_file() - .metadata() + .and_then(|f| f.metadata()) .map(|m| m.len()) .map_err(Into::into) } else { @@ -235,7 +235,8 @@ pub(crate) fn oneoff( handle_error_event(event, Errno::Notsup, events); } } else { - panic!("can poll FdEvent for OS resources only"); + log::error!("can poll FdEvent for OS resources only"); + return Err(Errno::Badf); } } From 553a57da402c91ea660b04d9b5cdc56766d8bde6 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 28 Apr 2020 18:16:24 +0200 Subject: [PATCH 08/16] Remove unnecessary as_file conversion --- crates/wasi-common/src/sys/windows/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/wasi-common/src/sys/windows/mod.rs b/crates/wasi-common/src/sys/windows/mod.rs index 40cb32103457..b2a92ccb5260 100644 --- a/crates/wasi-common/src/sys/windows/mod.rs +++ b/crates/wasi-common/src/sys/windows/mod.rs @@ -35,7 +35,6 @@ pub(crate) fn get_file_type(file: &File) -> io::Result { types::Filetype::CharacterDevice } else if file_type.is_disk() { // disk file: file, dir or disk device - let file = file.as_file()?; let meta = file.metadata()?; if meta.is_dir() { types::Filetype::Directory From 94d8ea13d032ac360422cd917d3c720fb4bbcabd Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 29 Apr 2020 11:56:51 +0200 Subject: [PATCH 09/16] Remove unnecessary check for TTY for Stdio handle type --- crates/wasi-common/src/sys/stdio.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/crates/wasi-common/src/sys/stdio.rs b/crates/wasi-common/src/sys/stdio.rs index 90aec2b05583..3cbba5ae464e 100644 --- a/crates/wasi-common/src/sys/stdio.rs +++ b/crates/wasi-common/src/sys/stdio.rs @@ -16,6 +16,23 @@ pub(crate) trait StdioExt: Sized { fn stderr() -> io::Result>; } +// The reason we have a separate Stdio type is to correctly facilitate redirects on Windows. +// To elaborate further, in POSIX, we can get a stdio handle by opening a specific fd {0,1,2}. +// On Windows however, we need to issue a syscall that's separate from standard Windows "open" +// to get a console handle, and this is GetStdHandle. This is exactly what Rust does and what +// is wrapped inside their Stdio object in the libstd. We wrap it here as well because of this +// nuance on Windows: +// +// The standard handles of a process may be redirected by a call to SetStdHandle, in which +// case GetStdHandle returns the redirected handle. +// +// The MSDN also says this however: +// +// If the standard handles have been redirected, you can specify the CONIN$ value in a call +// to the CreateFile function to get a handle to a console's input buffer. Similarly, you +// can specify the CONOUT$ value to get a handle to a console's active screen buffer. +// +// TODO it might worth re-investigating the suitability of this type on Windows. #[derive(Debug, Clone)] #[allow(dead_code)] pub(crate) enum Stdio { @@ -75,11 +92,7 @@ impl Handle for Stdio { // lock for the duration of the scope let stdout = io::stdout(); let mut stdout = stdout.lock(); - let nwritten = if self.is_tty() { - SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)? - } else { - stdout.write_vectored(&iovs)? - }; + let nwritten = SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)?; stdout.flush()?; nwritten } From 4049c18ecdda7c9bb1054465650730bf02604b6b Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sat, 2 May 2020 09:10:51 +0200 Subject: [PATCH 10/16] Fix incorrect stdio ctors on Unix --- crates/wasi-common/src/sys/unix/stdio.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/wasi-common/src/sys/unix/stdio.rs b/crates/wasi-common/src/sys/unix/stdio.rs index 8e8dc847107a..63bae9c73e59 100644 --- a/crates/wasi-common/src/sys/unix/stdio.rs +++ b/crates/wasi-common/src/sys/unix/stdio.rs @@ -26,14 +26,14 @@ impl StdioExt for Stdio { Ok(Box::new(Self::In { rights })) } fn stdout() -> io::Result> { - let file = unsafe { File::from_raw_fd(io::stdin().as_raw_fd()) }; + let file = unsafe { File::from_raw_fd(io::stdout().as_raw_fd()) }; let file = ManuallyDrop::new(file); let rights = get_rights(&file)?; let rights = Cell::new(rights); Ok(Box::new(Self::Out { rights })) } fn stderr() -> io::Result> { - let file = unsafe { File::from_raw_fd(io::stdin().as_raw_fd()) }; + let file = unsafe { File::from_raw_fd(io::stderr().as_raw_fd()) }; let file = ManuallyDrop::new(file); let rights = get_rights(&file)?; let rights = Cell::new(rights); From f0ba02f93a4090d3fd6f4039e00a232cd5d15fc0 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sat, 2 May 2020 22:44:57 +0200 Subject: [PATCH 11/16] Split Stdio into three separate types: Stdin, Stdout, Stderr --- crates/wasi-common/src/ctx.rs | 16 +- crates/wasi-common/src/sys/mod.rs | 10 +- crates/wasi-common/src/sys/stdio.rs | 172 +++++++++++++------- crates/wasi-common/src/sys/unix/stdio.rs | 36 ++-- crates/wasi-common/src/sys/windows/poll.rs | 46 +++--- crates/wasi-common/src/sys/windows/stdio.rs | 36 ++-- 6 files changed, 208 insertions(+), 108 deletions(-) diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 3fb135e9959c..e099a8e7b7fd 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -3,7 +3,7 @@ use crate::fdpool::FdPool; use crate::handle::Handle; use crate::sys::osdir::OsDir; use crate::sys::osother::{OsOther, OsOtherExt}; -use crate::sys::stdio::{Stdio, StdioExt}; +use crate::sys::stdio::{Stderr, StderrExt, Stdin, StdinExt, Stdout, StdoutExt}; use crate::virtfs::{VirtualDir, VirtualDirEntry}; use crate::wasi::types; use crate::wasi::{Errno, Result}; @@ -170,27 +170,27 @@ impl WasiCtxBuilder { /// Inherit stdin from the host process. pub fn inherit_stdin(&mut self) -> &mut Self { - self.stdin = Some(PendingEntry::Thunk(Stdio::stdin)); + self.stdin = Some(PendingEntry::Thunk(Stdin::stdin)); self } /// Inherit stdout from the host process. pub fn inherit_stdout(&mut self) -> &mut Self { - self.stdout = Some(PendingEntry::Thunk(Stdio::stdout)); + self.stdout = Some(PendingEntry::Thunk(Stdout::stdout)); self } - /// Inherit stdout from the host process. + /// Inherit stderr from the host process. pub fn inherit_stderr(&mut self) -> &mut Self { - self.stderr = Some(PendingEntry::Thunk(Stdio::stderr)); + self.stderr = Some(PendingEntry::Thunk(Stderr::stderr)); self } /// Inherit the stdin, stdout, and stderr streams from the host process. pub fn inherit_stdio(&mut self) -> &mut Self { - self.stdin = Some(PendingEntry::Thunk(Stdio::stdin)); - self.stdout = Some(PendingEntry::Thunk(Stdio::stdout)); - self.stderr = Some(PendingEntry::Thunk(Stdio::stderr)); + self.stdin = Some(PendingEntry::Thunk(Stdin::stdin)); + self.stdout = Some(PendingEntry::Thunk(Stdout::stdout)); + self.stderr = Some(PendingEntry::Thunk(Stderr::stderr)); self } diff --git a/crates/wasi-common/src/sys/mod.rs b/crates/wasi-common/src/sys/mod.rs index 279a1002616e..bca99f146d08 100644 --- a/crates/wasi-common/src/sys/mod.rs +++ b/crates/wasi-common/src/sys/mod.rs @@ -33,7 +33,7 @@ use std::convert::TryFrom; use std::fs::File; use std::io; use std::mem::ManuallyDrop; -use stdio::Stdio; +use stdio::{Stderr, Stdin, Stdout}; use sys_impl::get_file_type; pub(crate) trait AsFile { @@ -46,8 +46,12 @@ impl AsFile for dyn Handle + 'static { file.as_file() } else if let Some(dir) = self.as_any().downcast_ref::() { dir.as_file() - } else if let Some(stdio) = self.as_any().downcast_ref::() { - stdio.as_file() + } else if let Some(stdin) = self.as_any().downcast_ref::() { + stdin.as_file() + } else if let Some(stdout) = self.as_any().downcast_ref::() { + stdout.as_file() + } else if let Some(stderr) = self.as_any().downcast_ref::() { + stderr.as_file() } else if let Some(other) = self.as_any().downcast_ref::() { other.as_file() } else { diff --git a/crates/wasi-common/src/sys/stdio.rs b/crates/wasi-common/src/sys/stdio.rs index 3cbba5ae464e..6aeed07dabcd 100644 --- a/crates/wasi-common/src/sys/stdio.rs +++ b/crates/wasi-common/src/sys/stdio.rs @@ -1,22 +1,4 @@ -use super::{fd, AsFile}; -use crate::handle::{Handle, HandleRights}; -use crate::sandboxed_tty_writer::SandboxedTTYWriter; -use crate::wasi::types::{self, Filetype}; -use crate::wasi::{Errno, Result}; -use std::any::Any; -use std::cell::Cell; -use std::io::{self, Read, Write}; - -pub(crate) trait StdioExt: Sized { - /// Create `Stdio` from `io::stdin`. - fn stdin() -> io::Result>; - /// Create `Stdio` from `io::stdout`. - fn stdout() -> io::Result>; - /// Create `Stdio` from `io::stderr`. - fn stderr() -> io::Result>; -} - -// The reason we have a separate Stdio type is to correctly facilitate redirects on Windows. +// The reason we have a separate Stdio wrappers is to correctly facilitate redirects on Windows. // To elaborate further, in POSIX, we can get a stdio handle by opening a specific fd {0,1,2}. // On Windows however, we need to issue a syscall that's separate from standard Windows "open" // to get a console handle, and this is GetStdHandle. This is exactly what Rust does and what @@ -33,15 +15,27 @@ pub(crate) trait StdioExt: Sized { // can specify the CONOUT$ value to get a handle to a console's active screen buffer. // // TODO it might worth re-investigating the suitability of this type on Windows. + +use super::{fd, AsFile}; +use crate::handle::{Handle, HandleRights}; +use crate::sandboxed_tty_writer::SandboxedTTYWriter; +use crate::wasi::types::{self, Filetype}; +use crate::wasi::Result; +use std::any::Any; +use std::cell::Cell; +use std::io::{self, Read, Write}; + +pub(crate) trait StdinExt: Sized { + /// Create `Stdin` from `io::stdin`. + fn stdin() -> io::Result>; +} + #[derive(Debug, Clone)] -#[allow(dead_code)] -pub(crate) enum Stdio { - In { rights: Cell }, - Out { rights: Cell }, - Err { rights: Cell }, +pub(crate) struct Stdin { + pub(crate) rights: Cell, } -impl Handle for Stdio { +impl Handle for Stdin { fn as_any(&self) -> &dyn Any { self } @@ -52,18 +46,10 @@ impl Handle for Stdio { Filetype::CharacterDevice } fn get_rights(&self) -> HandleRights { - match self { - Self::In { rights } => rights.get(), - Self::Out { rights } => rights.get(), - Self::Err { rights } => rights.get(), - } + self.rights.get() } fn set_rights(&self, new_rights: HandleRights) { - match self { - Self::In { rights } => rights.set(new_rights), - Self::Out { rights } => rights.set(new_rights), - Self::Err { rights } => rights.set(new_rights), - } + self.rights.set(new_rights) } // FdOps fn fdstat_get(&self) -> Result { @@ -79,29 +65,105 @@ impl Handle for Stdio { Ok(()) } fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result { - let nread = match self { - Self::In { .. } => io::stdin().read_vectored(iovs)?, - _ => return Err(Errno::Badf), - }; + let nread = io::stdin().read_vectored(iovs)?; Ok(nread) } +} + +pub(crate) trait StdoutExt: Sized { + /// Create `Stdout` from `io::stdout`. + fn stdout() -> io::Result>; +} + +#[derive(Debug, Clone)] +pub(crate) struct Stdout { + pub(crate) rights: Cell, +} + +impl Handle for Stdout { + fn as_any(&self) -> &dyn Any { + self + } + fn try_clone(&self) -> io::Result> { + Ok(Box::new(self.clone())) + } + fn get_file_type(&self) -> Filetype { + Filetype::CharacterDevice + } + fn get_rights(&self) -> HandleRights { + self.rights.get() + } + fn set_rights(&self, new_rights: HandleRights) { + self.rights.set(new_rights) + } + // FdOps + fn fdstat_get(&self) -> Result { + fd::fdstat_get(&*self.as_file()?) + } + fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { + if let Some(_) = fd::fdstat_set_flags(&*self.as_file()?, fdflags)? { + // OK, this means we should somehow update the underlying os handle, + // and we can't do that with `std::io::std{in, out, err}`, so we'll + // panic for now. + panic!("Tried updating Fdflags on Stdio handle by re-opening as file!"); + } + Ok(()) + } + fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { + // lock for the duration of the scope + let stdout = io::stdout(); + let mut stdout = stdout.lock(); + let nwritten = SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)?; + stdout.flush()?; + Ok(nwritten) + } +} + +pub(crate) trait StderrExt: Sized { + /// Create `Stderr` from `io::stderr`. + fn stderr() -> io::Result>; +} + +#[derive(Debug, Clone)] +pub(crate) struct Stderr { + pub(crate) rights: Cell, +} + +impl Handle for Stderr { + fn as_any(&self) -> &dyn Any { + self + } + fn try_clone(&self) -> io::Result> { + Ok(Box::new(self.clone())) + } + fn get_file_type(&self) -> Filetype { + Filetype::CharacterDevice + } + fn get_rights(&self) -> HandleRights { + self.rights.get() + } + fn set_rights(&self, new_rights: HandleRights) { + self.rights.set(new_rights) + } + // FdOps + fn fdstat_get(&self) -> Result { + fd::fdstat_get(&*self.as_file()?) + } + fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { + if let Some(_) = fd::fdstat_set_flags(&*self.as_file()?, fdflags)? { + // OK, this means we should somehow update the underlying os handle, + // and we can't do that with `std::io::std{in, out, err}`, so we'll + // panic for now. + panic!("Tried updating Fdflags on Stdio handle by re-opening as file!"); + } + Ok(()) + } fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { - let nwritten = match self { - Self::In { .. } => return Err(Errno::Badf), - Self::Out { .. } => { - // lock for the duration of the scope - let stdout = io::stdout(); - let mut stdout = stdout.lock(); - let nwritten = SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)?; - stdout.flush()?; - nwritten - } - // Always sanitize stderr, even if it's not directly connected to a tty, - // because stderr is meant for diagnostics rather than binary output, - // and may be redirected to a file which could end up being displayed - // on a tty later. - Self::Err { .. } => SandboxedTTYWriter::new(&mut io::stderr()).write_vectored(&iovs)?, - }; + // Always sanitize stderr, even if it's not directly connected to a tty, + // because stderr is meant for diagnostics rather than binary output, + // and may be redirected to a file which could end up being displayed + // on a tty later. + let nwritten = SandboxedTTYWriter::new(&mut io::stderr()).write_vectored(&iovs)?; Ok(nwritten) } } diff --git a/crates/wasi-common/src/sys/unix/stdio.rs b/crates/wasi-common/src/sys/unix/stdio.rs index 63bae9c73e59..caaa4f3686d0 100644 --- a/crates/wasi-common/src/sys/unix/stdio.rs +++ b/crates/wasi-common/src/sys/unix/stdio.rs @@ -1,5 +1,5 @@ use crate::handle::{Handle, HandleRights}; -use crate::sys::stdio::{Stdio, StdioExt}; +use crate::sys::stdio::{Stderr, StderrExt, Stdin, StdinExt, Stdout, StdoutExt}; use crate::wasi::{types, RightsExt}; use std::cell::Cell; use std::fs::File; @@ -7,37 +7,51 @@ use std::io; use std::mem::ManuallyDrop; use std::os::unix::prelude::{AsRawFd, FromRawFd, RawFd}; -impl AsRawFd for Stdio { +impl AsRawFd for Stdin { fn as_raw_fd(&self) -> RawFd { - match self { - Self::In { .. } => io::stdin().as_raw_fd(), - Self::Out { .. } => io::stdout().as_raw_fd(), - Self::Err { .. } => io::stderr().as_raw_fd(), - } + io::stdin().as_raw_fd() } } -impl StdioExt for Stdio { +impl AsRawFd for Stdout { + fn as_raw_fd(&self) -> RawFd { + io::stdout().as_raw_fd() + } +} + +impl AsRawFd for Stderr { + fn as_raw_fd(&self) -> RawFd { + io::stderr().as_raw_fd() + } +} + +impl StdinExt for Stdin { fn stdin() -> io::Result> { let file = unsafe { File::from_raw_fd(io::stdin().as_raw_fd()) }; let file = ManuallyDrop::new(file); let rights = get_rights(&file)?; let rights = Cell::new(rights); - Ok(Box::new(Self::In { rights })) + Ok(Box::new(Self { rights })) } +} + +impl StdoutExt for Stdout { fn stdout() -> io::Result> { let file = unsafe { File::from_raw_fd(io::stdout().as_raw_fd()) }; let file = ManuallyDrop::new(file); let rights = get_rights(&file)?; let rights = Cell::new(rights); - Ok(Box::new(Self::Out { rights })) + Ok(Box::new(Self { rights })) } +} + +impl StderrExt for Stderr { fn stderr() -> io::Result> { let file = unsafe { File::from_raw_fd(io::stderr().as_raw_fd()) }; let file = ManuallyDrop::new(file); let rights = get_rights(&file)?; let rights = Cell::new(rights); - Ok(Box::new(Self::Err { rights })) + Ok(Box::new(Self { rights })) } } diff --git a/crates/wasi-common/src/sys/windows/poll.rs b/crates/wasi-common/src/sys/windows/poll.rs index 9da8a54c9865..d49524a5d588 100644 --- a/crates/wasi-common/src/sys/windows/poll.rs +++ b/crates/wasi-common/src/sys/windows/poll.rs @@ -3,7 +3,7 @@ use crate::poll::{ClockEventData, FdEventData}; use crate::sys::osdir::OsDir; use crate::sys::osfile::OsFile; use crate::sys::osother::OsOther; -use crate::sys::stdio::Stdio; +use crate::sys::stdio::{Stderr, Stdin, Stdout}; use crate::sys::AsFile; use crate::wasi::{types, Errno, Result}; use lazy_static::lazy_static; @@ -145,13 +145,15 @@ fn handle_timeout_event(timeout_event: ClockEventData, events: &mut Vec) { let handle = &event.handle; - let size = if let Some(stdio) = handle.as_any().downcast_ref::() { - match stdio { - // We return the only universally correct lower bound, see the comment later in the function. - Stdio::In { .. } => Ok(1), - // On Unix, ioctl(FIONREAD) will return 0 for stdout/stderr. Emulate the same behavior on Windows. - Stdio::Out { .. } | Stdio::Err { .. } => Ok(0), - } + let size = if let Some(_) = handle.as_any().downcast_ref::() { + // We return the only universally correct lower bound, see the comment later in the function. + Ok(1) + } else if let Some(_) = handle.as_any().downcast_ref::() { + // On Unix, ioctl(FIONREAD) will return 0 for stdout. Emulate the same behavior on Windows. + Ok(0) + } else if let Some(_) = handle.as_any().downcast_ref::() { + // On Unix, ioctl(FIONREAD) will return 0 for stdout/stderr. Emulate the same behavior on Windows. + Ok(0) } else { if event.r#type == types::Eventtype::FdRead { handle @@ -211,18 +213,22 @@ pub(crate) fn oneoff( immediate_events.push(event); } else if let Some(_) = handle.as_any().downcast_ref::() { immediate_events.push(event); - } else if let Some(stdio) = handle.as_any().downcast_ref::() { - match stdio { - Stdio::In { .. } if event.r#type == types::Eventtype::FdRead => { - stdin_events.push(event) - } - // stdout/stderr are always considered ready to write because there seems to - // be no way of checking if a write to stdout would block. - // - // If stdin is polled for anything else then reading, then it is also - // considered immediately ready, following the behavior on Linux. - _ => immediate_events.push(event), - }; + } else if let Some(_) = handle.as_any().downcast_ref::() { + stdin_events.push(event); + } else if let Some(_) = handle.as_any().downcast_ref::() { + // stdout are always considered ready to write because there seems to + // be no way of checking if a write to stdout would block. + // + // If stdin is polled for anything else then reading, then it is also + // considered immediately ready, following the behavior on Linux. + immediate_events.push(event); + } else if let Some(_) = handle.as_any().downcast_ref::() { + // stderr are always considered ready to write because there seems to + // be no way of checking if a write to stdout would block. + // + // If stdin is polled for anything else then reading, then it is also + // considered immediately ready, following the behavior on Linux. + immediate_events.push(event); } else if let Some(other) = handle.as_any().downcast_ref::() { if other.get_file_type() == types::Filetype::SocketStream { // We map pipe to SocketStream diff --git a/crates/wasi-common/src/sys/windows/stdio.rs b/crates/wasi-common/src/sys/windows/stdio.rs index abc100746532..f6d33aadedb7 100644 --- a/crates/wasi-common/src/sys/windows/stdio.rs +++ b/crates/wasi-common/src/sys/windows/stdio.rs @@ -1,35 +1,49 @@ use crate::handle::{Handle, HandleRights}; -use crate::sys::stdio::{Stdio, StdioExt}; +use crate::sys::stdio::{Stderr, StderrExt, Stdin, StdinExt, Stdout, StdoutExt}; use crate::wasi::{types, RightsExt}; use std::cell::Cell; use std::io; use std::os::windows::prelude::{AsRawHandle, RawHandle}; -impl AsRawHandle for Stdio { +impl AsRawHandle for Stdin { fn as_raw_handle(&self) -> RawHandle { - match self { - Self::In { .. } => io::stdin().as_raw_handle(), - Self::Out { .. } => io::stdout().as_raw_handle(), - Self::Err { .. } => io::stderr().as_raw_handle(), - } + io::stdin().as_raw_handle() } } -impl StdioExt for Stdio { +impl AsRawHandle for Stdout { + fn as_raw_handle(&self) -> RawHandle { + io::stdout().as_raw_handle() + } +} + +impl AsRawHandle for Stderr { + fn as_raw_handle(&self) -> RawHandle { + io::stderr().as_raw_handle() + } +} + +impl StdinExt for Stdin { fn stdin() -> io::Result> { let rights = get_rights()?; let rights = Cell::new(rights); - Ok(Box::new(Self::In { rights })) + Ok(Box::new(Self { rights })) } +} + +impl StdoutExt for Stdout { fn stdout() -> io::Result> { let rights = get_rights()?; let rights = Cell::new(rights); - Ok(Box::new(Self::Out { rights })) + Ok(Box::new(Self { rights })) } +} + +impl StderrExt for Stderr { fn stderr() -> io::Result> { let rights = get_rights()?; let rights = Cell::new(rights); - Ok(Box::new(Self::Err { rights })) + Ok(Box::new(Self { rights })) } } From e5036242fc747598941d083903c4c4417b11769c Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sat, 2 May 2020 23:30:03 +0200 Subject: [PATCH 12/16] Rename PendingEntry::File to PendingEntry::OsHandle to avoid confusion --- crates/wasi-common/src/ctx.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index e099a8e7b7fd..b7bcb959d0ac 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -47,7 +47,7 @@ type WasiCtxBuilderResult = std::result::Result; enum PendingEntry { Thunk(fn() -> io::Result>), - File(File), + OsHandle(File), } impl std::fmt::Debug for PendingEntry { @@ -58,7 +58,7 @@ impl std::fmt::Debug for PendingEntry { "PendingEntry::Thunk({:p})", f as *const fn() -> io::Result> ), - Self::File(f) => write!(fmt, "PendingEntry::File({:?})", f), + Self::OsHandle(f) => write!(fmt, "PendingEntry::OsHandle({:?})", f), } } } @@ -234,19 +234,19 @@ impl WasiCtxBuilder { /// Provide a File to use as stdin pub fn stdin(&mut self, file: File) -> &mut Self { - self.stdin = Some(PendingEntry::File(file)); + self.stdin = Some(PendingEntry::OsHandle(file)); self } /// Provide a File to use as stdout pub fn stdout(&mut self, file: File) -> &mut Self { - self.stdout = Some(PendingEntry::File(file)); + self.stdout = Some(PendingEntry::OsHandle(file)); self } /// Provide a File to use as stderr pub fn stderr(&mut self, file: File) -> &mut Self { - self.stderr = Some(PendingEntry::File(file)); + self.stderr = Some(PendingEntry::OsHandle(file)); self } @@ -345,7 +345,7 @@ impl WasiCtxBuilder { .insert(entry) .ok_or(WasiCtxBuilderError::TooManyFilesOpen)? } - PendingEntry::File(f) => { + PendingEntry::OsHandle(f) => { let handle = OsOther::try_from(f)?; let handle = EntryHandle::new(handle); let entry = Entry::new(handle); From 6f0d437d0965e17c56bb0d5aae197d71543ca030 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 4 May 2020 08:49:07 +0200 Subject: [PATCH 13/16] 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. --- crates/wasi-common/src/sys/osdir.rs | 4 +- crates/wasi-common/src/sys/osfile.rs | 8 ++-- crates/wasi-common/src/sys/osother.rs | 8 ++-- crates/wasi-common/src/sys/unix/bsd/osdir.rs | 6 +-- crates/wasi-common/src/sys/unix/fd.rs | 4 +- .../wasi-common/src/sys/unix/linux/osdir.rs | 6 +-- crates/wasi-common/src/sys/unix/osdir.rs | 4 +- crates/wasi-common/src/sys/unix/osfile.rs | 4 +- crates/wasi-common/src/sys/unix/oshandle.rs | 47 ++++++------------- crates/wasi-common/src/sys/unix/osother.rs | 4 +- crates/wasi-common/src/sys/windows/fd.rs | 9 ++-- crates/wasi-common/src/sys/windows/osdir.rs | 8 ++-- crates/wasi-common/src/sys/windows/osfile.rs | 4 +- .../wasi-common/src/sys/windows/oshandle.rs | 12 ++--- crates/wasi-common/src/sys/windows/osother.rs | 4 +- 15 files changed, 58 insertions(+), 74 deletions(-) diff --git a/crates/wasi-common/src/sys/osdir.rs b/crates/wasi-common/src/sys/osdir.rs index 27cc845ea232..29a69e0c106c 100644 --- a/crates/wasi-common/src/sys/osdir.rs +++ b/crates/wasi-common/src/sys/osdir.rs @@ -1,4 +1,4 @@ -use super::sys_impl::oshandle::OsHandle; +use super::sys_impl::oshandle::RawOsHandle; use super::{fd, path, AsFile}; use crate::handle::{Handle, HandleRights}; use crate::wasi::{types, Errno, Result}; @@ -13,7 +13,7 @@ use std::ops::Deref; pub(crate) use super::sys_impl::osdir::OsDir; impl Deref for OsDir { - type Target = OsHandle; + type Target = RawOsHandle; fn deref(&self) -> &Self::Target { &self.handle diff --git a/crates/wasi-common/src/sys/osfile.rs b/crates/wasi-common/src/sys/osfile.rs index 5d1bfb21fcaa..a3491f5432d9 100644 --- a/crates/wasi-common/src/sys/osfile.rs +++ b/crates/wasi-common/src/sys/osfile.rs @@ -1,4 +1,4 @@ -use super::sys_impl::oshandle::OsHandle; +use super::sys_impl::oshandle::RawOsHandle; use super::{fd, AsFile}; use crate::handle::{Handle, HandleRights}; use crate::wasi::{types, Errno, Result}; @@ -11,18 +11,18 @@ use std::ops::Deref; #[derive(Debug)] pub(crate) struct OsFile { rights: Cell, - handle: OsHandle, + handle: RawOsHandle, } impl OsFile { - pub(super) fn new(rights: HandleRights, handle: OsHandle) -> Self { + pub(super) fn new(rights: HandleRights, handle: RawOsHandle) -> Self { let rights = Cell::new(rights); Self { rights, handle } } } impl Deref for OsFile { - type Target = OsHandle; + type Target = RawOsHandle; fn deref(&self) -> &Self::Target { &self.handle diff --git a/crates/wasi-common/src/sys/osother.rs b/crates/wasi-common/src/sys/osother.rs index bf5c32c51cfd..0edce93f48f2 100644 --- a/crates/wasi-common/src/sys/osother.rs +++ b/crates/wasi-common/src/sys/osother.rs @@ -1,4 +1,4 @@ -use super::sys_impl::oshandle::OsHandle; +use super::sys_impl::oshandle::RawOsHandle; use super::{fd, AsFile}; use crate::handle::{Handle, HandleRights}; use crate::sandboxed_tty_writer::SandboxedTTYWriter; @@ -19,11 +19,11 @@ pub(crate) trait OsOtherExt { pub(crate) struct OsOther { file_type: Filetype, rights: Cell, - handle: OsHandle, + handle: RawOsHandle, } impl OsOther { - pub(super) fn new(file_type: Filetype, rights: HandleRights, handle: OsHandle) -> Self { + pub(super) fn new(file_type: Filetype, rights: HandleRights, handle: RawOsHandle) -> Self { let rights = Cell::new(rights); Self { file_type, @@ -34,7 +34,7 @@ impl OsOther { } impl Deref for OsOther { - type Target = OsHandle; + type Target = RawOsHandle; fn deref(&self) -> &Self::Target { &self.handle diff --git a/crates/wasi-common/src/sys/unix/bsd/osdir.rs b/crates/wasi-common/src/sys/unix/bsd/osdir.rs index 03d5682a86d6..0726ad2ef6ed 100644 --- a/crates/wasi-common/src/sys/unix/bsd/osdir.rs +++ b/crates/wasi-common/src/sys/unix/bsd/osdir.rs @@ -1,5 +1,5 @@ use crate::handle::HandleRights; -use crate::sys::sys_impl::oshandle::OsHandle; +use crate::sys::sys_impl::oshandle::RawOsHandle; use crate::wasi::Result; use std::cell::{Cell, RefCell, RefMut}; use std::io; @@ -8,7 +8,7 @@ use yanix::dir::Dir; #[derive(Debug)] pub(crate) struct OsDir { pub(crate) rights: Cell, - pub(crate) handle: OsHandle, + pub(crate) handle: RawOsHandle, // When the client makes a `fd_readdir` syscall on this descriptor, // we will need to cache the `libc::DIR` pointer manually in order // to be able to seek on it later. While on Linux, this is handled @@ -23,7 +23,7 @@ pub(crate) struct OsDir { } impl OsDir { - pub(crate) fn new(rights: HandleRights, handle: OsHandle) -> io::Result { + pub(crate) fn new(rights: HandleRights, handle: RawOsHandle) -> io::Result { let rights = Cell::new(rights); // We need to duplicate the handle, because `opendir(3)`: // Upon successful return from fdopendir(), the file descriptor is under diff --git a/crates/wasi-common/src/sys/unix/fd.rs b/crates/wasi-common/src/sys/unix/fd.rs index 02d00c136dd1..83d717711c4a 100644 --- a/crates/wasi-common/src/sys/unix/fd.rs +++ b/crates/wasi-common/src/sys/unix/fd.rs @@ -1,4 +1,4 @@ -use super::oshandle::OsHandle; +use super::oshandle::RawOsHandle; use crate::sys::osdir::OsDir; use crate::sys::osfile::OsFile; use crate::wasi::{self, types, Result}; @@ -11,7 +11,7 @@ pub(crate) fn fdstat_get(fd: &File) -> Result { Ok(fdflags.into()) } -pub(crate) fn fdstat_set_flags(fd: &File, fdflags: types::Fdflags) -> Result> { +pub(crate) fn fdstat_set_flags(fd: &File, fdflags: types::Fdflags) -> Result> { unsafe { yanix::fcntl::set_status_flags(fd.as_raw_fd(), fdflags.into())? }; // We return None here to signal that the operation succeeded on the original // file descriptor and mutating the original WASI Descriptor is thus unnecessary. diff --git a/crates/wasi-common/src/sys/unix/linux/osdir.rs b/crates/wasi-common/src/sys/unix/linux/osdir.rs index 9f6ef2bc6d75..2fff99ec7bbf 100644 --- a/crates/wasi-common/src/sys/unix/linux/osdir.rs +++ b/crates/wasi-common/src/sys/unix/linux/osdir.rs @@ -1,5 +1,5 @@ use crate::handle::HandleRights; -use crate::sys::sys_impl::oshandle::OsHandle; +use crate::sys::sys_impl::oshandle::RawOsHandle; use crate::wasi::Result; use std::cell::Cell; use std::io; @@ -8,11 +8,11 @@ use yanix::dir::Dir; #[derive(Debug)] pub(crate) struct OsDir { pub(crate) rights: Cell, - pub(crate) handle: OsHandle, + pub(crate) handle: RawOsHandle, } impl OsDir { - pub(crate) fn new(rights: HandleRights, handle: OsHandle) -> io::Result { + pub(crate) fn new(rights: HandleRights, handle: RawOsHandle) -> io::Result { let rights = Cell::new(rights); Ok(Self { rights, handle }) } diff --git a/crates/wasi-common/src/sys/unix/osdir.rs b/crates/wasi-common/src/sys/unix/osdir.rs index e339c30b33f3..47b264208fa5 100644 --- a/crates/wasi-common/src/sys/unix/osdir.rs +++ b/crates/wasi-common/src/sys/unix/osdir.rs @@ -1,4 +1,4 @@ -use super::oshandle::OsHandle; +use super::oshandle::RawOsHandle; use crate::handle::HandleRights; use crate::wasi::{types, RightsExt}; use std::convert::TryFrom; @@ -17,7 +17,7 @@ impl TryFrom for OsDir { return Err(io::Error::from_raw_os_error(libc::EINVAL)); } let rights = get_rights(&file)?; - let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; + let handle = unsafe { RawOsHandle::from_raw_fd(file.into_raw_fd()) }; Self::new(rights, handle) } } diff --git a/crates/wasi-common/src/sys/unix/osfile.rs b/crates/wasi-common/src/sys/unix/osfile.rs index a3259b587d85..f4d3b6e85442 100644 --- a/crates/wasi-common/src/sys/unix/osfile.rs +++ b/crates/wasi-common/src/sys/unix/osfile.rs @@ -1,4 +1,4 @@ -use super::oshandle::OsHandle; +use super::oshandle::RawOsHandle; use crate::handle::HandleRights; use crate::sys::osfile::OsFile; use crate::wasi::{types, RightsExt}; @@ -16,7 +16,7 @@ impl TryFrom for OsFile { return Err(io::Error::from_raw_os_error(libc::EINVAL)); } let rights = get_rights(&file)?; - let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; + let handle = unsafe { RawOsHandle::from_raw_fd(file.into_raw_fd()) }; Ok(Self::new(rights, handle)) } } diff --git a/crates/wasi-common/src/sys/unix/oshandle.rs b/crates/wasi-common/src/sys/unix/oshandle.rs index 8419839f08c5..513e4b7787ae 100644 --- a/crates/wasi-common/src/sys/unix/oshandle.rs +++ b/crates/wasi-common/src/sys/unix/oshandle.rs @@ -1,59 +1,40 @@ -use crate::sys::AsFile; -use std::cell::Cell; use std::fs::File; use std::io; -use std::mem::ManuallyDrop; use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; #[derive(Debug)] -pub(crate) struct OsHandle(Cell); +pub(crate) struct RawOsHandle(File); -impl OsHandle { +impl RawOsHandle { /// Tries clone `self`. pub(crate) fn try_clone(&self) -> io::Result { - let fd = self.as_file()?.try_clone()?; - Ok(Self(Cell::new(fd.into_raw_fd()))) + let fd = self.0.try_clone()?; + Ok(unsafe { Self::from_raw_fd(fd.into_raw_fd()) }) } /// Consumes `other` taking the ownership of the underlying /// `RawFd` file descriptor. /// /// Note that the state of `Dir` stream pointer *will* not be carried /// across from `other` to `self`. - pub(crate) fn update_from(&self, other: Self) { - let new_fd = other.into_raw_fd(); - let old_fd = self.0.get(); - self.0.set(new_fd); - // We need to remember to close the old_fd. - unsafe { - File::from_raw_fd(old_fd); - } + pub(crate) fn update_from(&self, _other: Self) { + panic!("RawOsHandle::update_from should never be issued on Unix!") } } -impl Drop for OsHandle { - fn drop(&mut self) { - unsafe { - File::from_raw_fd(self.as_raw_fd()); - } - } -} - -impl AsRawFd for OsHandle { +impl AsRawFd for RawOsHandle { fn as_raw_fd(&self) -> RawFd { - self.0.get() + self.0.as_raw_fd() } } -impl FromRawFd for OsHandle { - unsafe fn from_raw_fd(fd: RawFd) -> Self { - Self(Cell::new(fd)) +impl IntoRawFd for RawOsHandle { + fn into_raw_fd(self) -> RawFd { + self.0.into_raw_fd() } } -impl IntoRawFd for OsHandle { - fn into_raw_fd(self) -> RawFd { - // We need to prevent dropping of self - let wrapped = ManuallyDrop::new(self); - wrapped.0.get() +impl FromRawFd for RawOsHandle { + unsafe fn from_raw_fd(raw: RawFd) -> Self { + Self(File::from_raw_fd(raw)) } } diff --git a/crates/wasi-common/src/sys/unix/osother.rs b/crates/wasi-common/src/sys/unix/osother.rs index 266f888405c3..14ee6e365cdd 100644 --- a/crates/wasi-common/src/sys/unix/osother.rs +++ b/crates/wasi-common/src/sys/unix/osother.rs @@ -1,5 +1,5 @@ use super::get_file_type; -use super::oshandle::OsHandle; +use super::oshandle::RawOsHandle; use crate::handle::{Handle, HandleRights}; use crate::sys::osother::{OsOther, OsOtherExt}; use crate::wasi::{types, RightsExt}; @@ -17,7 +17,7 @@ impl TryFrom for OsOther { return Err(io::Error::from_raw_os_error(libc::EINVAL)); } let rights = get_rights(&file, &file_type)?; - let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) }; + let handle = unsafe { RawOsHandle::from_raw_fd(file.into_raw_fd()) }; Ok(Self::new(file_type, rights, handle)) } } diff --git a/crates/wasi-common/src/sys/windows/fd.rs b/crates/wasi-common/src/sys/windows/fd.rs index 1f89ff75a838..78604a47194a 100644 --- a/crates/wasi-common/src/sys/windows/fd.rs +++ b/crates/wasi-common/src/sys/windows/fd.rs @@ -1,5 +1,5 @@ use super::file_serial_no; -use super::oshandle::OsHandle; +use super::oshandle::RawOsHandle; use crate::path; use crate::sys::osdir::OsDir; use crate::sys::osfile::OsFile; @@ -41,7 +41,10 @@ pub(crate) fn fdstat_get(file: &File) -> Result { // handle came from `CreateFile`, but the Rust's libstd will use `GetStdHandle` // rather than `CreateFile`. Relevant discussion can be found in: // https://github.com/rust-lang/rust/issues/40490 -pub(crate) fn fdstat_set_flags(file: &File, fdflags: types::Fdflags) -> Result> { +pub(crate) fn fdstat_set_flags( + file: &File, + fdflags: types::Fdflags, +) -> Result> { let handle = file.as_raw_handle(); let access_mode = winx::file::query_access_information(handle)?; let new_access_mode = file_access_mode_from_fdflags( @@ -51,7 +54,7 @@ pub(crate) fn fdstat_set_flags(file: &File, fdflags: types::Fdflags) -> Result, - pub(crate) handle: OsHandle, + pub(crate) handle: RawOsHandle, } impl OsDir { - pub(crate) fn new(rights: HandleRights, handle: OsHandle) -> io::Result { + pub(crate) fn new(rights: HandleRights, handle: RawOsHandle) -> io::Result { let rights = Cell::new(rights); Ok(Self { rights, handle }) } @@ -29,7 +29,7 @@ impl TryFrom for OsDir { return Err(io::Error::from_raw_os_error(libc::EINVAL)); } let rights = get_rights(&file)?; - let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; + let handle = unsafe { RawOsHandle::from_raw_handle(file.into_raw_handle()) }; Self::new(rights, handle) } } diff --git a/crates/wasi-common/src/sys/windows/osfile.rs b/crates/wasi-common/src/sys/windows/osfile.rs index 77f0fb3d16b2..0ef24c0b3040 100644 --- a/crates/wasi-common/src/sys/windows/osfile.rs +++ b/crates/wasi-common/src/sys/windows/osfile.rs @@ -1,4 +1,4 @@ -use super::oshandle::OsHandle; +use super::oshandle::RawOsHandle; use crate::handle::HandleRights; use crate::sys::osfile::OsFile; use crate::wasi::{types, RightsExt}; @@ -16,7 +16,7 @@ impl TryFrom for OsFile { return Err(io::Error::from_raw_os_error(libc::EINVAL)); } let rights = get_rights(&file)?; - let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; + let handle = unsafe { RawOsHandle::from_raw_handle(file.into_raw_handle()) }; Ok(Self::new(rights, handle)) } } diff --git a/crates/wasi-common/src/sys/windows/oshandle.rs b/crates/wasi-common/src/sys/windows/oshandle.rs index b46ce37cdfea..89884d03a8d9 100644 --- a/crates/wasi-common/src/sys/windows/oshandle.rs +++ b/crates/wasi-common/src/sys/windows/oshandle.rs @@ -6,9 +6,9 @@ use std::mem::ManuallyDrop; use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; #[derive(Debug)] -pub(crate) struct OsHandle(Cell); +pub(crate) struct RawOsHandle(Cell); -impl OsHandle { +impl RawOsHandle { /// Tries cloning `self`. pub(crate) fn try_clone(&self) -> io::Result { let handle = self.as_file()?.try_clone()?; @@ -27,7 +27,7 @@ impl OsHandle { } } -impl Drop for OsHandle { +impl Drop for RawOsHandle { fn drop(&mut self) { unsafe { File::from_raw_handle(self.as_raw_handle()); @@ -35,19 +35,19 @@ impl Drop for OsHandle { } } -impl AsRawHandle for OsHandle { +impl AsRawHandle for RawOsHandle { fn as_raw_handle(&self) -> RawHandle { self.0.get() } } -impl FromRawHandle for OsHandle { +impl FromRawHandle for RawOsHandle { unsafe fn from_raw_handle(handle: RawHandle) -> Self { Self(Cell::new(handle)) } } -impl IntoRawHandle for OsHandle { +impl IntoRawHandle for RawOsHandle { fn into_raw_handle(self) -> RawHandle { // We need to prevent dropping of the OsFile let wrapped = ManuallyDrop::new(self); diff --git a/crates/wasi-common/src/sys/windows/osother.rs b/crates/wasi-common/src/sys/windows/osother.rs index 3ec5fd4f84be..5f907980fd87 100644 --- a/crates/wasi-common/src/sys/windows/osother.rs +++ b/crates/wasi-common/src/sys/windows/osother.rs @@ -1,5 +1,5 @@ use super::get_file_type; -use super::oshandle::OsHandle; +use super::oshandle::RawOsHandle; use crate::handle::{Handle, HandleRights}; use crate::sys::osother::{OsOther, OsOtherExt}; use crate::wasi::{types, RightsExt}; @@ -17,7 +17,7 @@ impl TryFrom for OsOther { return Err(io::Error::from_raw_os_error(libc::EINVAL)); } let rights = get_rights(&file_type)?; - let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) }; + let handle = unsafe { RawOsHandle::from_raw_handle(file.into_raw_handle()) }; Ok(Self::new(file_type, rights, handle)) } } From 40a9e08ec2f108f6289f068049d968e9281c4958 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 4 May 2020 09:14:52 +0200 Subject: [PATCH 14/16] Add docs explaining what OsOther is --- crates/wasi-common/src/sys/osother.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/wasi-common/src/sys/osother.rs b/crates/wasi-common/src/sys/osother.rs index 0edce93f48f2..de30a4f2dd0a 100644 --- a/crates/wasi-common/src/sys/osother.rs +++ b/crates/wasi-common/src/sys/osother.rs @@ -15,6 +15,11 @@ pub(crate) trait OsOtherExt { fn from_null() -> io::Result>; } +/// `OsOther` is something of a catch-all for everything not covered with the specific handle +/// types (`OsFile`, `OsDir`, `Stdio`). It currently encapsulates handles such as OS pipes, +/// sockets, streams, etc. As such, when redirecting stdio within `WasiCtxBuilder`, the redirected +/// pipe should be encapsulated within this instance _and not_ `OsFile` which represents a regular +/// OS file. #[derive(Debug)] pub(crate) struct OsOther { file_type: Filetype, From 991ce43ea829472c9dfc598cb936a9f983cb2e92 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 4 May 2020 23:41:54 +0200 Subject: [PATCH 15/16] Allow for stdio to be non-character-device (e.g., piped) --- crates/wasi-common/src/sys/stdio.rs | 15 ++++-- crates/wasi-common/src/sys/unix/mod.rs | 49 ++++++++++++++++++- crates/wasi-common/src/sys/unix/osother.rs | 47 ++---------------- crates/wasi-common/src/sys/unix/stdio.rs | 34 +++++-------- crates/wasi-common/src/sys/windows/mod.rs | 33 ++++++++++++- crates/wasi-common/src/sys/windows/osother.rs | 27 ++-------- crates/wasi-common/src/sys/windows/stdio.rs | 34 +++++++------ 7 files changed, 128 insertions(+), 111 deletions(-) diff --git a/crates/wasi-common/src/sys/stdio.rs b/crates/wasi-common/src/sys/stdio.rs index 6aeed07dabcd..bbd2339ed0ce 100644 --- a/crates/wasi-common/src/sys/stdio.rs +++ b/crates/wasi-common/src/sys/stdio.rs @@ -32,6 +32,7 @@ pub(crate) trait StdinExt: Sized { #[derive(Debug, Clone)] pub(crate) struct Stdin { + pub(crate) file_type: Filetype, pub(crate) rights: Cell, } @@ -43,7 +44,7 @@ impl Handle for Stdin { Ok(Box::new(self.clone())) } fn get_file_type(&self) -> Filetype { - Filetype::CharacterDevice + self.file_type } fn get_rights(&self) -> HandleRights { self.rights.get() @@ -77,6 +78,7 @@ pub(crate) trait StdoutExt: Sized { #[derive(Debug, Clone)] pub(crate) struct Stdout { + pub(crate) file_type: Filetype, pub(crate) rights: Cell, } @@ -88,7 +90,7 @@ impl Handle for Stdout { Ok(Box::new(self.clone())) } fn get_file_type(&self) -> Filetype { - Filetype::CharacterDevice + self.file_type } fn get_rights(&self) -> HandleRights { self.rights.get() @@ -113,7 +115,11 @@ impl Handle for Stdout { // lock for the duration of the scope let stdout = io::stdout(); let mut stdout = stdout.lock(); - let nwritten = SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)?; + let nwritten = if self.is_tty() { + SandboxedTTYWriter::new(&mut stdout).write_vectored(&iovs)? + } else { + stdout.write_vectored(iovs)? + }; stdout.flush()?; Ok(nwritten) } @@ -126,6 +132,7 @@ pub(crate) trait StderrExt: Sized { #[derive(Debug, Clone)] pub(crate) struct Stderr { + pub(crate) file_type: Filetype, pub(crate) rights: Cell, } @@ -137,7 +144,7 @@ impl Handle for Stderr { Ok(Box::new(self.clone())) } fn get_file_type(&self) -> Filetype { - Filetype::CharacterDevice + self.file_type } fn get_rights(&self) -> HandleRights { self.rights.get() diff --git a/crates/wasi-common/src/sys/unix/mod.rs b/crates/wasi-common/src/sys/unix/mod.rs index c1fac592b0e4..e71635db0423 100644 --- a/crates/wasi-common/src/sys/unix/mod.rs +++ b/crates/wasi-common/src/sys/unix/mod.rs @@ -26,8 +26,9 @@ cfg_if::cfg_if! { } } +use crate::handle::HandleRights; use crate::sys::AsFile; -use crate::wasi::{types, Errno, Result}; +use crate::wasi::{types, Errno, Result, RightsExt}; use std::convert::{TryFrom, TryInto}; use std::fs::File; use std::io; @@ -78,6 +79,52 @@ pub(super) fn get_file_type(file: &File) -> io::Result { Ok(file_type) } +pub(super) fn get_rights(file: &File, file_type: &types::Filetype) -> io::Result { + use yanix::{fcntl, file::OFlag}; + let (base, inheriting) = match file_type { + types::Filetype::BlockDevice => ( + types::Rights::block_device_base(), + types::Rights::block_device_inheriting(), + ), + types::Filetype::CharacterDevice => { + use yanix::file::isatty; + if unsafe { isatty(file.as_raw_fd())? } { + (types::Rights::tty_base(), types::Rights::tty_base()) + } else { + ( + types::Rights::character_device_base(), + types::Rights::character_device_inheriting(), + ) + } + } + types::Filetype::SocketDgram | types::Filetype::SocketStream => ( + types::Rights::socket_base(), + types::Rights::socket_inheriting(), + ), + types::Filetype::SymbolicLink | types::Filetype::Unknown => ( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ), + types::Filetype::Directory => ( + types::Rights::directory_base(), + types::Rights::directory_inheriting(), + ), + types::Filetype::RegularFile => ( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ), + }; + let mut rights = HandleRights::new(base, inheriting); + let flags = unsafe { fcntl::get_status_flags(file.as_raw_fd())? }; + let accmode = flags & OFlag::ACCMODE; + if accmode == OFlag::RDONLY { + rights.base &= !types::Rights::FD_WRITE; + } else if accmode == OFlag::WRONLY { + rights.base &= !types::Rights::FD_READ; + } + Ok(rights) +} + pub fn preopen_dir>(path: P) -> io::Result { File::open(path) } diff --git a/crates/wasi-common/src/sys/unix/osother.rs b/crates/wasi-common/src/sys/unix/osother.rs index 14ee6e365cdd..260c47db8fec 100644 --- a/crates/wasi-common/src/sys/unix/osother.rs +++ b/crates/wasi-common/src/sys/unix/osother.rs @@ -1,12 +1,12 @@ -use super::get_file_type; use super::oshandle::RawOsHandle; -use crate::handle::{Handle, HandleRights}; +use super::{get_file_type, get_rights}; +use crate::handle::Handle; use crate::sys::osother::{OsOther, OsOtherExt}; -use crate::wasi::{types, RightsExt}; +use crate::wasi::types; use std::convert::TryFrom; use std::fs::{File, OpenOptions}; use std::io; -use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd}; +use std::os::unix::prelude::{FromRawFd, IntoRawFd}; impl TryFrom for OsOther { type Error = io::Error; @@ -22,45 +22,6 @@ impl TryFrom for OsOther { } } -fn get_rights(file: &File, file_type: &types::Filetype) -> io::Result { - use yanix::{fcntl, file::OFlag}; - let (base, inheriting) = match file_type { - types::Filetype::BlockDevice => ( - types::Rights::block_device_base(), - types::Rights::block_device_inheriting(), - ), - types::Filetype::CharacterDevice => { - use yanix::file::isatty; - if unsafe { isatty(file.as_raw_fd())? } { - (types::Rights::tty_base(), types::Rights::tty_base()) - } else { - ( - types::Rights::character_device_base(), - types::Rights::character_device_inheriting(), - ) - } - } - types::Filetype::SocketDgram | types::Filetype::SocketStream => ( - types::Rights::socket_base(), - types::Rights::socket_inheriting(), - ), - types::Filetype::SymbolicLink | types::Filetype::Unknown => ( - types::Rights::regular_file_base(), - types::Rights::regular_file_inheriting(), - ), - _ => unreachable!("these should have been handled already!"), - }; - let mut rights = HandleRights::new(base, inheriting); - let flags = unsafe { fcntl::get_status_flags(file.as_raw_fd())? }; - let accmode = flags & OFlag::ACCMODE; - if accmode == OFlag::RDONLY { - rights.base &= !types::Rights::FD_WRITE; - } else if accmode == OFlag::WRONLY { - rights.base &= !types::Rights::FD_READ; - } - Ok(rights) -} - impl OsOtherExt for OsOther { fn from_null() -> io::Result> { let file = OpenOptions::new() diff --git a/crates/wasi-common/src/sys/unix/stdio.rs b/crates/wasi-common/src/sys/unix/stdio.rs index caaa4f3686d0..5a38a2992b1c 100644 --- a/crates/wasi-common/src/sys/unix/stdio.rs +++ b/crates/wasi-common/src/sys/unix/stdio.rs @@ -1,6 +1,6 @@ -use crate::handle::{Handle, HandleRights}; +use super::{get_file_type, get_rights}; +use crate::handle::Handle; use crate::sys::stdio::{Stderr, StderrExt, Stdin, StdinExt, Stdout, StdoutExt}; -use crate::wasi::{types, RightsExt}; use std::cell::Cell; use std::fs::File; use std::io; @@ -29,9 +29,10 @@ impl StdinExt for Stdin { fn stdin() -> io::Result> { let file = unsafe { File::from_raw_fd(io::stdin().as_raw_fd()) }; let file = ManuallyDrop::new(file); - let rights = get_rights(&file)?; + let file_type = get_file_type(&file)?; + let rights = get_rights(&file, &file_type)?; let rights = Cell::new(rights); - Ok(Box::new(Self { rights })) + Ok(Box::new(Self { file_type, rights })) } } @@ -39,9 +40,10 @@ impl StdoutExt for Stdout { fn stdout() -> io::Result> { let file = unsafe { File::from_raw_fd(io::stdout().as_raw_fd()) }; let file = ManuallyDrop::new(file); - let rights = get_rights(&file)?; + let file_type = get_file_type(&file)?; + let rights = get_rights(&file, &file_type)?; let rights = Cell::new(rights); - Ok(Box::new(Self { rights })) + Ok(Box::new(Self { file_type, rights })) } } @@ -49,23 +51,9 @@ impl StderrExt for Stderr { fn stderr() -> io::Result> { let file = unsafe { File::from_raw_fd(io::stderr().as_raw_fd()) }; let file = ManuallyDrop::new(file); - let rights = get_rights(&file)?; + let file_type = get_file_type(&file)?; + let rights = get_rights(&file, &file_type)?; let rights = Cell::new(rights); - Ok(Box::new(Self { rights })) + Ok(Box::new(Self { file_type, rights })) } } - -fn get_rights(file: &File) -> io::Result { - use yanix::file::isatty; - let (base, inheriting) = { - if unsafe { isatty(file.as_raw_fd())? } { - (types::Rights::tty_base(), types::Rights::tty_base()) - } else { - ( - types::Rights::character_device_base(), - types::Rights::character_device_inheriting(), - ) - } - }; - Ok(HandleRights::new(base, inheriting)) -} diff --git a/crates/wasi-common/src/sys/windows/mod.rs b/crates/wasi-common/src/sys/windows/mod.rs index b2a92ccb5260..109385f15adf 100644 --- a/crates/wasi-common/src/sys/windows/mod.rs +++ b/crates/wasi-common/src/sys/windows/mod.rs @@ -8,8 +8,9 @@ pub(crate) mod path; pub(crate) mod poll; pub(crate) mod stdio; +use crate::handle::HandleRights; use crate::sys::AsFile; -use crate::wasi::{types, Errno, Result}; +use crate::wasi::{types, Errno, Result, RightsExt}; use std::convert::{TryFrom, TryInto}; use std::fs::File; use std::mem::ManuallyDrop; @@ -27,7 +28,7 @@ impl AsFile for T { } } -pub(crate) fn get_file_type(file: &File) -> io::Result { +pub(super) fn get_file_type(file: &File) -> io::Result { let file_type = unsafe { winx::file::get_file_type(file.as_raw_handle())? }; let file_type = if file_type.is_char() { // character file: LPT device or console @@ -53,6 +54,34 @@ pub(crate) fn get_file_type(file: &File) -> io::Result { Ok(file_type) } +pub(super) fn get_rights(file_type: &types::Filetype) -> io::Result { + let (base, inheriting) = match file_type { + types::Filetype::BlockDevice => ( + types::Rights::block_device_base(), + types::Rights::block_device_inheriting(), + ), + types::Filetype::CharacterDevice => (types::Rights::tty_base(), types::Rights::tty_base()), + types::Filetype::SocketDgram | types::Filetype::SocketStream => ( + types::Rights::socket_base(), + types::Rights::socket_inheriting(), + ), + types::Filetype::SymbolicLink | types::Filetype::Unknown => ( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ), + types::Filetype::Directory => ( + types::Rights::directory_base(), + types::Rights::directory_inheriting(), + ), + types::Filetype::RegularFile => ( + types::Rights::regular_file_base(), + types::Rights::regular_file_inheriting(), + ), + }; + let rights = HandleRights::new(base, inheriting); + Ok(rights) +} + pub fn preopen_dir>(path: P) -> io::Result { use std::fs::OpenOptions; use std::os::windows::fs::OpenOptionsExt; diff --git a/crates/wasi-common/src/sys/windows/osother.rs b/crates/wasi-common/src/sys/windows/osother.rs index 5f907980fd87..47db73217d90 100644 --- a/crates/wasi-common/src/sys/windows/osother.rs +++ b/crates/wasi-common/src/sys/windows/osother.rs @@ -1,8 +1,8 @@ -use super::get_file_type; use super::oshandle::RawOsHandle; -use crate::handle::{Handle, HandleRights}; +use super::{get_file_type, get_rights}; +use crate::handle::Handle; use crate::sys::osother::{OsOther, OsOtherExt}; -use crate::wasi::{types, RightsExt}; +use crate::wasi::types; use std::convert::TryFrom; use std::fs::{File, OpenOptions}; use std::io; @@ -22,27 +22,6 @@ impl TryFrom for OsOther { } } -fn get_rights(file_type: &types::Filetype) -> io::Result { - let (base, inheriting) = match file_type { - types::Filetype::BlockDevice => ( - types::Rights::block_device_base(), - types::Rights::block_device_inheriting(), - ), - types::Filetype::CharacterDevice => (types::Rights::tty_base(), types::Rights::tty_base()), - types::Filetype::SocketDgram | types::Filetype::SocketStream => ( - types::Rights::socket_base(), - types::Rights::socket_inheriting(), - ), - types::Filetype::SymbolicLink | types::Filetype::Unknown => ( - types::Rights::regular_file_base(), - types::Rights::regular_file_inheriting(), - ), - _ => unreachable!("these should have been handled already!"), - }; - let rights = HandleRights::new(base, inheriting); - Ok(rights) -} - impl OsOtherExt for OsOther { fn from_null() -> io::Result> { let file = OpenOptions::new().read(true).write(true).open("NUL")?; diff --git a/crates/wasi-common/src/sys/windows/stdio.rs b/crates/wasi-common/src/sys/windows/stdio.rs index f6d33aadedb7..25140db90a9a 100644 --- a/crates/wasi-common/src/sys/windows/stdio.rs +++ b/crates/wasi-common/src/sys/windows/stdio.rs @@ -1,9 +1,11 @@ -use crate::handle::{Handle, HandleRights}; +use super::{get_file_type, get_rights}; +use crate::handle::Handle; use crate::sys::stdio::{Stderr, StderrExt, Stdin, StdinExt, Stdout, StdoutExt}; -use crate::wasi::{types, RightsExt}; use std::cell::Cell; +use std::fs::File; use std::io; -use std::os::windows::prelude::{AsRawHandle, RawHandle}; +use std::mem::ManuallyDrop; +use std::os::windows::prelude::{AsRawHandle, FromRawHandle, RawHandle}; impl AsRawHandle for Stdin { fn as_raw_handle(&self) -> RawHandle { @@ -25,29 +27,33 @@ impl AsRawHandle for Stderr { impl StdinExt for Stdin { fn stdin() -> io::Result> { - let rights = get_rights()?; + let file = unsafe { File::from_raw_handle(io::stdin().as_raw_handle()) }; + let file = ManuallyDrop::new(file); + let file_type = get_file_type(&file)?; + let rights = get_rights(&file_type)?; let rights = Cell::new(rights); - Ok(Box::new(Self { rights })) + Ok(Box::new(Self { file_type, rights })) } } impl StdoutExt for Stdout { fn stdout() -> io::Result> { - let rights = get_rights()?; + let file = unsafe { File::from_raw_handle(io::stdin().as_raw_handle()) }; + let file = ManuallyDrop::new(file); + let file_type = get_file_type(&file)?; + let rights = get_rights(&file_type)?; let rights = Cell::new(rights); - Ok(Box::new(Self { rights })) + Ok(Box::new(Self { file_type, rights })) } } impl StderrExt for Stderr { fn stderr() -> io::Result> { - let rights = get_rights()?; + let file = unsafe { File::from_raw_handle(io::stdin().as_raw_handle()) }; + let file = ManuallyDrop::new(file); + let file_type = get_file_type(&file)?; + let rights = get_rights(&file_type)?; let rights = Cell::new(rights); - Ok(Box::new(Self { rights })) + Ok(Box::new(Self { file_type, rights })) } } - -fn get_rights() -> io::Result { - let rights = HandleRights::new(types::Rights::tty_base(), types::Rights::tty_base()); - Ok(rights) -} From 36f07cff880254f257b12124d6c9790a941a2a4b Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Thu, 7 May 2020 23:40:54 +0200 Subject: [PATCH 16/16] Return error on bad preopen rather than panic --- crates/wasi-common/src/ctx.rs | 65 ++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index b7bcb959d0ac..895c96ad6f17 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -35,9 +35,9 @@ pub enum WasiCtxBuilderError { /// Provided sequence of bytes contained an unexpected NUL byte. #[error("provided sequence contained an unexpected NUL byte")] UnexpectedNul(#[from] ffi::NulError), - /// Provided `File` is not a directory. - #[error("preopened directory path {} is not a directory", .0.display())] - NotADirectory(PathBuf), + /// The root of a VirtualDirEntry tree must be a VirtualDirEntry::Directory. + #[error("the root of a VirtualDirEntry tree at {} must be a VirtualDirEntry::Directory", .0.display())] + VirtualDirEntryRootNotADirectory(PathBuf), /// `WasiCtx` has too many opened files. #[error("context object has too many opened files")] TooManyFilesOpen, @@ -108,12 +108,27 @@ impl PendingCString { } } +struct PendingPreopen(Box WasiCtxBuilderResult>>); + +impl PendingPreopen { + fn new(f: F) -> Self + where + F: FnOnce() -> WasiCtxBuilderResult> + 'static, + { + Self(Box::new(f)) + } + + fn into(self) -> WasiCtxBuilderResult> { + self.0() + } +} + /// A builder allowing customizable construction of `WasiCtx` instances. pub struct WasiCtxBuilder { stdin: Option, stdout: Option, stderr: Option, - preopens: Option)>>, + preopens: Option>, args: Option>, env: Option>, } @@ -252,10 +267,14 @@ impl WasiCtxBuilder { /// Add a preopened directory. pub fn preopened_dir>(&mut self, dir: File, guest_path: P) -> &mut Self { - self.preopens.as_mut().unwrap().push(( - guest_path.as_ref().to_owned(), - Box::new(OsDir::try_from(dir).expect("valid OsDir handle")), - )); + let preopen = PendingPreopen::new(move || { + let dir = OsDir::try_from(dir).map_err(WasiCtxBuilderError::from)?; + Ok(Box::new(dir)) + }); + self.preopens + .as_mut() + .unwrap() + .push((guest_path.as_ref().to_owned(), preopen)); self } @@ -280,18 +299,22 @@ impl WasiCtxBuilder { } } - let dir = if let VirtualDirEntry::Directory(entries) = dir { - let mut dir = VirtualDir::new(true); - populate_directory(entries, &mut dir); - Box::new(dir) - } else { - panic!("the root of a VirtualDirEntry tree must be a VirtualDirEntry::Directory"); - }; - + let guest_path_owned = guest_path.as_ref().to_owned(); + let preopen = PendingPreopen::new(move || { + if let VirtualDirEntry::Directory(entries) = dir { + let mut dir = VirtualDir::new(true); + populate_directory(entries, &mut dir); + Ok(Box::new(dir)) + } else { + Err(WasiCtxBuilderError::VirtualDirEntryRootNotADirectory( + guest_path_owned, + )) + } + }); self.preopens .as_mut() .unwrap() - .push((guest_path.as_ref().to_owned(), dir)); + .push((guest_path.as_ref().to_owned(), preopen)); self } @@ -357,12 +380,8 @@ impl WasiCtxBuilder { log::debug!("WasiCtx inserted at {:?}", fd); } // Then add the preopen entries. - for (guest_path, dir) in self.preopens.take().unwrap() { - if !dir.is_directory() { - return Err(WasiCtxBuilderError::NotADirectory(guest_path)); - } - - let handle = EntryHandle::from(dir); + for (guest_path, preopen) in self.preopens.take().unwrap() { + let handle = EntryHandle::from(preopen.into()?); let mut entry = Entry::new(handle); entry.preopen_path = Some(guest_path); let fd = entries