From 65ebfc3a035a66843e0b1fc2fd03723665f4de88 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 10 Jun 2020 16:37:42 +0200 Subject: [PATCH] wasi-common: don't rely on platform dependent "NUL" device If stdio is not inherited nor associated with a file, WasiCtxBuilder tries to open "/dev/null" ("NUL" on Windows) and attach stdio to it. While most platforms today support those device files, it would be good to avoid unnecessary access to the host device if possible. This patch instead uses a virtual Handle that emulates the "NUL" device. --- crates/wasi-common/src/ctx.rs | 8 +-- crates/wasi-common/src/lib.rs | 2 +- crates/wasi-common/src/sys/osother.rs | 7 --- crates/wasi-common/src/sys/stdio.rs | 59 ++++++++++++++++++- crates/wasi-common/src/sys/unix/osother.rs | 16 +---- crates/wasi-common/src/sys/windows/osother.rs | 13 +--- 6 files changed, 67 insertions(+), 38 deletions(-) diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 3121cb1462..05bb0fce94 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -2,7 +2,7 @@ use crate::entry::{Entry, EntryHandle}; use crate::fdpool::FdPool; use crate::handle::Handle; use crate::sys::osdir::OsDir; -use crate::sys::osother::{OsOther, OsOtherExt}; +use crate::sys::stdio::NullDevice; use crate::sys::stdio::{Stderr, StderrExt, Stdin, StdinExt, Stdout, StdoutExt}; use crate::virtfs::{VirtualDir, VirtualDirEntry}; use crate::wasi::types; @@ -136,9 +136,9 @@ pub struct WasiCtxBuilder { impl WasiCtxBuilder { /// Builder for a new `WasiCtx`. pub fn new() -> Self { - let stdin = Some(PendingEntry::Thunk(OsOther::from_null)); - let stdout = Some(PendingEntry::Thunk(OsOther::from_null)); - let stderr = Some(PendingEntry::Thunk(OsOther::from_null)); + let stdin = Some(PendingEntry::Handle(Box::new(NullDevice::new()))); + let stdout = Some(PendingEntry::Handle(Box::new(NullDevice::new()))); + let stderr = Some(PendingEntry::Handle(Box::new(NullDevice::new()))); Self { stdin, diff --git a/crates/wasi-common/src/lib.rs b/crates/wasi-common/src/lib.rs index f9c17841a9..e6f8cef6a6 100644 --- a/crates/wasi-common/src/lib.rs +++ b/crates/wasi-common/src/lib.rs @@ -39,6 +39,6 @@ pub use ctx::{WasiCtx, WasiCtxBuilder, WasiCtxBuilderError}; pub use handle::{Handle, HandleRights}; pub use sys::osdir::OsDir; pub use sys::osfile::OsFile; -pub use sys::osother::{OsOther, OsOtherExt}; +pub use sys::osother::OsOther; pub use sys::preopen_dir; pub use virtfs::{FileContents, VirtualDirEntry}; diff --git a/crates/wasi-common/src/sys/osother.rs b/crates/wasi-common/src/sys/osother.rs index 314d91dea3..4e5c90192d 100644 --- a/crates/wasi-common/src/sys/osother.rs +++ b/crates/wasi-common/src/sys/osother.rs @@ -10,13 +10,6 @@ use std::fs::File; use std::io::{self, Read, Write}; use std::ops::Deref; -/// Extra methods for `OsOther` that are only available when configured for -/// some operating systems. -pub trait OsOtherExt { - /// Create `OsOther` as `dyn Handle` from null device. - 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 diff --git a/crates/wasi-common/src/sys/stdio.rs b/crates/wasi-common/src/sys/stdio.rs index bbd2339ed0..786643b559 100644 --- a/crates/wasi-common/src/sys/stdio.rs +++ b/crates/wasi-common/src/sys/stdio.rs @@ -20,9 +20,10 @@ 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 crate::wasi::{Errno, Result, RightsExt}; use std::any::Any; use std::cell::Cell; +use std::convert::TryInto; use std::io::{self, Read, Write}; pub(crate) trait StdinExt: Sized { @@ -174,3 +175,59 @@ impl Handle for Stderr { Ok(nwritten) } } + +#[derive(Debug, Clone)] +pub(crate) struct NullDevice { + pub(crate) rights: Cell, + pub(crate) fd_flags: Cell, +} + +impl NullDevice { + pub(crate) fn new() -> Self { + let rights = HandleRights::new( + types::Rights::character_device_base(), + types::Rights::character_device_inheriting(), + ); + let rights = Cell::new(rights); + let fd_flags = types::Fdflags::empty(); + let fd_flags = Cell::new(fd_flags); + Self { rights, fd_flags } + } +} + +impl Handle for NullDevice { + fn as_any(&self) -> &dyn Any { + self + } + fn try_clone(&self) -> io::Result> { + Ok(Box::new(self.clone())) + } + fn get_file_type(&self) -> types::Filetype { + types::Filetype::CharacterDevice + } + fn get_rights(&self) -> HandleRights { + self.rights.get() + } + fn set_rights(&self, rights: HandleRights) { + self.rights.set(rights) + } + // FdOps + fn fdstat_get(&self) -> Result { + Ok(self.fd_flags.get()) + } + fn fdstat_set_flags(&self, fdflags: types::Fdflags) -> Result<()> { + self.fd_flags.set(fdflags); + Ok(()) + } + fn read_vectored(&self, _iovs: &mut [io::IoSliceMut]) -> Result { + Ok(0) + } + fn write_vectored(&self, iovs: &[io::IoSlice]) -> Result { + let mut total_len = 0u32; + for iov in iovs { + let len: types::Size = iov.len().try_into()?; + total_len = total_len.checked_add(len).ok_or(Errno::Overflow)?; + } + Ok(total_len as usize) + } +} diff --git a/crates/wasi-common/src/sys/unix/osother.rs b/crates/wasi-common/src/sys/unix/osother.rs index 260c47db8f..19688e269b 100644 --- a/crates/wasi-common/src/sys/unix/osother.rs +++ b/crates/wasi-common/src/sys/unix/osother.rs @@ -1,10 +1,9 @@ use super::oshandle::RawOsHandle; use super::{get_file_type, get_rights}; -use crate::handle::Handle; -use crate::sys::osother::{OsOther, OsOtherExt}; +use crate::sys::osother::OsOther; use crate::wasi::types; use std::convert::TryFrom; -use std::fs::{File, OpenOptions}; +use std::fs::File; use std::io; use std::os::unix::prelude::{FromRawFd, IntoRawFd}; @@ -21,14 +20,3 @@ impl TryFrom for OsOther { Ok(Self::new(file_type, rights, handle)) } } - -impl OsOtherExt for OsOther { - 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/windows/osother.rs b/crates/wasi-common/src/sys/windows/osother.rs index 47db73217d..13fb0c0a93 100644 --- a/crates/wasi-common/src/sys/windows/osother.rs +++ b/crates/wasi-common/src/sys/windows/osother.rs @@ -1,10 +1,9 @@ use super::oshandle::RawOsHandle; use super::{get_file_type, get_rights}; -use crate::handle::Handle; -use crate::sys::osother::{OsOther, OsOtherExt}; +use crate::sys::osother::OsOther; use crate::wasi::types; use std::convert::TryFrom; -use std::fs::{File, OpenOptions}; +use std::fs::File; use std::io; use std::os::windows::prelude::{FromRawHandle, IntoRawHandle}; @@ -21,11 +20,3 @@ impl TryFrom for OsOther { Ok(Self::new(file_type, rights, handle)) } } - -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)) - } -}