diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index d7e5ae5fca..cdf575b446 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -5,11 +5,12 @@ use crate::virtfs::{VirtualDir, VirtualDirEntry}; use crate::wasi::types; use crate::wasi::{Errno, Result}; use std::borrow::Borrow; -use std::cell::{Ref, RefCell, RefMut}; +use std::cell::RefCell; use std::collections::HashMap; use std::ffi::{self, CString, OsString}; use std::fs::File; use std::path::{Path, PathBuf}; +use std::rc::Rc; use std::{env, io, string}; /// Possible errors when `WasiCtxBuilder` fails building @@ -381,7 +382,7 @@ impl WasiCtxBuilder { #[derive(Debug)] struct EntryTable { fd_pool: FdPool, - entries: HashMap, + entries: HashMap>, } impl EntryTable { @@ -398,23 +399,19 @@ impl EntryTable { fn insert(&mut self, entry: Entry) -> Option { let fd = self.fd_pool.allocate()?; - self.entries.insert(fd, entry); + self.entries.insert(fd, Rc::new(entry)); Some(fd) } - fn insert_at(&mut self, fd: &types::Fd, entry: Entry) { + fn insert_at(&mut self, fd: &types::Fd, entry: Rc) { self.entries.insert(*fd, entry); } - fn get(&self, fd: &types::Fd) -> Option<&Entry> { - self.entries.get(fd) + fn get(&self, fd: &types::Fd) -> Option> { + self.entries.get(fd).map(Rc::clone) } - fn get_mut(&mut self, fd: &types::Fd) -> Option<&mut Entry> { - self.entries.get_mut(fd) - } - - fn remove(&mut self, fd: types::Fd) -> Option { + fn remove(&mut self, fd: types::Fd) -> Option> { let entry = self.entries.remove(&fd)?; self.fd_pool.deallocate(fd); Some(entry) @@ -450,24 +447,11 @@ impl WasiCtx { } /// Get an immutable `Entry` corresponding to the specified raw WASI `fd`. - pub(crate) fn get_entry(&self, fd: types::Fd) -> Result> { - if !self.contains_entry(fd) { - return Err(Errno::Badf); + pub(crate) fn get_entry(&self, fd: types::Fd) -> Result> { + match self.entries.borrow().get(&fd) { + Some(entry) => Ok(entry), + None => Err(Errno::Badf), } - Ok(Ref::map(self.entries.borrow(), |entries| { - entries.get(&fd).unwrap() - })) - } - - /// Get a mutable `Entry` corresponding to the specified raw WASI `fd`. - // TODO This runs the risk of a potential difficult-to-predict panic down-the-line. - pub(crate) fn get_entry_mut(&self, fd: types::Fd) -> Result> { - if !self.contains_entry(fd) { - return Err(Errno::Badf); - } - Ok(RefMut::map(self.entries.borrow_mut(), |entries| { - entries.get_mut(&fd).unwrap() - })) } /// Insert the specified `Entry` into the `WasiCtx` object. @@ -480,12 +464,12 @@ impl WasiCtx { /// Insert the specified `Entry` with the specified raw WASI `fd` key into the `WasiCtx` /// object. - pub(crate) fn insert_entry_at(&self, fd: types::Fd, entry: Entry) { + pub(crate) fn insert_entry_at(&self, fd: types::Fd, entry: Rc) { self.entries.borrow_mut().insert_at(&fd, entry) } /// Remove `Entry` corresponding to the specified raw WASI `fd` from the `WasiCtx` object. - pub(crate) fn remove_entry(&self, fd: types::Fd) -> Result { + pub(crate) fn remove_entry(&self, fd: types::Fd) -> Result> { self.entries.borrow_mut().remove(fd).ok_or(Errno::Badf) } } diff --git a/crates/wasi-common/src/entry.rs b/crates/wasi-common/src/entry.rs index 8bd5072ce3..c96beea172 100644 --- a/crates/wasi-common/src/entry.rs +++ b/crates/wasi-common/src/entry.rs @@ -3,10 +3,12 @@ use crate::sys::entry::{descriptor_as_oshandle, determine_type_and_access_rights use crate::virtfs::VirtualFile; use crate::wasi::types::{Filetype, Rights}; use crate::wasi::{Errno, Result}; +use std::cell::{Cell, RefCell}; use std::marker::PhantomData; use std::mem::ManuallyDrop; use std::ops::Deref; use std::path::PathBuf; +use std::rc::Rc; use std::{fmt, fs, io}; pub(crate) enum Descriptor { @@ -42,193 +44,12 @@ impl fmt::Debug for Descriptor { } impl Descriptor { - pub(crate) fn try_clone(&self) -> io::Result { - match self { - Self::OsHandle(file) => file.try_clone().map(|f| OsHandle::from(f).into()), - Self::VirtualFile(virt) => virt.try_clone().map(Self::VirtualFile), - Self::Stdin => Ok(Self::Stdin), - Self::Stdout => Ok(Self::Stdout), - Self::Stderr => Ok(Self::Stderr), - } - } - - /// Return a reference to the `OsHandle` or `VirtualFile` treating it as an - /// actual file/dir, and allowing operations which require an actual file and - /// not just a stream or socket file descriptor. - pub(crate) fn as_file<'descriptor>(&'descriptor self) -> Result<&'descriptor Self> { - match self { - Self::OsHandle(_) => Ok(self), - Self::VirtualFile(_) => Ok(self), - _ => Err(Errno::Badf), - } - } - /// Return an `OsHandle`, which may be a stream or socket file descriptor. pub(crate) fn as_os_handle<'descriptor>(&'descriptor self) -> OsHandleRef<'descriptor> { descriptor_as_oshandle(self) } } -/// An abstraction struct serving as a wrapper for a host `Descriptor` object which requires -/// certain base rights `rights_base` and inheriting rights `rights_inheriting` in order to be -/// accessed correctly. -/// -/// 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` and -/// `Entry::as_descriptor_mut` methods which require a set of base and inheriting rights to be -/// specified, verifying whether the stored `Descriptor` object is valid for the rights specified. -#[derive(Debug)] -pub(crate) struct Entry { - pub(crate) file_type: Filetype, - descriptor: Descriptor, - pub(crate) rights_base: Rights, - pub(crate) rights_inheriting: Rights, - pub(crate) preopen_path: Option, - // TODO: directories -} - -impl Entry { - pub(crate) fn from(file: Descriptor) -> io::Result { - match file { - Descriptor::OsHandle(handle) => unsafe { determine_type_and_access_rights(&handle) } - .map(|(file_type, rights_base, rights_inheriting)| Self { - file_type, - descriptor: handle.into(), - rights_base, - rights_inheriting, - preopen_path: None, - }), - Descriptor::VirtualFile(virt) => { - let file_type = virt.get_file_type(); - let rights_base = virt.get_rights_base(); - let rights_inheriting = virt.get_rights_inheriting(); - - Ok(Self { - file_type, - descriptor: virt.into(), - rights_base, - rights_inheriting, - preopen_path: None, - }) - } - Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { - panic!("implementation error, stdin/stdout/stderr Entry must not be constructed from Entry::from"); - } - } - } - - pub(crate) fn duplicate_stdin() -> io::Result { - unsafe { determine_type_and_access_rights(&io::stdin()) }.map( - |(file_type, rights_base, rights_inheriting)| Self { - file_type, - descriptor: Descriptor::Stdin, - rights_base, - rights_inheriting, - preopen_path: None, - }, - ) - } - - pub(crate) fn duplicate_stdout() -> io::Result { - unsafe { determine_type_and_access_rights(&io::stdout()) }.map( - |(file_type, rights_base, rights_inheriting)| Self { - file_type, - descriptor: Descriptor::Stdout, - rights_base, - rights_inheriting, - preopen_path: None, - }, - ) - } - - pub(crate) fn duplicate_stderr() -> io::Result { - unsafe { determine_type_and_access_rights(&io::stderr()) }.map( - |(file_type, rights_base, rights_inheriting)| Self { - file_type, - descriptor: Descriptor::Stderr, - rights_base, - rights_inheriting, - preopen_path: None, - }, - ) - } - - pub(crate) fn null() -> io::Result { - Self::from(OsHandle::from(dev_null()?).into()) - } - - /// Convert this `Entry` into a host `Descriptor` object provided the specified - /// `rights_base` and `rights_inheriting` rights are set on this `Entry` object. - /// - /// The `Entry` can only be converted into a valid `Descriptor` object if - /// the specified set of base rights `rights_base`, and inheriting rights `rights_inheriting` - /// 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_descriptor( - &self, - rights_base: Rights, - rights_inheriting: Rights, - ) -> Result<&Descriptor> { - self.validate_rights(rights_base, rights_inheriting)?; - Ok(&self.descriptor) - } - - /// Convert this `Entry` into a mutable host `Descriptor` object provided the specified - /// `rights_base` and `rights_inheriting` rights are set on this `Entry` object. - /// - /// The `Entry` can only be converted into a valid `Descriptor` object if - /// the specified set of base rights `rights_base`, and inheriting rights `rights_inheriting` - /// 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_descriptor_mut( - &mut self, - rights_base: Rights, - rights_inheriting: Rights, - ) -> Result<&mut Descriptor> { - self.validate_rights(rights_base, rights_inheriting)?; - Ok(&mut self.descriptor) - } - - /// Check if this `Entry` object satisfies the specified base rights `rights_base`, and - /// inheriting rights `rights_inheriting`; 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_base: Rights, - rights_inheriting: Rights, - ) -> Result<()> { - let missing_base = !self.rights_base & rights_base; - let missing_inheriting = !self.rights_inheriting & rights_inheriting; - if missing_base != Rights::empty() || missing_inheriting != Rights::empty() { - log::trace!( - " | validate_rights failed: required: \ - rights_base = {}, rights_inheriting = {}; \ - actual: rights_base = {}, rights_inheriting = {}; \ - missing_base = {}, missing_inheriting = {}", - rights_base, - rights_inheriting, - self.rights_base, - self.rights_inheriting, - missing_base, - missing_inheriting - ); - Err(Errno::Notcapable) - } else { - Ok(()) - } - } - - /// 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_base & (Rights::FD_SEEK | Rights::FD_TELL)) == Rights::empty() - } -} - /// This allows an `OsHandle` to be temporarily borrowed from a /// `Descriptor`. The `Descriptor` continues to own the resource, /// and `OsHandleRef`'s lifetime parameter ensures that it doesn't @@ -254,3 +75,166 @@ impl<'descriptor> Deref for OsHandleRef<'descriptor> { &self.handle } } + +/// 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 { + 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 EntryRights { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + writeln!( + f, + "EntryRights {{\n\tbase: {},\n\tinheriting: {}\n}}", + self.base, self.inheriting + ) + } +} + +/// An abstraction struct serving as a wrapper for a host `Descriptor` object which requires +/// certain rights `rights` in order to be accessed correctly. +/// +/// 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 +/// which require a set of base and inheriting rights to be specified, verifying whether the stored +/// `Descriptor` object is valid for the rights specified. +#[derive(Debug)] +pub(crate) struct Entry { + pub(crate) file_type: Filetype, + descriptor: Rc>, + pub(crate) rights: Cell, + pub(crate) preopen_path: Option, + // TODO: directories +} + +impl Entry { + pub(crate) fn from(file: Descriptor) -> io::Result { + match file { + Descriptor::OsHandle(handle) => unsafe { determine_type_and_access_rights(&handle) } + .map(|(file_type, rights)| Self { + file_type, + descriptor: Rc::new(RefCell::new(handle.into())), + rights: Cell::new(rights), + preopen_path: None, + }), + Descriptor::VirtualFile(virt) => { + let file_type = virt.get_file_type(); + let rights = EntryRights::new(virt.get_rights_base(), virt.get_rights_inheriting()); + + Ok(Self { + file_type, + descriptor: Rc::new(RefCell::new(virt.into())), + rights: Cell::new(rights), + preopen_path: None, + }) + } + Descriptor::Stdin | Descriptor::Stdout | Descriptor::Stderr => { + panic!("implementation error, stdin/stdout/stderr Entry must not be constructed from Entry::from"); + } + } + } + + pub(crate) fn duplicate_stdin() -> io::Result { + unsafe { determine_type_and_access_rights(&io::stdin()) }.map(|(file_type, rights)| Self { + file_type, + descriptor: Rc::new(RefCell::new(Descriptor::Stdin)), + rights: Cell::new(rights), + preopen_path: None, + }) + } + + pub(crate) fn duplicate_stdout() -> io::Result { + unsafe { determine_type_and_access_rights(&io::stdout()) }.map(|(file_type, rights)| Self { + file_type, + descriptor: Rc::new(RefCell::new(Descriptor::Stdout)), + rights: Cell::new(rights), + preopen_path: None, + }) + } + + pub(crate) fn duplicate_stderr() -> io::Result { + unsafe { determine_type_and_access_rights(&io::stderr()) }.map(|(file_type, rights)| Self { + file_type, + descriptor: Rc::new(RefCell::new(Descriptor::Stderr)), + rights: Cell::new(rights), + preopen_path: None, + }) + } + + pub(crate) fn null() -> io::Result { + Self::from(OsHandle::from(dev_null()?).into()) + } + + /// Convert this `Entry` into a host `Descriptor` 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 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 + /// performed using `Entry::validate_rights` method. If the check fails, `Errno::Notcapable` + /// is returned. + pub(crate) fn as_descriptor(&self, rights: &EntryRights) -> Result>> { + self.validate_rights(rights)?; + Ok(Rc::clone(&self.descriptor)) + } + + /// Check if this `Entry` object satisfies the specified `EntryRights`; 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) { + Ok(()) + } else { + log::trace!( + " | validate_rights failed: required rights = {}; actual rights = {}", + rights, + self.rights.get(), + ); + Err(Errno::Notcapable) + } + } + + /// 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)) + } +} diff --git a/crates/wasi-common/src/path.rs b/crates/wasi-common/src/path.rs index 80f80f9c2f..d7254ade74 100644 --- a/crates/wasi-common/src/path.rs +++ b/crates/wasi-common/src/path.rs @@ -1,7 +1,7 @@ +use crate::entry::{Descriptor, Entry, EntryRights}; use crate::sys; use crate::sys::entry::OsHandle; use crate::wasi::{types, Errno, Result}; -use crate::{entry::Descriptor, entry::Entry}; use std::path::{Component, Path}; use std::str; use wiggle::{GuestBorrows, GuestPtr}; @@ -102,8 +102,7 @@ impl<'a, 'b> PathRef<'a, 'b> { /// This is a workaround for not having Capsicum support in the OS. pub(crate) fn get( fe: &Entry, - rights_base: types::Rights, - rights_inheriting: types::Rights, + required_rights: &EntryRights, dirflags: types::Lookupflags, path: &GuestPtr<'_, str>, needs_final_component: bool, @@ -129,10 +128,12 @@ pub(crate) fn get( return Err(Errno::Notdir); } - let dirfd = fe - .as_descriptor(rights_base, rights_inheriting)? - .as_file()? - .try_clone()?; + let desc = fe.as_descriptor(required_rights)?; + let dirfd = match &*desc.borrow() { + Descriptor::OsHandle(file) => file.try_clone().map(|f| OsHandle::from(f).into())?, + Descriptor::VirtualFile(virt) => virt.try_clone().map(Descriptor::VirtualFile)?, + _ => return Err(Errno::Badf), + }; // Stack of directory file descriptors. Index 0 always corresponds with the directory provided // to this function. Entering a directory causes a file descriptor to be pushed, while handling diff --git a/crates/wasi-common/src/poll.rs b/crates/wasi-common/src/poll.rs index 3842534dae..755072fc1a 100644 --- a/crates/wasi-common/src/poll.rs +++ b/crates/wasi-common/src/poll.rs @@ -1,7 +1,8 @@ use crate::entry::Descriptor; use crate::sys; use crate::wasi::types; -use std::cell::Ref; +use std::cell::RefCell; +use std::rc::Rc; pub(crate) use sys::poll::*; @@ -12,8 +13,8 @@ pub(crate) struct ClockEventData { } #[derive(Debug)] -pub(crate) struct FdEventData<'a> { - pub(crate) descriptor: Ref<'a, Descriptor>, +pub(crate) struct FdEventData { + pub(crate) descriptor: Rc>, pub(crate) r#type: types::Eventtype, pub(crate) userdata: types::Userdata, } diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index 7038bea6e7..fd3024c822 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -1,11 +1,10 @@ -use crate::entry::{Descriptor, Entry}; +use crate::entry::{Descriptor, Entry, EntryRights}; use crate::sandboxed_tty_writer::SandboxedTTYWriter; use crate::wasi::wasi_snapshot_preview1::WasiSnapshotPreview1; use crate::wasi::{types, AsBytes, Errno, Result}; use crate::WasiCtx; use crate::{clock, fd, path, poll}; use log::{debug, error, trace}; -use std::cell::Ref; use std::convert::TryInto; use std::fs::File; use std::io::{self, Read, Seek, SeekFrom, Write}; @@ -124,19 +123,15 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { len, advice ); + let required_rights = EntryRights::from_base(types::Rights::FD_ADVISE); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor(types::Rights::FD_ADVISE, types::Rights::empty())? - .as_file()?; - match file { - Descriptor::OsHandle(fd) => fd::advise(&fd, advice, offset, len), - Descriptor::VirtualFile(virt) => virt.advise(advice, offset, len), - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } - } + let desc = entry.as_descriptor(&required_rights)?; + match &*desc.borrow() { + Descriptor::OsHandle(fd) => fd::advise(&fd, advice, offset, len)?, + Descriptor::VirtualFile(virt) => virt.advise(advice, offset, len)?, + _ => return Err(Errno::Badf), + }; + Ok(()) } fn fd_allocate( @@ -147,11 +142,10 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ) -> Result<()> { trace!("fd_allocate(fd={:?}, offset={}, len={})", fd, offset, len); + let required_rights = EntryRights::from_base(types::Rights::FD_ALLOCATE); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor(types::Rights::FD_ALLOCATE, types::Rights::empty())? - .as_file()?; - match file { + let desc = entry.as_descriptor(&required_rights)?; + match &*desc.borrow() { Descriptor::OsHandle(fd) => { let metadata = fd.metadata()?; let current_size = metadata.len(); @@ -163,15 +157,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { if wanted_size > current_size { fd.set_len(wanted_size)?; } - Ok(()) } - Descriptor::VirtualFile(virt) => virt.allocate(offset, len), - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } - } + Descriptor::VirtualFile(virt) => virt.allocate(offset, len)?, + _ => return Err(Errno::Badf), + }; + Ok(()) } fn fd_close(&self, fd: types::Fd) -> Result<()> { @@ -191,9 +181,10 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn fd_datasync(&self, fd: types::Fd) -> Result<()> { trace!("fd_datasync(fd={:?})", fd); + let required_rights = EntryRights::from_base(types::Rights::FD_DATASYNC); let entry = self.get_entry(fd)?; - let file = entry.as_descriptor(types::Rights::FD_DATASYNC, types::Rights::empty())?; - match file { + let file = entry.as_descriptor(&required_rights)?; + match &*file.borrow() { Descriptor::OsHandle(fd) => fd.sync_data()?, Descriptor::VirtualFile(virt) => virt.datasync()?, other => other.as_os_handle().sync_data()?, @@ -205,16 +196,17 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { trace!("fd_fdstat_get(fd={:?})", fd); let fe = self.get_entry(fd)?; - let wasi_file = fe.as_descriptor(types::Rights::empty(), types::Rights::empty())?; - let fs_flags = match wasi_file { + let wasi_file = fe.as_descriptor(&EntryRights::empty())?; + let fs_flags = match &*wasi_file.borrow() { Descriptor::OsHandle(wasi_fd) => fd::fdstat_get(&wasi_fd)?, Descriptor::VirtualFile(virt) => virt.fdstat_get(), other => fd::fdstat_get(&other.as_os_handle())?, }; + let rights = fe.rights.get(); let fdstat = types::Fdstat { fs_filetype: fe.file_type, - fs_rights_base: fe.rights_base, - fs_rights_inheriting: fe.rights_inheriting, + fs_rights_base: rights.base, + fs_rights_inheriting: rights.inheriting, fs_flags, }; @@ -226,27 +218,24 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn fd_fdstat_set_flags(&self, fd: types::Fd, flags: types::Fdflags) -> Result<()> { trace!("fd_fdstat_set_flags(fd={:?}, fdflags={})", fd, flags); - let mut entry = self.get_entry_mut(fd)?; - let descriptor = - entry.as_descriptor_mut(types::Rights::FD_FDSTAT_SET_FLAGS, types::Rights::empty())?; - match descriptor { + let required_rights = EntryRights::from_base(types::Rights::FD_FDSTAT_SET_FLAGS); + let entry = self.get_entry(fd)?; + let desc = entry.as_descriptor(&required_rights)?; + let maybe_new_desc = match &*desc.borrow() { Descriptor::OsHandle(handle) => { - let set_result = fd::fdstat_set_flags(&handle, flags)?.map(Descriptor::OsHandle); - if let Some(new_descriptor) = set_result { - *descriptor = new_descriptor; - } + fd::fdstat_set_flags(&handle, flags)?.map(Descriptor::OsHandle) } Descriptor::VirtualFile(handle) => { - handle.fdstat_set_flags(flags)?; + handle.fdstat_set_flags(flags)?.map(Descriptor::VirtualFile) } - _ => { - let set_result = fd::fdstat_set_flags(&descriptor.as_os_handle(), flags)? - .map(Descriptor::OsHandle); - if let Some(new_descriptor) = set_result { - *descriptor = new_descriptor; - } + stream => { + fd::fdstat_set_flags(&stream.as_os_handle(), flags)?.map(Descriptor::OsHandle) } }; + // TODO What happens on None? + if let Some(new_desc) = maybe_new_desc { + *desc.borrow_mut() = new_desc; + } Ok(()) } @@ -262,32 +251,25 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fs_rights_base, fs_rights_inheriting ); - let mut entry = self.get_entry_mut(fd)?; - if entry.rights_base & fs_rights_base != fs_rights_base - || entry.rights_inheriting & fs_rights_inheriting != fs_rights_inheriting - { + let rights = EntryRights::new(fs_rights_base, fs_rights_inheriting); + let entry = self.get_entry(fd)?; + if !entry.rights.get().contains(&rights) { return Err(Errno::Notcapable); } - entry.rights_base = fs_rights_base; - entry.rights_inheriting = fs_rights_inheriting; + entry.rights.set(rights); Ok(()) } fn fd_filestat_get(&self, fd: types::Fd) -> Result { trace!("fd_filestat_get(fd={:?})", fd); + let required_rights = EntryRights::from_base(types::Rights::FD_FILESTAT_GET); let entry = self.get_entry(fd)?; - let fd = entry - .as_descriptor(types::Rights::FD_FILESTAT_GET, types::Rights::empty())? - .as_file()?; - let host_filestat = match fd { + let desc = entry.as_descriptor(&required_rights)?; + let host_filestat = match &*desc.borrow() { Descriptor::OsHandle(fd) => fd::filestat_get(&fd)?, Descriptor::VirtualFile(virt) => virt.filestat_get()?, - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } + _ => return Err(Errno::Badf), }; trace!(" | *filestat_ptr={:?}", host_filestat); @@ -298,22 +280,17 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn fd_filestat_set_size(&self, fd: types::Fd, size: types::Filesize) -> Result<()> { trace!("fd_filestat_set_size(fd={:?}, size={})", fd, size); + let required_rights = EntryRights::from_base(types::Rights::FD_FILESTAT_SET_SIZE); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor(types::Rights::FD_FILESTAT_SET_SIZE, types::Rights::empty())? - .as_file()?; + let desc = entry.as_descriptor(&required_rights)?; // This check will be unnecessary when rust-lang/rust#63326 is fixed if size > i64::max_value() as u64 { return Err(Errno::TooBig); } - match file { + match &*desc.borrow() { Descriptor::OsHandle(fd) => fd.set_len(size)?, Descriptor::VirtualFile(virt) => virt.filestat_set_size(size)?, - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } + _ => return Err(Errno::Badf), }; Ok(()) } @@ -332,11 +309,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { mtim, fst_flags ); + let required_rights = EntryRights::from_base(types::Rights::FD_FILESTAT_SET_TIMES); let entry = self.get_entry(fd)?; - let fd = entry - .as_descriptor(types::Rights::FD_FILESTAT_SET_TIMES, types::Rights::empty())? - .as_file()?; - fd::filestat_set_times_impl(&fd, atim, mtim, fst_flags) + let desc = entry.as_descriptor(&required_rights)?; + fd::filestat_set_times_impl(&desc.borrow(), atim, mtim, fst_flags)?; + Ok(()) } fn fd_pread( @@ -361,19 +338,16 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { buf.push(io::IoSliceMut::new(slice)); } + let required_rights = + EntryRights::from_base(types::Rights::FD_READ | types::Rights::FD_SEEK); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor( - types::Rights::FD_READ | types::Rights::FD_SEEK, - types::Rights::empty(), - )? - .as_file()?; + let desc = entry.as_descriptor(&required_rights)?; if offset > i64::max_value() as u64 { return Err(Errno::Io); } - let host_nread = match file { + let host_nread = match &*desc.borrow() { Descriptor::OsHandle(fd) => { let mut fd: &File = fd; let cur_pos = fd.seek(SeekFrom::Current(0))?; @@ -383,11 +357,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { nread } Descriptor::VirtualFile(virt) => virt.preadv(&mut buf, offset)?, - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } + _ => return Err(Errno::Badf), }; let host_nread = host_nread.try_into()?; @@ -475,19 +445,16 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { buf.push(io::IoSlice::new(slice)); } + let required_rights = + EntryRights::from_base(types::Rights::FD_WRITE | types::Rights::FD_SEEK); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor( - types::Rights::FD_WRITE | types::Rights::FD_SEEK, - types::Rights::empty(), - )? - .as_file()?; + let desc = entry.as_descriptor(&required_rights)?; if offset > i64::max_value() as u64 { return Err(Errno::Io); } - let host_nwritten = match file { + let host_nwritten = match &*desc.borrow() { Descriptor::OsHandle(fd) => { let mut fd: &File = fd; let cur_pos = fd.seek(SeekFrom::Current(0))?; @@ -497,11 +464,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { nwritten } Descriptor::VirtualFile(virt) => virt.pwritev(&buf, offset)?, - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } + _ => return Err(Errno::Badf), }; trace!(" | *nwritten={:?}", host_nwritten); let host_nwritten = host_nwritten.try_into()?; @@ -526,14 +489,14 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { slices.push(io::IoSliceMut::new(slice)); } + let required_rights = EntryRights::from_base(types::Rights::FD_READ); let entry = self.get_entry(fd)?; - let host_nread = - match entry.as_descriptor(types::Rights::FD_READ, types::Rights::empty())? { - Descriptor::OsHandle(file) => (file as &File).read_vectored(&mut slices)?, - Descriptor::VirtualFile(virt) => virt.read_vectored(&mut slices)?, - Descriptor::Stdin => io::stdin().read_vectored(&mut slices)?, - _ => return Err(Errno::Badf), - }; + let host_nread = match &*entry.as_descriptor(&required_rights)?.borrow() { + Descriptor::OsHandle(file) => (file as &File).read_vectored(&mut slices)?, + Descriptor::VirtualFile(virt) => virt.read_vectored(&mut slices)?, + Descriptor::Stdin => io::stdin().read_vectored(&mut slices)?, + _ => return Err(Errno::Badf), + }; let host_nread = host_nread.try_into()?; trace!(" | *nread={:?}", host_nread); @@ -556,10 +519,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { cookie, ); + let required_rights = EntryRights::from_base(types::Rights::FD_READDIR); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor(types::Rights::FD_READDIR, types::Rights::empty())? - .as_file()?; + let desc = entry.as_descriptor(&required_rights)?; fn copy_entities>>( iter: T, @@ -587,14 +549,10 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } Ok(bufused) } - let bufused = match file { + let bufused = match &*desc.borrow() { Descriptor::OsHandle(file) => copy_entities(fd::readdir(file, cookie)?, buf, buf_len)?, Descriptor::VirtualFile(virt) => copy_entities(virt.readdir(cookie)?, buf, buf_len)?, - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } + _ => return Err(Errno::Badf), }; trace!(" | *buf_used={:?}", bufused); @@ -640,28 +598,23 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { whence, ); - let rights = if offset == 0 && whence == types::Whence::Cur { + let base = if offset == 0 && whence == types::Whence::Cur { types::Rights::FD_TELL } else { types::Rights::FD_SEEK | types::Rights::FD_TELL }; + let required_rights = EntryRights::from_base(base); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor(rights, types::Rights::empty())? - .as_file()?; + let desc = entry.as_descriptor(&required_rights)?; let pos = match whence { types::Whence::Cur => SeekFrom::Current(offset), types::Whence::End => SeekFrom::End(offset), types::Whence::Set => SeekFrom::Start(offset as u64), }; - let host_newoffset = match file { + let host_newoffset = match &*desc.borrow() { Descriptor::OsHandle(fd) => (fd as &File).seek(pos)?, Descriptor::VirtualFile(virt) => virt.seek(pos)?, - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } + _ => return Err(Errno::Badf), }; trace!(" | *newoffset={:?}", host_newoffset); @@ -672,18 +625,13 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn fd_sync(&self, fd: types::Fd) -> Result<()> { trace!("fd_sync(fd={:?})", fd); + let required_rights = EntryRights::from_base(types::Rights::FD_SYNC); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor(types::Rights::FD_SYNC, types::Rights::empty())? - .as_file()?; - match file { + let desc = entry.as_descriptor(&required_rights)?; + match &*desc.borrow() { Descriptor::OsHandle(fd) => fd.sync_all()?, Descriptor::VirtualFile(virt) => virt.sync()?, - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } + _ => return Err(Errno::Badf), }; Ok(()) } @@ -691,18 +639,13 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn fd_tell(&self, fd: types::Fd) -> Result { trace!("fd_tell(fd={:?})", fd); + let required_rights = EntryRights::from_base(types::Rights::FD_TELL); let entry = self.get_entry(fd)?; - let file = entry - .as_descriptor(types::Rights::FD_TELL, types::Rights::empty())? - .as_file()?; - let host_offset = match file { + let desc = entry.as_descriptor(&required_rights)?; + let host_offset = match &*desc.borrow() { Descriptor::OsHandle(fd) => (fd as &File).seek(SeekFrom::Current(0))?, Descriptor::VirtualFile(virt) => virt.seek(SeekFrom::Current(0))?, - _ => { - unreachable!( - "implementation error: fd should have been checked to not be a stream already" - ); - } + _ => return Err(Errno::Badf), }; trace!(" | *newoffset={:?}", host_offset); @@ -728,10 +671,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } // perform unbuffered writes + let required_rights = EntryRights::from_base(types::Rights::FD_WRITE); let entry = self.get_entry(fd)?; let isatty = entry.isatty(); - let desc = entry.as_descriptor(types::Rights::FD_WRITE, types::Rights::empty())?; - let host_nwritten = match desc { + let desc = entry.as_descriptor(&required_rights)?; + let host_nwritten = match &*desc.borrow() { Descriptor::OsHandle(file) => { if isatty { SandboxedTTYWriter::new(&mut (file as &File)).write_vectored(&slices)? @@ -774,12 +718,12 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn path_create_directory(&self, dirfd: types::Fd, path: &GuestPtr<'_, str>) -> Result<()> { trace!("path_create_directory(dirfd={:?}, path={:?})", dirfd, path); - let rights = types::Rights::PATH_OPEN | types::Rights::PATH_CREATE_DIRECTORY; + let required_rights = + EntryRights::from_base(types::Rights::PATH_OPEN | types::Rights::PATH_CREATE_DIRECTORY); let entry = self.get_entry(dirfd)?; let resolved = path::get( &entry, - rights, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), path, false, @@ -800,15 +744,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { path, ); + let required_rights = EntryRights::from_base(types::Rights::PATH_FILESTAT_GET); let entry = self.get_entry(dirfd)?; - let resolved = path::get( - &entry, - types::Rights::PATH_FILESTAT_GET, - types::Rights::empty(), - flags, - path, - false, - )?; + let resolved = path::get(&entry, &required_rights, flags, path, false)?; let host_filestat = match resolved.dirfd() { Descriptor::VirtualFile(virt) => virt .openat( @@ -846,15 +784,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fst_flags, ); + let required_rights = EntryRights::from_base(types::Rights::PATH_FILESTAT_SET_TIMES); let entry = self.get_entry(dirfd)?; - let resolved = path::get( - &entry, - types::Rights::PATH_FILESTAT_SET_TIMES, - types::Rights::empty(), - flags, - path, - false, - )?; + let resolved = path::get(&entry, &required_rights, flags, path, false)?; match resolved.dirfd() { Descriptor::VirtualFile(_virt) => { unimplemented!("virtual filestat_set_times"); @@ -880,20 +812,20 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { new_path, ); + let required_rights = EntryRights::from_base(types::Rights::PATH_LINK_SOURCE); let old_entry = self.get_entry(old_fd)?; let resolved_old = path::get( &old_entry, - types::Rights::PATH_LINK_SOURCE, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), old_path, false, )?; + let required_rights = EntryRights::from_base(types::Rights::PATH_LINK_TARGET); let new_entry = self.get_entry(new_fd)?; let resolved_new = path::get( &new_entry, - types::Rights::PATH_LINK_TARGET, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), new_path, false, @@ -926,21 +858,19 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fdflags, ); - let (needed_base, needed_inheriting) = - path::open_rights(fs_rights_base, fs_rights_inheriting, oflags, fdflags); - - trace!( - " | needed_base = {}, needed_inheriting = {}", - needed_base, - needed_inheriting + let needed_rights = path::open_rights( + &EntryRights::new(fs_rights_base, fs_rights_inheriting), + oflags, + fdflags, ); + trace!(" | needed rights = {}", needed_rights); + let resolved = { let entry = self.get_entry(dirfd)?; path::get( &entry, - needed_base, - needed_inheriting, + &needed_rights, dirflags, path, oflags & types::Oflags::CREAT != types::Oflags::empty(), @@ -964,11 +894,13 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ); let fd = resolved.open_with(read, write, oflags, fdflags)?; - let mut fe = Entry::from(fd)?; + let fe = Entry::from(fd)?; // We need to manually deny the rights which are not explicitly requested // because Entry::from will assign maximal consistent rights. - fe.rights_base &= fs_rights_base; - fe.rights_inheriting &= fs_rights_inheriting; + let mut rights = fe.rights.get(); + rights.base &= fs_rights_base; + rights.inheriting &= fs_rights_inheriting; + fe.rights.set(rights); let guest_fd = self.insert_entry(fe)?; trace!(" | *fd={:?}", guest_fd); @@ -991,11 +923,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { buf_len, ); + let required_rights = EntryRights::from_base(types::Rights::PATH_READLINK); let entry = self.get_entry(dirfd)?; let resolved = path::get( &entry, - types::Rights::PATH_READLINK, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), path, false, @@ -1024,11 +956,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn path_remove_directory(&self, dirfd: types::Fd, path: &GuestPtr<'_, str>) -> Result<()> { trace!("path_remove_directory(dirfd={:?}, path={:?})", dirfd, path); + let required_rights = EntryRights::from_base(types::Rights::PATH_REMOVE_DIRECTORY); let entry = self.get_entry(dirfd)?; let resolved = path::get( &entry, - types::Rights::PATH_REMOVE_DIRECTORY, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), path, true, @@ -1057,20 +989,20 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { new_path, ); + let required_rights = EntryRights::from_base(types::Rights::PATH_RENAME_SOURCE); let entry = self.get_entry(old_fd)?; let resolved_old = path::get( &entry, - types::Rights::PATH_RENAME_SOURCE, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), old_path, true, )?; + let required_rights = EntryRights::from_base(types::Rights::PATH_RENAME_TARGET); let entry = self.get_entry(new_fd)?; let resolved_new = path::get( &entry, - types::Rights::PATH_RENAME_TARGET, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), new_path, true, @@ -1103,11 +1035,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { new_path, ); + let required_rights = EntryRights::from_base(types::Rights::PATH_SYMLINK); let entry = self.get_entry(dirfd)?; let resolved_new = path::get( &entry, - types::Rights::PATH_SYMLINK, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), new_path, true, @@ -1132,11 +1064,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { fn path_unlink_file(&self, dirfd: types::Fd, path: &GuestPtr<'_, str>) -> Result<()> { trace!("path_unlink_file(dirfd={:?}, path={:?})", dirfd, path); + let required_rights = EntryRights::from_base(types::Rights::PATH_UNLINK_FILE); let entry = self.get_entry(dirfd)?; let resolved = path::get( &entry, - types::Rights::PATH_UNLINK_FILE, - types::Rights::empty(), + &required_rights, types::Lookupflags::empty(), path, false, @@ -1201,7 +1133,9 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } types::SubscriptionU::FdRead(fd_read) => { let fd = fd_read.file_descriptor; - let rights = types::Rights::FD_READ | types::Rights::POLL_FD_READWRITE; + let required_rights = EntryRights::from_base( + types::Rights::FD_READ | types::Rights::POLL_FD_READWRITE, + ); let entry = match self.get_entry(fd) { Ok(entry) => entry, Err(error) => { @@ -1217,21 +1151,17 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { continue; } }; - // TODO Can this be simplified? - // Validate rights on the entry before converting into host descriptor. - entry.validate_rights(rights, types::Rights::empty())?; - let descriptor = Ref::map(entry, |entry| { - entry.as_descriptor(rights, types::Rights::empty()).unwrap() - }); fd_events.push(poll::FdEventData { - descriptor, + descriptor: entry.as_descriptor(&required_rights)?, r#type: types::Eventtype::FdRead, userdata: subscription.userdata, }); } types::SubscriptionU::FdWrite(fd_write) => { let fd = fd_write.file_descriptor; - let rights = types::Rights::FD_WRITE | types::Rights::POLL_FD_READWRITE; + let required_rights = EntryRights::from_base( + types::Rights::FD_WRITE | types::Rights::POLL_FD_READWRITE, + ); let entry = match self.get_entry(fd) { Ok(entry) => entry, Err(error) => { @@ -1247,14 +1177,8 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { continue; } }; - // TODO Can this be simplified? - // Validate rights on the entry before converting into host descriptor. - entry.validate_rights(rights, types::Rights::empty())?; - let descriptor = Ref::map(entry, |entry| { - entry.as_descriptor(rights, types::Rights::empty()).unwrap() - }); fd_events.push(poll::FdEventData { - descriptor, + descriptor: entry.as_descriptor(&required_rights)?, r#type: types::Eventtype::FdWrite, userdata: subscription.userdata, }); diff --git a/crates/wasi-common/src/sys/unix/bsd/fd.rs b/crates/wasi-common/src/sys/unix/bsd/fd.rs deleted file mode 100644 index 884ccf0c6b..0000000000 --- a/crates/wasi-common/src/sys/unix/bsd/fd.rs +++ /dev/null @@ -1,21 +0,0 @@ -use crate::sys::entry::OsHandle; -use crate::wasi::Result; -use std::cell::RefMut; -use yanix::dir::Dir; - -pub(crate) fn get_dir_from_os_handle(os_handle: &OsHandle) -> Result> { - if os_handle.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 fd = (*os_handle).try_clone()?; - let d = Dir::from(fd)?; - *os_handle.dir.borrow_mut() = Some(d); - } - Ok(RefMut::map(os_handle.dir.borrow_mut(), |dir| { - dir.as_mut().unwrap() - })) -} diff --git a/crates/wasi-common/src/sys/unix/bsd/mod.rs b/crates/wasi-common/src/sys/unix/bsd/mod.rs index 153a194641..a58b813e4a 100644 --- a/crates/wasi-common/src/sys/unix/bsd/mod.rs +++ b/crates/wasi-common/src/sys/unix/bsd/mod.rs @@ -1,4 +1,3 @@ -pub(crate) mod fd; pub(crate) mod oshandle; pub(crate) mod path; diff --git a/crates/wasi-common/src/sys/unix/bsd/oshandle.rs b/crates/wasi-common/src/sys/unix/bsd/oshandle.rs index 1918a2af34..efd5d2602c 100644 --- a/crates/wasi-common/src/sys/unix/bsd/oshandle.rs +++ b/crates/wasi-common/src/sys/unix/bsd/oshandle.rs @@ -1,4 +1,5 @@ -use std::cell::RefCell; +use crate::wasi::Result; +use std::cell::{RefCell, RefMut}; use std::fs; use std::ops::Deref; use std::os::unix::prelude::{AsRawFd, RawFd}; @@ -6,7 +7,7 @@ use yanix::dir::Dir; #[derive(Debug)] pub(crate) struct OsHandle { - pub(crate) file: fs::File, + file: fs::File, // 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 @@ -18,7 +19,26 @@ pub(crate) struct OsHandle { // > 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. - pub(crate) dir: RefCell>, + dir: RefCell>, +} + +impl OsHandle { + 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 fd = self.file.try_clone()?; + let d = Dir::from(fd)?; + *self.dir.borrow_mut() = Some(d); + } + Ok(RefMut::map(self.dir.borrow_mut(), |dir| { + dir.as_mut().unwrap() + })) + } } impl From for OsHandle { diff --git a/crates/wasi-common/src/sys/unix/emscripten/mod.rs b/crates/wasi-common/src/sys/unix/emscripten/mod.rs index 9fca512635..8f233de8fe 100644 --- a/crates/wasi-common/src/sys/unix/emscripten/mod.rs +++ b/crates/wasi-common/src/sys/unix/emscripten/mod.rs @@ -1,5 +1,3 @@ -#[path = "../linux/fd.rs"] -pub(crate) mod fd; #[path = "../linux/oshandle.rs"] pub(crate) mod oshandle; #[path = "../linux/path.rs"] diff --git a/crates/wasi-common/src/sys/unix/entry.rs b/crates/wasi-common/src/sys/unix/entry.rs index 2592ef7cd2..49e82ec481 100644 --- a/crates/wasi-common/src/sys/unix/entry.rs +++ b/crates/wasi-common/src/sys/unix/entry.rs @@ -1,4 +1,4 @@ -use crate::entry::{Descriptor, OsHandleRef}; +use crate::entry::{Descriptor, EntryRights, OsHandleRef}; use crate::wasi::{types, RightsExt}; use std::fs::File; use std::io; @@ -33,19 +33,19 @@ pub(crate) fn descriptor_as_oshandle<'lifetime>( /// This function is unsafe because it operates on a raw file descriptor. pub(crate) unsafe fn determine_type_and_access_rights( fd: &Fd, -) -> io::Result<(types::Filetype, types::Rights, types::Rights)> { - let (file_type, mut rights_base, rights_inheriting) = determine_type_rights(fd)?; +) -> io::Result<(types::Filetype, EntryRights)> { + let (file_type, mut rights) = determine_type_rights(fd)?; use yanix::{fcntl, file::OFlag}; let flags = fcntl::get_status_flags(fd.as_raw_fd())?; let accmode = flags & OFlag::ACCMODE; if accmode == OFlag::RDONLY { - rights_base &= !types::Rights::FD_WRITE; + rights.base &= !types::Rights::FD_WRITE; } else if accmode == OFlag::WRONLY { - rights_base &= !types::Rights::FD_READ; + rights.base &= !types::Rights::FD_READ; } - Ok((file_type, rights_base, rights_inheriting)) + Ok((file_type, rights)) } /// Returns the set of all possible rights that are relevant for file type. @@ -53,12 +53,12 @@ pub(crate) unsafe fn determine_type_and_access_rights( /// This function is unsafe because it operates on a raw file descriptor. pub(crate) unsafe fn determine_type_rights( fd: &Fd, -) -> io::Result<(types::Filetype, types::Rights, types::Rights)> { - let (file_type, rights_base, rights_inheriting) = { +) -> io::Result<(types::Filetype, EntryRights)> { + let (file_type, rights) = { // we just make a `File` here for convenience; we don't want it to close when it drops let file = std::mem::ManuallyDrop::new(std::fs::File::from_raw_fd(fd.as_raw_fd())); let ft = file.metadata()?.file_type(); - if ft.is_block_device() { + let (filetype, base, inheriting) = if ft.is_block_device() { log::debug!("Host fd {:?} is a block device", fd.as_raw_fd()); ( types::Filetype::BlockDevice, @@ -121,8 +121,9 @@ pub(crate) unsafe fn determine_type_rights( } else { log::debug!("Host fd {:?} is unknown", fd.as_raw_fd()); return Err(io::Error::from_raw_os_error(libc::EINVAL)); - } + }; + (filetype, EntryRights::new(base, inheriting)) }; - Ok((file_type, rights_base, rights_inheriting)) + Ok((file_type, rights)) } diff --git a/crates/wasi-common/src/sys/unix/fd.rs b/crates/wasi-common/src/sys/unix/fd.rs index a2732f831f..d150a68ac6 100644 --- a/crates/wasi-common/src/sys/unix/fd.rs +++ b/crates/wasi-common/src/sys/unix/fd.rs @@ -1,4 +1,3 @@ -use super::sys_impl::fd::get_dir_from_os_handle; use crate::sys::entry::OsHandle; use crate::wasi::{self, types, Result}; use std::convert::TryInto; @@ -12,7 +11,9 @@ pub(crate) fn fdstat_get(fd: &File) -> 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())? }; - // TODO why are we returning Ok(None) here? + // We return None here to signal that the operation succeeded on the original + // file descriptor and mutating the original WASI Descriptor is thus unnecessary. + // This is needed as on Windows this operation required reopening a file. Ok(None) } @@ -51,7 +52,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 = get_dir_from_os_handle(os_handle)?; + let mut dir = os_handle.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/fd.rs b/crates/wasi-common/src/sys/unix/linux/fd.rs deleted file mode 100644 index f480294a4e..0000000000 --- a/crates/wasi-common/src/sys/unix/linux/fd.rs +++ /dev/null @@ -1,19 +0,0 @@ -use crate::sys::entry::OsHandle; -use crate::wasi::Result; -use yanix::dir::Dir; - -pub(crate) fn get_dir_from_os_handle(os_handle: &OsHandle) -> 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 fd = os_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(fd)?)) -} diff --git a/crates/wasi-common/src/sys/unix/linux/mod.rs b/crates/wasi-common/src/sys/unix/linux/mod.rs index a0427fcdc3..9d7cc60191 100644 --- a/crates/wasi-common/src/sys/unix/linux/mod.rs +++ b/crates/wasi-common/src/sys/unix/linux/mod.rs @@ -1,4 +1,3 @@ -pub(crate) mod fd; pub(crate) mod oshandle; pub(crate) mod path; diff --git a/crates/wasi-common/src/sys/unix/linux/oshandle.rs b/crates/wasi-common/src/sys/unix/linux/oshandle.rs index bdf75e7f68..241f6cc6b7 100644 --- a/crates/wasi-common/src/sys/unix/linux/oshandle.rs +++ b/crates/wasi-common/src/sys/unix/linux/oshandle.rs @@ -1,10 +1,30 @@ +use crate::wasi::Result; use std::fs; use std::ops::Deref; use std::os::unix::prelude::{AsRawFd, RawFd}; +use yanix::dir::Dir; #[derive(Debug)] pub(crate) struct OsHandle(fs::File); +impl OsHandle { + 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 fd = 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(fd)?)) + } +} + impl From for OsHandle { fn from(file: fs::File) -> Self { Self(file) diff --git a/crates/wasi-common/src/sys/unix/path.rs b/crates/wasi-common/src/sys/unix/path.rs index 1dd0858227..e693041ade 100644 --- a/crates/wasi-common/src/sys/unix/path.rs +++ b/crates/wasi-common/src/sys/unix/path.rs @@ -1,4 +1,4 @@ -use crate::entry::Descriptor; +use crate::entry::{Descriptor, EntryRights}; use crate::path::PathGet; use crate::sys::entry::OsHandle; use crate::sys::unix::sys_impl; @@ -22,14 +22,13 @@ pub(crate) fn from_host>(s: S) -> Result { } pub(crate) fn open_rights( - rights_base: types::Rights, - rights_inheriting: types::Rights, + input_rights: &EntryRights, oflags: types::Oflags, fs_flags: types::Fdflags, -) -> (types::Rights, types::Rights) { +) -> EntryRights { // which rights are needed on the dirfd? let mut needed_base = types::Rights::PATH_OPEN; - let mut needed_inheriting = rights_base | rights_inheriting; + let mut needed_inheriting = input_rights.base | input_rights.inheriting; // convert open flags let oflags: OFlag = oflags.into(); @@ -49,7 +48,7 @@ pub(crate) fn open_rights( needed_inheriting |= types::Rights::FD_SYNC; } - (needed_base, needed_inheriting) + EntryRights::new(needed_base, needed_inheriting) } pub(crate) fn openat(dirfd: &File, path: &str) -> Result { diff --git a/crates/wasi-common/src/sys/unix/poll.rs b/crates/wasi-common/src/sys/unix/poll.rs index 93cac39a68..dcd382d293 100644 --- a/crates/wasi-common/src/sys/unix/poll.rs +++ b/crates/wasi-common/src/sys/unix/poll.rs @@ -26,7 +26,7 @@ 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.descriptor.as_raw_fd(), flags) } + unsafe { PollFd::new(event.descriptor.borrow().as_raw_fd(), flags) } }) .collect(); @@ -69,8 +69,8 @@ fn handle_timeout_event(timeout: ClockEventData, events: &mut Vec) }); } -fn handle_fd_event<'a>( - ready_events: impl Iterator, yanix::poll::PollFd)>, +fn handle_fd_event( + ready_events: impl Iterator, events: &mut Vec, ) -> Result<()> { use crate::entry::Descriptor; @@ -103,7 +103,7 @@ fn handle_fd_event<'a>( log::debug!("poll_oneoff_handle_fd_event revents = {:?}", revents); let nbytes = if fd_event.r#type == types::Eventtype::FdRead { - query_nbytes(&fd_event.descriptor)? + query_nbytes(&fd_event.descriptor.borrow())? } else { 0 }; diff --git a/crates/wasi-common/src/sys/windows/entry.rs b/crates/wasi-common/src/sys/windows/entry.rs index e4d386188f..ffb96f4f44 100644 --- a/crates/wasi-common/src/sys/windows/entry.rs +++ b/crates/wasi-common/src/sys/windows/entry.rs @@ -1,4 +1,4 @@ -use crate::entry::{Descriptor, OsHandleRef}; +use crate::entry::{Descriptor, EntryRights, OsHandleRef}; use crate::wasi::{types, RightsExt}; use std::fs::File; use std::io; @@ -57,19 +57,19 @@ pub(crate) fn descriptor_as_oshandle<'lifetime>( /// This function is unsafe because it operates on a raw file descriptor. pub(crate) unsafe fn determine_type_and_access_rights( handle: &Handle, -) -> io::Result<(types::Filetype, types::Rights, types::Rights)> { +) -> io::Result<(types::Filetype, EntryRights)> { use winx::file::{query_access_information, AccessMode}; - let (file_type, mut rights_base, rights_inheriting) = determine_type_rights(handle)?; + let (file_type, mut rights) = determine_type_rights(handle)?; match file_type { types::Filetype::Directory | types::Filetype::RegularFile => { let mode = query_access_information(handle.as_raw_handle())?; if mode.contains(AccessMode::FILE_GENERIC_READ) { - rights_base |= types::Rights::FD_READ; + rights.base |= types::Rights::FD_READ; } if mode.contains(AccessMode::FILE_GENERIC_WRITE) { - rights_base |= types::Rights::FD_WRITE; + rights.base |= types::Rights::FD_WRITE; } } _ => { @@ -78,7 +78,7 @@ pub(crate) unsafe fn determine_type_and_access_rights( } } - Ok((file_type, rights_base, rights_inheriting)) + Ok((file_type, rights)) } /// Returns the set of all possible rights that are relevant for file type. @@ -86,10 +86,10 @@ pub(crate) unsafe fn determine_type_and_access_rights( /// This function is unsafe because it operates on a raw file descriptor. pub(crate) unsafe fn determine_type_rights( handle: &Handle, -) -> io::Result<(types::Filetype, types::Rights, types::Rights)> { - let (file_type, rights_base, rights_inheriting) = { +) -> io::Result<(types::Filetype, EntryRights)> { + let (file_type, rights) = { let file_type = winx::file::get_file_type(handle.as_raw_handle())?; - if file_type.is_char() { + let (file_type, base, inheriting) = if file_type.is_char() { // character file: LPT device or console // TODO: rule out LPT device ( @@ -126,7 +126,8 @@ pub(crate) unsafe fn determine_type_rights( ) } else { return Err(io::Error::from_raw_os_error(libc::EINVAL)); - } + }; + (file_type, EntryRights::new(base, inheriting)) }; - Ok((file_type, rights_base, rights_inheriting)) + Ok((file_type, rights)) } diff --git a/crates/wasi-common/src/sys/windows/path.rs b/crates/wasi-common/src/sys/windows/path.rs index 32f9d7e46d..24ddf78779 100644 --- a/crates/wasi-common/src/sys/windows/path.rs +++ b/crates/wasi-common/src/sys/windows/path.rs @@ -1,4 +1,4 @@ -use crate::entry::Descriptor; +use crate::entry::{Descriptor, EntryRights}; use crate::fd; use crate::path::PathGet; use crate::sys::entry::OsHandle; @@ -41,14 +41,13 @@ impl PathGetExt for PathGet { } pub(crate) fn open_rights( - rights_base: types::Rights, - rights_inheriting: types::Rights, + input_rights: &EntryRights, oflags: types::Oflags, fdflags: types::Fdflags, -) -> (types::Rights, types::Rights) { +) -> EntryRights { // which rights are needed on the dirfd? let mut needed_base = types::Rights::PATH_OPEN; - let mut needed_inheriting = rights_base | rights_inheriting; + let mut needed_inheriting = input_rights.base | input_rights.inheriting; // convert open flags if oflags.contains(&types::Oflags::CREAT) { @@ -66,7 +65,7 @@ pub(crate) fn open_rights( needed_inheriting |= types::Rights::FD_SYNC; } - (needed_base, needed_inheriting) + EntryRights::new(needed_base, needed_inheriting) } pub(crate) fn openat(dirfd: &File, path: &str) -> Result { diff --git a/crates/wasi-common/src/sys/windows/poll.rs b/crates/wasi-common/src/sys/windows/poll.rs index 71309d1875..7472501d56 100644 --- a/crates/wasi-common/src/sys/windows/poll.rs +++ b/crates/wasi-common/src/sys/windows/poll.rs @@ -5,6 +5,7 @@ use lazy_static::lazy_static; use log::{debug, error, trace, warn}; use std::convert::TryInto; use std::os::windows::io::AsRawHandle; +use std::rc::Rc; use std::sync::mpsc::{self, Receiver, RecvTimeoutError, Sender, TryRecvError}; use std::sync::Mutex; use std::thread; @@ -140,8 +141,7 @@ fn handle_timeout_event(timeout_event: ClockEventData, events: &mut Vec) { - let descriptor: &Descriptor = &event.descriptor; - let size = match descriptor { + let size = match &*event.descriptor.borrow() { Descriptor::OsHandle(os_handle) => { if event.r#type == types::Eventtype::FdRead { os_handle.metadata().map(|m| m.len()).map_err(Into::into) @@ -201,8 +201,8 @@ pub(crate) fn oneoff( let mut pipe_events = vec![]; for event in fd_events { - let descriptor: &Descriptor = &event.descriptor; - match descriptor { + let descriptor = Rc::clone(&event.descriptor); + match &*descriptor.borrow() { Descriptor::Stdin if event.r#type == types::Eventtype::FdRead => { stdin_events.push(event) } @@ -230,7 +230,7 @@ pub(crate) fn oneoff( Descriptor::VirtualFile(_) => { panic!("virtual files do not get rw events"); } - } + }; } let immediate = !immediate_events.is_empty(); diff --git a/crates/wasi-common/src/virtfs.rs b/crates/wasi-common/src/virtfs.rs index 7a2068ab6b..8627c8afd2 100644 --- a/crates/wasi-common/src/virtfs.rs +++ b/crates/wasi-common/src/virtfs.rs @@ -1,7 +1,7 @@ use crate::wasi::{self, types, Errno, Result, RightsExt}; use filetime::FileTime; use log::trace; -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::collections::hash_map::Entry; use std::collections::HashMap; use std::convert::TryInto; @@ -127,7 +127,7 @@ pub(crate) trait VirtualFile: MovableFile { Err(Errno::Badf) } - fn fdstat_set_flags(&mut self, _fdflags: types::Fdflags) -> Result<()> { + fn fdstat_set_flags(&self, _fdflags: types::Fdflags) -> Result>> { Err(Errno::Badf) } @@ -254,26 +254,26 @@ 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 { - cursor: RefCell, + cursor: Cell, parent: Rc>>>, - fd_flags: types::Fdflags, + fd_flags: Cell, data: Rc>>, } impl InMemoryFile { pub fn memory_backed() -> Self { Self { - cursor: RefCell::new(0), + cursor: Cell::new(0), parent: Rc::new(RefCell::new(None)), - fd_flags: types::Fdflags::empty(), + fd_flags: Cell::new(types::Fdflags::empty()), data: Rc::new(RefCell::new(Box::new(VecFileContents::new()))), } } pub fn new(contents: Box) -> Self { Self { - cursor: RefCell::new(0), - fd_flags: types::Fdflags::empty(), + cursor: Cell::new(0), + fd_flags: Cell::new(types::Fdflags::empty()), parent: Rc::new(RefCell::new(None)), data: Rc::new(RefCell::new(contents)), } @@ -288,13 +288,13 @@ impl MovableFile for InMemoryFile { impl VirtualFile for InMemoryFile { fn fdstat_get(&self) -> types::Fdflags { - self.fd_flags + self.fd_flags.get() } fn try_clone(&self) -> io::Result> { Ok(Box::new(Self { - cursor: RefCell::new(0), - fd_flags: self.fd_flags, + cursor: Cell::new(0), + fd_flags: self.fd_flags.clone(), parent: Rc::clone(&self.parent), data: Rc::clone(&self.data), })) @@ -351,23 +351,27 @@ impl VirtualFile for InMemoryFile { Err(Errno::Notdir) } - fn fdstat_set_flags(&mut self, fdflags: types::Fdflags) -> Result<()> { - self.fd_flags = fdflags; - Ok(()) + fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result>> { + self.fd_flags.set(fdflags); + // We return None here to signal that the operation succeeded on the original + // file descriptor and mutating the original WASI Descriptor is thus unnecessary. + // This is needed as on Windows this operation required reopening a file. So we're + // adhering to the common signature required across platforms. + Ok(None) } fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { trace!("write_vectored(iovs={:?})", iovs); let mut data = self.data.borrow_mut(); - let append_mode = self.fd_flags.contains(&types::Fdflags::APPEND); - trace!(" | fd_flags={}", self.fd_flags); + let append_mode = self.fd_flags.get().contains(&types::Fdflags::APPEND); + trace!(" | fd_flags={}", self.fd_flags.get()); // If this file is in append mode, we write to the end. let write_start = if append_mode { data.size() } else { - *self.cursor.borrow() + self.cursor.get() }; let max_size = iovs @@ -396,7 +400,8 @@ impl VirtualFile for InMemoryFile { // If we are not appending, adjust the cursor appropriately for the write, too. This can't // overflow, as we checked against that before writing any data. if !append_mode { - *self.cursor.borrow_mut() += written as u64; + let update = self.cursor.get() + written as u64; + self.cursor.set(update); } Ok(written) @@ -404,8 +409,8 @@ impl VirtualFile for InMemoryFile { fn read_vectored(&self, iovs: &mut [io::IoSliceMut]) -> Result { trace!("read_vectored(iovs={:?})", iovs); - trace!(" | *read_start={:?}", self.cursor); - self.data.borrow_mut().preadv(iovs, *self.cursor.borrow()) + trace!(" | *read_start={:?}", self.cursor.get()); + self.data.borrow_mut().preadv(iovs, self.cursor.get()) } fn preadv(&self, buf: &mut [io::IoSliceMut], offset: types::Filesize) -> Result { @@ -422,30 +427,30 @@ impl VirtualFile for InMemoryFile { SeekFrom::Current(offset) => { let new_cursor = if offset < 0 { self.cursor - .borrow() + .get() .checked_sub(offset.wrapping_neg() as u64) .ok_or(Errno::Inval)? } else { self.cursor - .borrow() + .get() .checked_add(offset as u64) .ok_or(Errno::Inval)? }; - *self.cursor.borrow_mut() = std::cmp::min(content_len, new_cursor); + self.cursor.set(std::cmp::min(content_len, new_cursor)); } SeekFrom::End(offset) => { // A negative offset from the end would be past the end of the file, let offset: u64 = offset.try_into().map_err(|_| Errno::Inval)?; - *self.cursor.borrow_mut() = content_len.saturating_sub(offset); + self.cursor.set(content_len.saturating_sub(offset)); } SeekFrom::Start(offset) => { // A negative offset from the end would be before the start of the file. let offset: u64 = offset.try_into().map_err(|_| Errno::Inval)?; - *self.cursor.borrow_mut() = std::cmp::min(content_len, offset); + self.cursor.set(std::cmp::min(content_len, offset)); } } - Ok(*self.cursor.borrow()) + Ok(self.cursor.get()) } fn advise( @@ -656,8 +661,8 @@ impl VirtualFile for VirtualDir { path.display() ); - let mut file = Box::new(InMemoryFile::memory_backed()); - file.fd_flags = fd_flags; + let file = Box::new(InMemoryFile::memory_backed()); + file.fd_flags.set(fd_flags); file.set_parent(Some(self.try_clone().expect("can clone self"))); v.insert(file).try_clone().map_err(Into::into) } else {