From b2752735db8d296df118038c915f67295818d2b2 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Wed, 15 May 2019 15:11:29 -0700 Subject: [PATCH 1/2] avoid opening files when not needed in WASI, check for write permissions --- lib/wasi/src/state.rs | 13 +++++++ lib/wasi/src/syscalls/mod.rs | 74 +++++++++--------------------------- 2 files changed, 32 insertions(+), 55 deletions(-) diff --git a/lib/wasi/src/state.rs b/lib/wasi/src/state.rs index ff137451756..9a57e97a33f 100644 --- a/lib/wasi/src/state.rs +++ b/lib/wasi/src/state.rs @@ -451,3 +451,16 @@ pub struct WasiState<'a> { pub args: &'a [Vec], pub envs: &'a [Vec], } + +pub fn host_file_type_to_wasi_file_type(file_type: fs::FileType) -> __wasi_filetype_t { + // TODO: handle other file types + if file_type.is_dir() { + __WASI_FILETYPE_DIRECTORY + } else if file_type.is_file() { + __WASI_FILETYPE_REGULAR_FILE + } else if file_type.is_symlink() { + __WASI_FILETYPE_SYMBOLIC_LINK + } else { + __WASI_FILETYPE_UNKNOWN + } +} diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs index 69dcb998c2c..1ef936c0a91 100644 --- a/lib/wasi/src/syscalls/mod.rs +++ b/lib/wasi/src/syscalls/mod.rs @@ -8,7 +8,9 @@ pub mod windows; use self::types::*; use crate::{ ptr::{Array, WasmPtr}, - state::{Fd, InodeVal, Kind, WasiFile, WasiState, MAX_SYMLINKS}, + state::{ + host_file_type_to_wasi_file_type, Fd, InodeVal, Kind, WasiFile, WasiState, MAX_SYMLINKS, + }, ExitCode, }; use rand::{thread_rng, Rng}; @@ -796,19 +798,9 @@ pub fn fd_readdir( d_next: cur_cookie, d_ino: 0, // TODO: inode d_namlen: namlen as u32, - d_type: { - let file_type = wasi_try!(entry.file_type().map_err(|_| __WASI_EIO)); - // TODO: handle other file types - if file_type.is_dir() { - __WASI_FILETYPE_DIRECTORY - } else if file_type.is_file() { - __WASI_FILETYPE_REGULAR_FILE - } else if file_type.is_symlink() { - __WASI_FILETYPE_SYMBOLIC_LINK - } else { - __WASI_FILETYPE_UNKNOWN - } - }, + d_type: host_file_type_to_wasi_file_type(wasi_try!(entry + .file_type() + .map_err(|_| __WASI_EIO))), }; let dirent_bytes = dirent_to_le_bytes(&dirent); let upper_limit = std::cmp::min( @@ -1219,64 +1211,31 @@ pub fn path_filestat_get( } } - let final_inode = match &state.fs.inodes[inode].kind { + let stat = match &state.fs.inodes[inode].kind { Kind::Dir { path, entries, .. } => { - // TODO: fail earlier if size 0 + // read it from internal data structures if we can let last_segment = path_vec.last().unwrap(); cumulative_path.push(last_segment); if entries.contains_key(last_segment) { - entries[last_segment] + state.fs.inodes[entries[last_segment]].stat } else { - // lazily load it if we can + // otherwise read it from the host FS if !cumulative_path.exists() { return __WASI_ENOENT; } let final_path_metadata = wasi_try!(cumulative_path.metadata().map_err(|_| __WASI_EIO)); - let new_inode = if final_path_metadata.is_dir() { - debug!("Opening host directory {:#?}", &cumulative_path); - state.fs.inodes.insert(InodeVal { - stat: __wasi_filestat_t::default(), - is_preopened: false, // is this correct? - name: last_segment.clone(), - kind: Kind::Dir { - parent: Some(inode), - path: std::path::PathBuf::from(&last_segment), - entries: Default::default(), - }, - }) - } else { - debug!("Opening host file {:#?}", &cumulative_path); - let real_open_file = wasi_try!(std::fs::OpenOptions::new() - .read(true) - .write(true) - .open(&cumulative_path) - .map_err(|_| __WASI_ENOENT)); - - state.fs.inodes.insert(InodeVal { - stat: __wasi_filestat_t::default(), - is_preopened: false, // is this correct? - name: last_segment.clone(), - kind: Kind::File { - handle: WasiFile::HostFile(real_open_file), - }, - }) - }; - // reborrow to insert entry - if let Kind::Dir { entries, .. } = &mut state.fs.inodes[inode].kind { - entries.insert(last_segment.clone(), new_inode); + __wasi_filestat_t { + st_filetype: host_file_type_to_wasi_file_type(final_path_metadata.file_type()), + ..Default::default() } - new_inode } } _ => { return __WASI_ENOTDIR; } }; - - let stat = state.fs.inodes[final_inode].stat; - buf_cell.set(stat); __WASI_ESUCCESS @@ -1540,7 +1499,12 @@ pub fn path_open( // file is not a dir let real_opened_file = { let mut open_options = std::fs::OpenOptions::new(); - let open_options = open_options.read(true).write(true); + let open_options = open_options.read(true); + let open_options = if dbg!(fs_rights_base & __WASI_RIGHT_FD_WRITE) != 0 { + open_options.write(true) + } else { + open_options + }; let open_options = if o_flags & __WASI_O_CREAT != 0 { debug!( "File {:?} may be created when opened if it does not exist", From 83deae80dcd224639fb251f917bf8c9003e4ecf8 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Wed, 15 May 2019 15:16:52 -0700 Subject: [PATCH 2/2] update changelog; clean up --- CHANGELOG.md | 1 + lib/wasi/src/syscalls/mod.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb13b2d3bc5..6586edc7644 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Blocks of changes will separated by version increments. ## **[Unreleased]** +- [#449](https://github.com/wasmerio/wasmer/pull/449) Fix bugs: opening host files in filestat and opening with write permissions unconditionally in path_open - [#442](https://github.com/wasmerio/wasmer/pull/442) Misc. WASI FS fixes and implement readdir - [#440](https://github.com/wasmerio/wasmer/pull/440) Fix type mismatch between `wasmer_instance_call` and `wasmer_export_func_*_arity` functions in the runtime C API. - [#269](https://github.com/wasmerio/wasmer/pull/269) Add better runtime docs diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs index 1ef936c0a91..f313b966479 100644 --- a/lib/wasi/src/syscalls/mod.rs +++ b/lib/wasi/src/syscalls/mod.rs @@ -1500,7 +1500,7 @@ pub fn path_open( let real_opened_file = { let mut open_options = std::fs::OpenOptions::new(); let open_options = open_options.read(true); - let open_options = if dbg!(fs_rights_base & __WASI_RIGHT_FD_WRITE) != 0 { + let open_options = if fs_rights_base & __WASI_RIGHT_FD_WRITE != 0 { open_options.write(true) } else { open_options