From f4d3f08fc6314fb65993b19b7380f9cc1a74aadd Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 30 Oct 2019 22:41:43 +0100 Subject: [PATCH] Fix path_get returning ENOTDIR when base not dir (#159) * Fix path_get returning ENOTDIR when base not dir This commit makes certain adjustments to `WasiCtx` and `FdEntry` by introducing methods `get_nonvalidated_fd_entry` and `get_nonvalidated_fd_entry_mut` which only check if the `FdEntry` corresponding to the specified raw WASI fd exists in the context object but **without** automatically validating the rights. Thanks to postponing the validation until after the `FdEntry` object is extracted from the context, it is possible to check if the extracted `FdEntry` object is indeed a valid dir descriptor when processing paths in `path_get` helper fn. In essence then, this commit closes #158. * Remove potentially useless FdObject struct This commit removes `FdObject` struct which wasn't really used in our codebase, and flattens its contents into `FdEntry`. IMHO, this cleans up a fair amount of code and generally unclutters it. * Refactor and document `WasiCtx` and `FdEntry` This commit refactors `WasiCtx` struct by making the collection of `FdEntry`s stored within private, and only allowing it to be accessed via the provided set of modifier methods (push, insert, remove, etc.). Additionally, this commit documents the methods of `WasiCtx` and `FdEntry` structs for easier debugging and maintenance in the future. --- misc_testsuite | 2 +- src/ctx.rs | 57 +++--- src/fdentry.rs | 93 +++++++--- src/hostcalls_impl/fs.rs | 307 +++++++++++++++++-------------- src/hostcalls_impl/fs_helpers.rs | 13 +- src/hostcalls_impl/misc.rs | 6 +- 6 files changed, 281 insertions(+), 197 deletions(-) diff --git a/misc_testsuite b/misc_testsuite index 39cfd6cb71..f677228706 160000 --- a/misc_testsuite +++ b/misc_testsuite @@ -1 +1 @@ -Subproject commit 39cfd6cb71cfed30bc3864004b4ab161d732b3b0 +Subproject commit f677228706347c05793a7ee2dbd91629c61d043f diff --git a/src/ctx.rs b/src/ctx.rs index b75270f82e..7969cd51f5 100644 --- a/src/ctx.rs +++ b/src/ctx.rs @@ -162,7 +162,7 @@ impl WasiCtxBuilder { #[derive(Debug)] pub struct WasiCtx { - pub(crate) fds: HashMap, + fds: HashMap, pub(crate) args: Vec, pub(crate) env: Vec, } @@ -183,48 +183,28 @@ impl WasiCtx { .and_then(|ctx| ctx.build()) } + /// Check if `WasiCtx` contains the specified raw WASI `fd`. pub(crate) unsafe fn contains_fd_entry(&self, fd: host::__wasi_fd_t) -> bool { self.fds.contains_key(&fd) } - pub(crate) unsafe fn get_fd_entry( - &self, - fd: host::__wasi_fd_t, - rights_base: host::__wasi_rights_t, - rights_inheriting: host::__wasi_rights_t, - ) -> Result<&FdEntry> { - if let Some(fe) = self.fds.get(&fd) { - Self::validate_rights(fe, rights_base, rights_inheriting).and(Ok(fe)) - } else { - Err(Error::EBADF) - } + /// Get an immutable `FdEntry` corresponding to the specified raw WASI `fd`. + pub(crate) unsafe fn get_fd_entry(&self, fd: host::__wasi_fd_t) -> Result<&FdEntry> { + self.fds.get(&fd).ok_or(Error::EBADF) } + /// Get a mutable `FdEntry` corresponding to the specified raw WASI `fd`. pub(crate) unsafe fn get_fd_entry_mut( &mut self, fd: host::__wasi_fd_t, - rights_base: host::__wasi_rights_t, - rights_inheriting: host::__wasi_rights_t, ) -> Result<&mut FdEntry> { - if let Some(fe) = self.fds.get_mut(&fd) { - Self::validate_rights(fe, rights_base, rights_inheriting).and(Ok(fe)) - } else { - Err(Error::EBADF) - } - } - - fn validate_rights( - fe: &FdEntry, - rights_base: host::__wasi_rights_t, - rights_inheriting: host::__wasi_rights_t, - ) -> Result<()> { - if !fe.rights_base & rights_base != 0 || !fe.rights_inheriting & rights_inheriting != 0 { - Err(Error::ENOTCAPABLE) - } else { - Ok(()) - } + self.fds.get_mut(&fd).ok_or(Error::EBADF) } + /// Insert the specified `FdEntry` into the `WasiCtx` object. + /// + /// The `FdEntry` will automatically get another free raw WASI `fd` assigned. Note that + /// the two subsequent free raw WASI `fd`s do not have to be stored contiguously. pub(crate) fn insert_fd_entry(&mut self, fe: FdEntry) -> Result { // never insert where stdio handles usually are let mut fd = 3; @@ -238,4 +218,19 @@ impl WasiCtx { self.fds.insert(fd, fe); Ok(fd) } + + /// Insert the specified `FdEntry` with the specified raw WASI `fd` key into the `WasiCtx` + /// object. + pub(crate) fn insert_fd_entry_at( + &mut self, + fd: host::__wasi_fd_t, + fe: FdEntry, + ) -> Option { + self.fds.insert(fd, fe) + } + + /// Remove `FdEntry` corresponding to the specified raw WASI `fd` from the `WasiCtx` object. + pub(crate) fn remove_fd_entry(&mut self, fd: host::__wasi_fd_t) -> Result { + self.fds.remove(&fd).ok_or(Error::EBADF) + } } diff --git a/src/fdentry.rs b/src/fdentry.rs index cdfbe4f06d..78527e3d21 100644 --- a/src/fdentry.rs +++ b/src/fdentry.rs @@ -58,29 +58,30 @@ impl Descriptor { } } -#[derive(Debug)] -pub(crate) struct FdObject { - pub(crate) file_type: host::__wasi_filetype_t, - pub(crate) descriptor: Descriptor, - // TODO: directories -} - +/// 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 `FdEntry::as_descriptor` and +/// `FdEntry::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 FdEntry { - pub(crate) fd_object: FdObject, + pub(crate) file_type: host::__wasi_filetype_t, + descriptor: Descriptor, pub(crate) rights_base: host::__wasi_rights_t, pub(crate) rights_inheriting: host::__wasi_rights_t, pub(crate) preopen_path: Option, + // TODO: directories } impl FdEntry { pub(crate) fn from(file: fs::File) -> Result { unsafe { determine_type_and_access_rights(&file) }.map( |(file_type, rights_base, rights_inheriting)| Self { - fd_object: FdObject { - file_type, - descriptor: Descriptor::OsFile(OsFile::from(file)), - }, + file_type, + descriptor: Descriptor::OsFile(OsFile::from(file)), rights_base, rights_inheriting, preopen_path: None, @@ -95,10 +96,8 @@ impl FdEntry { pub(crate) fn duplicate_stdin() -> Result { unsafe { determine_type_and_access_rights(&io::stdin()) }.map( |(file_type, rights_base, rights_inheriting)| Self { - fd_object: FdObject { - file_type, - descriptor: Descriptor::Stdin, - }, + file_type, + descriptor: Descriptor::Stdin, rights_base, rights_inheriting, preopen_path: None, @@ -109,10 +108,8 @@ impl FdEntry { pub(crate) fn duplicate_stdout() -> Result { unsafe { determine_type_and_access_rights(&io::stdout()) }.map( |(file_type, rights_base, rights_inheriting)| Self { - fd_object: FdObject { - file_type, - descriptor: Descriptor::Stdout, - }, + file_type, + descriptor: Descriptor::Stdout, rights_base, rights_inheriting, preopen_path: None, @@ -123,14 +120,62 @@ impl FdEntry { pub(crate) fn duplicate_stderr() -> Result { unsafe { determine_type_and_access_rights(&io::stderr()) }.map( |(file_type, rights_base, rights_inheriting)| Self { - fd_object: FdObject { - file_type, - descriptor: Descriptor::Stderr, - }, + file_type, + descriptor: Descriptor::Stderr, rights_base, rights_inheriting, preopen_path: None, }, ) } + + /// Convert this `FdEntry` into a host `Descriptor` object provided the specified + /// `rights_base` and `rights_inheriting` rights are set on this `FdEntry` object. + /// + /// The `FdEntry` 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 `FdEntry`. The check is performed using + /// `FdEntry::validate_rights` method. If the check fails, `Error::ENOTCAPABLE` is returned. + pub(crate) fn as_descriptor( + &self, + rights_base: host::__wasi_rights_t, + rights_inheriting: host::__wasi_rights_t, + ) -> Result<&Descriptor> { + self.validate_rights(rights_base, rights_inheriting)?; + Ok(&self.descriptor) + } + + /// Convert this `FdEntry` into a mutable host `Descriptor` object provided the specified + /// `rights_base` and `rights_inheriting` rights are set on this `FdEntry` object. + /// + /// The `FdEntry` 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 `FdEntry`. The check is performed using + /// `FdEntry::validate_rights` method. If the check fails, `Error::ENOTCAPABLE` is returned. + pub(crate) fn as_descriptor_mut( + &mut self, + rights_base: host::__wasi_rights_t, + rights_inheriting: host::__wasi_rights_t, + ) -> Result<&mut Descriptor> { + self.validate_rights(rights_base, rights_inheriting)?; + Ok(&mut self.descriptor) + } + + /// Check if this `FdEntry` object satisfies the specified base rights `rights_base`, and + /// inheriting rights `rights_inheriting`; i.e., if rights attached to this `FdEntry` object + /// are a superset. + /// + /// Upon unsuccessful check, `Error::ENOTCAPABLE` is returned. + fn validate_rights( + &self, + rights_base: host::__wasi_rights_t, + rights_inheriting: host::__wasi_rights_t, + ) -> Result<()> { + if !self.rights_base & rights_base != 0 || !self.rights_inheriting & rights_inheriting != 0 + { + Err(Error::ENOTCAPABLE) + } else { + Ok(()) + } + } } diff --git a/src/hostcalls_impl/fs.rs b/src/hostcalls_impl/fs.rs index 8eae1f25ac..e3413ccd2f 100644 --- a/src/hostcalls_impl/fs.rs +++ b/src/hostcalls_impl/fs.rs @@ -17,14 +17,14 @@ pub(crate) unsafe fn fd_close(wasi_ctx: &mut WasiCtx, fd: wasm32::__wasi_fd_t) - trace!("fd_close(fd={:?})", fd); let fd = dec_fd(fd); - if let Some(fdent) = wasi_ctx.fds.get(&fd) { + if let Ok(fe) = wasi_ctx.get_fd_entry(fd) { // can't close preopened files - if fdent.preopen_path.is_some() { + if fe.preopen_path.is_some() { return Err(Error::ENOTSUP); } } - wasi_ctx.fds.remove(&fd).ok_or(Error::EBADF)?; + wasi_ctx.remove_fd_entry(fd)?; Ok(()) } @@ -33,8 +33,9 @@ pub(crate) unsafe fn fd_datasync(wasi_ctx: &WasiCtx, fd: wasm32::__wasi_fd_t) -> let fd = dec_fd(fd); let fd = wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_FD_DATASYNC, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + .get_fd_entry(fd)? + .as_descriptor(host::__WASI_RIGHT_FD_DATASYNC, 0)? + .as_file()?; fd.sync_data().map_err(Into::into) } @@ -59,8 +60,9 @@ pub(crate) unsafe fn fd_pread( let fd = dec_fd(fd); let fd = wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_FD_READ, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + .get_fd_entry(fd)? + .as_descriptor(host::__WASI_RIGHT_FD_READ, 0)? + .as_file()?; let iovs = dec_iovec_slice(memory, iovs_ptr, iovs_len)?; @@ -109,8 +111,9 @@ pub(crate) unsafe fn fd_pwrite( let fd = dec_fd(fd); let fd = wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_FD_READ, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + .get_fd_entry(fd)? + .as_descriptor(host::__WASI_RIGHT_FD_READ, 0)? + .as_file()?; let iovs = dec_iovec_slice(memory, iovs_ptr, iovs_len)?; let offset = dec_filesize(offset); @@ -148,15 +151,17 @@ pub(crate) unsafe fn fd_read( nread ); - let fd = dec_fd(fd); let mut iovs = dec_iovec_slice(memory, iovs_ptr, iovs_len)?; - let fe = wasi_ctx.get_fd_entry_mut(fd, host::__WASI_RIGHT_FD_READ, 0)?; let mut iovs: Vec = iovs .iter_mut() .map(|vec| host::iovec_to_host_mut(vec)) .collect(); + let fd = dec_fd(fd); - let maybe_host_nread = match &mut fe.fd_object.descriptor { + let maybe_host_nread = match wasi_ctx + .get_fd_entry_mut(fd)? + .as_descriptor_mut(host::__WASI_RIGHT_FD_READ, 0)? + { Descriptor::OsFile(file) => file.read_vectored(&mut iovs), Descriptor::Stdin => io::stdin().lock().read_vectored(&mut iovs), _ => return Err(Error::EBADF), @@ -183,29 +188,29 @@ pub(crate) unsafe fn fd_renumber( return Err(Error::EBADF); } + let from_fe = wasi_ctx.get_fd_entry(from)?; + let to_fe = wasi_ctx.get_fd_entry(to)?; + // Don't allow renumbering over a pre-opened resource. // TODO: Eventually, we do want to permit this, once libpreopen in // userspace is capable of removing entries from its tables as well. - if wasi_ctx.fds[&from].preopen_path.is_some() || wasi_ctx.fds[&to].preopen_path.is_some() { + if from_fe.preopen_path.is_some() || to_fe.preopen_path.is_some() { return Err(Error::ENOTSUP); } // check if stdio fds // TODO should we renumber stdio fds? - if !wasi_ctx.fds[&from].fd_object.descriptor.is_file() - || !wasi_ctx.fds[&to].fd_object.descriptor.is_file() - { + if !from_fe.as_descriptor(0, 0)?.is_file() || !to_fe.as_descriptor(0, 0)?.is_file() { return Err(Error::EBADF); } - let fe_from_dup = wasi_ctx.fds[&from] - .fd_object - .descriptor + let fe_from_dup = from_fe + .as_descriptor(0, 0)? .as_file() .and_then(|file| FdEntry::duplicate(file))?; - wasi_ctx.fds.insert(to, fe_from_dup); - wasi_ctx.fds.remove(&from); + wasi_ctx.insert_fd_entry_at(to, fe_from_dup); + wasi_ctx.remove_fd_entry(from)?; Ok(()) } @@ -236,8 +241,9 @@ pub(crate) unsafe fn fd_seek( host::__WASI_RIGHT_FD_SEEK | host::__WASI_RIGHT_FD_TELL }; let fd = wasi_ctx - .get_fd_entry_mut(fd, rights, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file_mut())?; + .get_fd_entry_mut(fd)? + .as_descriptor_mut(rights, 0)? + .as_file_mut()?; let pos = match whence { host::__WASI_WHENCE_CUR => SeekFrom::Current(offset), @@ -262,8 +268,9 @@ pub(crate) unsafe fn fd_tell( let fd = dec_fd(fd); let fd = wasi_ctx - .get_fd_entry_mut(fd, host::__WASI_RIGHT_FD_TELL, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file_mut())?; + .get_fd_entry_mut(fd)? + .as_descriptor_mut(host::__WASI_RIGHT_FD_TELL, 0)? + .as_file_mut()?; let host_offset = fd.seek(SeekFrom::Current(0))?; @@ -282,12 +289,12 @@ pub(crate) unsafe fn fd_fdstat_get( let mut fdstat = dec_fdstat_byref(memory, fdstat_ptr)?; let fd = dec_fd(fd); - let fe = wasi_ctx.get_fd_entry(fd, 0, 0)?; - let fd = fe.fd_object.descriptor.as_file()?; + let wasi_fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?; - let fs_flags = hostcalls_impl::fd_fdstat_get(fd)?; + let fs_flags = hostcalls_impl::fd_fdstat_get(wasi_fd)?; - fdstat.fs_filetype = fe.fd_object.file_type; + let fe = wasi_ctx.get_fd_entry(fd)?; + fdstat.fs_filetype = fe.file_type; fdstat.fs_rights_base = fe.rights_base; fdstat.fs_rights_inheriting = fe.rights_inheriting; fdstat.fs_flags = fs_flags; @@ -306,9 +313,7 @@ pub(crate) unsafe fn fd_fdstat_set_flags( let fdflags = dec_fdflags(fdflags); let fd = dec_fd(fd); - let fd = wasi_ctx - .get_fd_entry(fd, 0, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + let fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?; hostcalls_impl::fd_fdstat_set_flags(fd, fdflags) } @@ -327,7 +332,7 @@ pub(crate) unsafe fn fd_fdstat_set_rights( ); let fd = dec_fd(fd); - let fe = wasi_ctx.fds.get_mut(&fd).ok_or(Error::EBADF)?; + let fe = &mut wasi_ctx.get_fd_entry_mut(fd)?; if fe.rights_base & fs_rights_base != fs_rights_base || fe.rights_inheriting & fs_rights_inheriting != fs_rights_inheriting @@ -345,8 +350,9 @@ pub(crate) unsafe fn fd_sync(wasi_ctx: &WasiCtx, fd: wasm32::__wasi_fd_t) -> Res let fd = dec_fd(fd); let fd = wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_FD_SYNC, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + .get_fd_entry(fd)? + .as_descriptor(host::__WASI_RIGHT_FD_SYNC, 0)? + .as_file()?; fd.sync_all().map_err(Into::into) } @@ -368,11 +374,13 @@ pub(crate) unsafe fn fd_write( let fd = dec_fd(fd); let iovs = dec_iovec_slice(memory, iovs_ptr, iovs_len)?; - let fe = wasi_ctx.get_fd_entry_mut(fd, host::__WASI_RIGHT_FD_WRITE, 0)?; let iovs: Vec = iovs.iter().map(|vec| host::iovec_to_host(vec)).collect(); // perform unbuffered writes - let host_nwritten = match &mut fe.fd_object.descriptor { + let host_nwritten = match wasi_ctx + .get_fd_entry_mut(fd)? + .as_descriptor_mut(host::__WASI_RIGHT_FD_WRITE, 0)? + { Descriptor::OsFile(file) => file.write_vectored(&iovs)?, Descriptor::Stdin => return Err(Error::EBADF), Descriptor::Stdout => { @@ -411,8 +419,9 @@ pub(crate) unsafe fn fd_advise( let offset = dec_filesize(offset); let len = dec_filesize(len); let fd = wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_FD_ADVISE, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + .get_fd_entry(fd)? + .as_descriptor(host::__WASI_RIGHT_FD_ADVISE, 0)? + .as_file()?; hostcalls_impl::fd_advise(fd, advice, offset, len) } @@ -429,8 +438,9 @@ pub(crate) unsafe fn fd_allocate( let offset = dec_filesize(offset); let len = dec_filesize(len); let fd = wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_FD_ALLOCATE, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + .get_fd_entry(fd)? + .as_descriptor(host::__WASI_RIGHT_FD_ALLOCATE, 0)? + .as_file()?; let metadata = fd.metadata()?; @@ -468,8 +478,8 @@ pub(crate) unsafe fn path_create_directory( trace!(" | (path_ptr,path_len)='{}'", path); let rights = host::__WASI_RIGHT_PATH_OPEN | host::__WASI_RIGHT_PATH_CREATE_DIRECTORY; - let fo = &wasi_ctx.get_fd_entry(dirfd, rights, 0)?.fd_object; - let resolved = path_get(fo, 0, path, false)?; + let fe = &wasi_ctx.get_fd_entry(dirfd)?; + let resolved = path_get(fe, rights, 0, 0, path, false)?; hostcalls_impl::path_create_directory(resolved) } @@ -506,14 +516,24 @@ pub(crate) unsafe fn path_link( trace!(" | (old_path_ptr,old_path_len)='{}'", old_path); trace!(" | (new_path_ptr,new_path_len)='{}'", new_path); - let old_fo = &wasi_ctx - .get_fd_entry(old_dirfd, host::__WASI_RIGHT_PATH_LINK_SOURCE, 0)? - .fd_object; - let new_fo = &wasi_ctx - .get_fd_entry(new_dirfd, host::__WASI_RIGHT_PATH_LINK_TARGET, 0)? - .fd_object; - let resolved_old = path_get(old_fo, 0, old_path, false)?; - let resolved_new = path_get(new_fo, 0, new_path, false)?; + let old_fe = &wasi_ctx.get_fd_entry(old_dirfd)?; + let new_fe = &wasi_ctx.get_fd_entry(new_dirfd)?; + let resolved_old = path_get( + old_fe, + host::__WASI_RIGHT_PATH_LINK_SOURCE, + 0, + 0, + old_path, + false, + )?; + let resolved_new = path_get( + new_fe, + host::__WASI_RIGHT_PATH_LINK_TARGET, + 0, + 0, + new_path, + false, + )?; hostcalls_impl::path_link(resolved_old, resolved_new) } @@ -560,10 +580,15 @@ pub(crate) unsafe fn path_open( let (needed_base, needed_inheriting) = path_open_rights(fs_rights_base, fs_rights_inheriting, oflags, fs_flags); - let fo = &wasi_ctx - .get_fd_entry(dirfd, needed_base, needed_inheriting)? - .fd_object; - let resolved = path_get(fo, dirflags, path, oflags & host::__WASI_O_CREAT != 0)?; + let fe = &wasi_ctx.get_fd_entry(dirfd)?; + let resolved = path_get( + fe, + needed_base, + needed_inheriting, + dirflags, + path, + oflags & host::__WASI_O_CREAT != 0, + )?; // which open mode do we need? let read = fs_rights_base & (host::__WASI_RIGHT_FD_READ | host::__WASI_RIGHT_FD_READDIR) != 0; @@ -610,8 +635,9 @@ pub(crate) unsafe fn fd_readdir( let fd = dec_fd(fd); let file = wasi_ctx - .get_fd_entry_mut(fd, host::__WASI_RIGHT_FD_READDIR, 0) - .and_then(|entry| entry.fd_object.descriptor.as_file_mut())?; + .get_fd_entry_mut(fd)? + .as_descriptor_mut(host::__WASI_RIGHT_FD_READDIR, 0)? + .as_file_mut()?; let host_buf = dec_slice_of_mut::(memory, buf, buf_len)?; trace!(" | (buf,buf_len)={:?}", host_buf); @@ -652,10 +678,8 @@ pub(crate) unsafe fn path_readlink( trace!(" | (path_ptr,path_len)='{}'", &path); - let fo = &wasi_ctx - .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_READLINK, 0)? - .fd_object; - let resolved = path_get(fo, 0, &path, false)?; + let fe = &wasi_ctx.get_fd_entry(dirfd)?; + let resolved = path_get(fe, host::__WASI_RIGHT_PATH_READLINK, 0, 0, &path, false)?; let mut buf = dec_slice_of_mut::(memory, buf_ptr, buf_len)?; @@ -697,14 +721,24 @@ pub(crate) unsafe fn path_rename( trace!(" | (old_path_ptr,old_path_len)='{}'", old_path); trace!(" | (new_path_ptr,new_path_len)='{}'", new_path); - let old_fo = &wasi_ctx - .get_fd_entry(old_dirfd, host::__WASI_RIGHT_PATH_RENAME_SOURCE, 0)? - .fd_object; - let new_fo = &wasi_ctx - .get_fd_entry(new_dirfd, host::__WASI_RIGHT_PATH_RENAME_TARGET, 0)? - .fd_object; - let resolved_old = path_get(old_fo, 0, old_path, true)?; - let resolved_new = path_get(new_fo, 0, new_path, true)?; + let old_fe = &wasi_ctx.get_fd_entry(old_dirfd)?; + let new_fe = &wasi_ctx.get_fd_entry(new_dirfd)?; + let resolved_old = path_get( + old_fe, + host::__WASI_RIGHT_PATH_RENAME_SOURCE, + 0, + 0, + old_path, + true, + )?; + let resolved_new = path_get( + new_fe, + host::__WASI_RIGHT_PATH_RENAME_TARGET, + 0, + 0, + new_path, + true, + )?; log::debug!("path_rename resolved_old={:?}", resolved_old); log::debug!("path_rename resolved_new={:?}", resolved_new); @@ -725,9 +759,7 @@ pub(crate) unsafe fn fd_filestat_get( ); let fd = dec_fd(fd); - let fd = wasi_ctx - .get_fd_entry(fd, 0, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + let fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?; let host_filestat = hostcalls_impl::fd_filestat_get_impl(fd)?; @@ -753,8 +785,9 @@ pub(crate) unsafe fn fd_filestat_set_times( let fd = dec_fd(fd); let fd = wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_FD_FILESTAT_SET_TIMES, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + .get_fd_entry(fd)? + .as_descriptor(host::__WASI_RIGHT_FD_FILESTAT_SET_TIMES, 0)? + .as_file()?; let st_atim = dec_timestamp(st_atim); let st_mtim = dec_timestamp(st_mtim); @@ -808,8 +841,9 @@ pub(crate) unsafe fn fd_filestat_set_size( let fd = dec_fd(fd); let fd = wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_FD_FILESTAT_SET_SIZE, 0) - .and_then(|fe| fe.fd_object.descriptor.as_file())?; + .get_fd_entry(fd)? + .as_descriptor(host::__WASI_RIGHT_FD_FILESTAT_SET_SIZE, 0)? + .as_file()?; let st_size = dec_filesize(st_size); // This check will be unnecessary when rust-lang/rust#63326 is fixed @@ -843,10 +877,15 @@ pub(crate) unsafe fn path_filestat_get( trace!(" | (path_ptr,path_len)='{}'", path); - let fo = &wasi_ctx - .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_FILESTAT_GET, 0)? - .fd_object; - let resolved = path_get(fo, dirflags, path, false)?; + let fe = &wasi_ctx.get_fd_entry(dirfd)?; + let resolved = path_get( + fe, + host::__WASI_RIGHT_PATH_FILESTAT_GET, + 0, + dirflags, + path, + false, + )?; let host_filestat = hostcalls_impl::path_filestat_get(resolved, dirflags)?; trace!(" | *filestat_ptr={:?}", host_filestat); @@ -885,10 +924,15 @@ pub(crate) unsafe fn path_filestat_set_times( let st_mtim = dec_timestamp(st_mtim); let fst_flags = dec_fstflags(fst_flags); - let fo = &wasi_ctx - .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_FILESTAT_SET_TIMES, 0)? - .fd_object; - let resolved = path_get(fo, dirflags, path, false)?; + let fe = &wasi_ctx.get_fd_entry(dirfd)?; + let resolved = path_get( + fe, + host::__WASI_RIGHT_PATH_FILESTAT_SET_TIMES, + 0, + dirflags, + path, + false, + )?; hostcalls_impl::path_filestat_set_times(resolved, dirflags, st_atim, st_mtim, fst_flags) } @@ -920,10 +964,8 @@ pub(crate) unsafe fn path_symlink( trace!(" | (old_path_ptr,old_path_len)='{}'", old_path); trace!(" | (new_path_ptr,new_path_len)='{}'", new_path); - let fo = &wasi_ctx - .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_SYMLINK, 0)? - .fd_object; - let resolved_new = path_get(fo, 0, new_path, true)?; + let fe = &wasi_ctx.get_fd_entry(dirfd)?; + let resolved_new = path_get(fe, host::__WASI_RIGHT_PATH_SYMLINK, 0, 0, new_path, true)?; hostcalls_impl::path_symlink(old_path, resolved_new) } @@ -947,10 +989,8 @@ pub(crate) unsafe fn path_unlink_file( trace!(" | (path_ptr,path_len)='{}'", path); - let fo = &wasi_ctx - .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_UNLINK_FILE, 0)? - .fd_object; - let resolved = path_get(fo, 0, path, false)?; + let fe = &wasi_ctx.get_fd_entry(dirfd)?; + let resolved = path_get(fe, host::__WASI_RIGHT_PATH_UNLINK_FILE, 0, 0, path, false)?; hostcalls_impl::path_unlink_file(resolved) } @@ -974,10 +1014,15 @@ pub(crate) unsafe fn path_remove_directory( trace!(" | (path_ptr,path_len)='{}'", path); - let fo = &wasi_ctx - .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_REMOVE_DIRECTORY, 0)? - .fd_object; - let resolved = path_get(fo, 0, path, true)?; + let fe = &wasi_ctx.get_fd_entry(dirfd)?; + let resolved = path_get( + fe, + host::__WASI_RIGHT_PATH_REMOVE_DIRECTORY, + 0, + 0, + path, + true, + )?; log::debug!("path_remove_directory resolved={:?}", resolved); @@ -997,30 +1042,27 @@ pub(crate) unsafe fn fd_prestat_get( ); let fd = dec_fd(fd); - // TODO: is this the correct right for this? - wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_PATH_OPEN, 0) - .and_then(|fe| { - let po_path = fe.preopen_path.as_ref().ok_or(Error::ENOTSUP)?; - if fe.fd_object.file_type != host::__WASI_FILETYPE_DIRECTORY { - return Err(Error::ENOTDIR); - } - - let path = host_impl::path_from_host(po_path.as_os_str())?; - - enc_prestat_byref( - memory, - prestat_ptr, - host::__wasi_prestat_t { - pr_type: host::__WASI_PREOPENTYPE_DIR, - u: host::__wasi_prestat_t___wasi_prestat_u { - dir: host::__wasi_prestat_t___wasi_prestat_u___wasi_prestat_u_dir_t { - pr_name_len: path.len(), - }, - }, + // TODO: should we validate any rights here? + let fe = &wasi_ctx.get_fd_entry(fd)?; + let po_path = fe.preopen_path.as_ref().ok_or(Error::ENOTSUP)?; + if fe.file_type != host::__WASI_FILETYPE_DIRECTORY { + return Err(Error::ENOTDIR); + } + + let path = host_impl::path_from_host(po_path.as_os_str())?; + + enc_prestat_byref( + memory, + prestat_ptr, + host::__wasi_prestat_t { + pr_type: host::__WASI_PREOPENTYPE_DIR, + u: host::__wasi_prestat_t___wasi_prestat_u { + dir: host::__wasi_prestat_t___wasi_prestat_u___wasi_prestat_u_dir_t { + pr_name_len: path.len(), }, - ) - }) + }, + }, + ) } pub(crate) unsafe fn fd_prestat_dir_name( @@ -1038,25 +1080,22 @@ pub(crate) unsafe fn fd_prestat_dir_name( ); let fd = dec_fd(fd); + // TODO: should we validate any rights here? + let fe = &wasi_ctx.get_fd_entry(fd)?; + let po_path = fe.preopen_path.as_ref().ok_or(Error::ENOTSUP)?; + if fe.file_type != host::__WASI_FILETYPE_DIRECTORY { + return Err(Error::ENOTDIR); + } - wasi_ctx - .get_fd_entry(fd, host::__WASI_RIGHT_PATH_OPEN, 0) - .and_then(|fe| { - let po_path = fe.preopen_path.as_ref().ok_or(Error::ENOTSUP)?; - if fe.fd_object.file_type != host::__WASI_FILETYPE_DIRECTORY { - return Err(Error::ENOTDIR); - } - - let path = host_impl::path_from_host(po_path.as_os_str())?; + let path = host_impl::path_from_host(po_path.as_os_str())?; - if path.len() > dec_usize(path_len) { - return Err(Error::ENAMETOOLONG); - } + if path.len() > dec_usize(path_len) { + return Err(Error::ENAMETOOLONG); + } - trace!(" | (path_ptr,path_len)='{}'", path); + trace!(" | (path_ptr,path_len)='{}'", path); - enc_slice_of(memory, path.as_bytes(), path_ptr) - }) + enc_slice_of(memory, path.as_bytes(), path_ptr) } #[allow(dead_code)] // trouble with sockets diff --git a/src/hostcalls_impl/fs_helpers.rs b/src/hostcalls_impl/fs_helpers.rs index 7cd0aa9fb6..e1da33b47f 100644 --- a/src/hostcalls_impl/fs_helpers.rs +++ b/src/hostcalls_impl/fs_helpers.rs @@ -2,7 +2,7 @@ use crate::helpers::str_to_cstring; use crate::sys::host_impl; use crate::sys::hostcalls_impl::fs_helpers::*; -use crate::{fdentry::FdObject, host, Error, Result}; +use crate::{fdentry::FdEntry, host, Error, Result}; use std::ffi::CString; use std::fs::File; use std::path::{Component, Path}; @@ -31,7 +31,9 @@ impl PathGet { /// /// This is a workaround for not having Capsicum support in the OS. pub(crate) fn path_get( - fo: &FdObject, + fe: &FdEntry, + rights_base: host::__wasi_rights_t, + rights_inheriting: host::__wasi_rights_t, dirflags: host::__wasi_lookupflags_t, path: &str, needs_final_component: bool, @@ -43,12 +45,15 @@ pub(crate) fn path_get( return Err(Error::EILSEQ); } - if fo.file_type != host::__WASI_FILETYPE_DIRECTORY { + if fe.file_type != host::__WASI_FILETYPE_DIRECTORY { // if `dirfd` doesn't refer to a directory, return `ENOTDIR`. return Err(Error::ENOTDIR); } - let dirfd = fo.descriptor.as_file()?.try_clone()?; + let dirfd = fe + .as_descriptor(rights_base, rights_inheriting)? + .as_file()? + .try_clone()?; // 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/src/hostcalls_impl/misc.rs b/src/hostcalls_impl/misc.rs index c77970e5f6..b6fdf04319 100644 --- a/src/hostcalls_impl/misc.rs +++ b/src/hostcalls_impl/misc.rs @@ -255,8 +255,8 @@ pub(crate) fn poll_oneoff( match unsafe { wasi_ctx - .get_fd_entry(wasi_fd, rights, 0) - .map(|fe| &fe.fd_object.descriptor) + .get_fd_entry(wasi_fd) + .and_then(|fe| fe.as_descriptor(rights, 0)) } { Ok(descriptor) => fd_events.push(FdEventData { descriptor, @@ -281,7 +281,7 @@ pub(crate) fn poll_oneoff( enc_event(event); events_count += 1; } - } + }; } _ => unreachable!(), }