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

@@ -1 +1 @@
-Subproject commit 39cfd6cb71cfed30bc3864004b4ab161d732b3b0
+Subproject commit f677228706347c05793a7ee2dbd91629c61d043f

src/ctx.rs

@@ -162,7 +162,7 @@ impl WasiCtxBuilder {
 
 #[derive(Debug)]
 pub struct WasiCtx {
-    pub(crate) fds: HashMap<host::__wasi_fd_t, FdEntry>,
+    fds: HashMap<host::__wasi_fd_t, FdEntry>,
     pub(crate) args: Vec<CString>,
     pub(crate) env: Vec<CString>,
 }
@@ -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(
+    /// Get an immutable `FdEntry` corresponding to the specified raw WASI `fd`.
-        &self,
+    pub(crate) unsafe fn get_fd_entry(&self, fd: host::__wasi_fd_t) -> Result<&FdEntry> {
-        fd: host::__wasi_fd_t,
+        self.fds.get(&fd).ok_or(Error::EBADF)
-        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 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.fds.get_mut(&fd).ok_or(Error::EBADF)
-            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(())
-        }
     }
 
+    /// 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<host::__wasi_fd_t> {
         // 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<FdEntry> {
+        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<FdEntry> {
+        self.fds.remove(&fd).ok_or(Error::EBADF)
+    }
 }

src/fdentry.rs

@@ -58,29 +58,30 @@ impl Descriptor {
     }
 }
 
-#[derive(Debug)]
+/// An abstraction struct serving as a wrapper for a host `Descriptor` object which requires
-pub(crate) struct FdObject {
+/// certain base rights `rights_base` and inheriting rights `rights_inheriting` in order to be
-    pub(crate) file_type: host::__wasi_filetype_t,
+/// accessed correctly.
-    pub(crate) descriptor: Descriptor,
+///
-    // TODO: directories
+/// 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<PathBuf>,
+    // TODO: directories
 }
 
 impl FdEntry {
     pub(crate) fn from(file: fs::File) -> Result<Self> {
         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)),
-                },
                 rights_base,
                 rights_inheriting,
                 preopen_path: None,
@@ -95,10 +96,8 @@ impl FdEntry {
     pub(crate) fn duplicate_stdin() -> Result<Self> {
         unsafe { determine_type_and_access_rights(&io::stdin()) }.map(
             |(file_type, rights_base, rights_inheriting)| Self {
-                fd_object: FdObject {
                 file_type,
                 descriptor: Descriptor::Stdin,
-                },
                 rights_base,
                 rights_inheriting,
                 preopen_path: None,
@@ -109,10 +108,8 @@ impl FdEntry {
     pub(crate) fn duplicate_stdout() -> Result<Self> {
         unsafe { determine_type_and_access_rights(&io::stdout()) }.map(
             |(file_type, rights_base, rights_inheriting)| Self {
-                fd_object: FdObject {
                 file_type,
                 descriptor: Descriptor::Stdout,
-                },
                 rights_base,
                 rights_inheriting,
                 preopen_path: None,
@@ -123,14 +120,62 @@ impl FdEntry {
     pub(crate) fn duplicate_stderr() -> Result<Self> {
         unsafe { determine_type_and_access_rights(&io::stderr()) }.map(
             |(file_type, rights_base, rights_inheriting)| Self {
-                fd_object: FdObject {
                 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(())
+        }
+    }
 }

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)
+        .get_fd_entry(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file())?;
+        .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)
+        .get_fd_entry(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file())?;
+        .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)
+        .get_fd_entry(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file())?;
+        .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<io::IoSliceMut> = 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()
+    if !from_fe.as_descriptor(0, 0)?.is_file() || !to_fe.as_descriptor(0, 0)?.is_file() {
-        || !wasi_ctx.fds[&to].fd_object.descriptor.is_file()
-    {
         return Err(Error::EBADF);
     }
 
-    let fe_from_dup = wasi_ctx.fds[&from]
+    let fe_from_dup = from_fe
-        .fd_object
+        .as_descriptor(0, 0)?
-        .descriptor
         .as_file()
         .and_then(|file| FdEntry::duplicate(file))?;
 
-    wasi_ctx.fds.insert(to, fe_from_dup);
+    wasi_ctx.insert_fd_entry_at(to, fe_from_dup);
-    wasi_ctx.fds.remove(&from);
+    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)
+        .get_fd_entry_mut(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file_mut())?;
+        .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)
+        .get_fd_entry_mut(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file_mut())?;
+        .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 wasi_fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?;
-    let fd = fe.fd_object.descriptor.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
+    let fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?;
-        .get_fd_entry(fd, 0, 0)
-        .and_then(|fe| fe.fd_object.descriptor.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)
+        .get_fd_entry(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file())?;
+        .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<io::IoSlice> = 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)
+        .get_fd_entry(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file())?;
+        .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)
+        .get_fd_entry(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file())?;
+        .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 fe = &wasi_ctx.get_fd_entry(dirfd)?;
-    let resolved = path_get(fo, 0, path, false)?;
+    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
+    let old_fe = &wasi_ctx.get_fd_entry(old_dirfd)?;
-        .get_fd_entry(old_dirfd, host::__WASI_RIGHT_PATH_LINK_SOURCE, 0)?
+    let new_fe = &wasi_ctx.get_fd_entry(new_dirfd)?;
-        .fd_object;
+    let resolved_old = path_get(
-    let new_fo = &wasi_ctx
+        old_fe,
-        .get_fd_entry(new_dirfd, host::__WASI_RIGHT_PATH_LINK_TARGET, 0)?
+        host::__WASI_RIGHT_PATH_LINK_SOURCE,
-        .fd_object;
+        0,
-    let resolved_old = path_get(old_fo, 0, old_path, false)?;
+        0,
-    let resolved_new = path_get(new_fo, 0, new_path, false)?;
+        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
+    let fe = &wasi_ctx.get_fd_entry(dirfd)?;
-        .get_fd_entry(dirfd, needed_base, needed_inheriting)?
+    let resolved = path_get(
-        .fd_object;
+        fe,
-    let resolved = path_get(fo, dirflags, path, oflags & host::__WASI_O_CREAT != 0)?;
+        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)
+        .get_fd_entry_mut(fd)?
-        .and_then(|entry| entry.fd_object.descriptor.as_file_mut())?;
+        .as_descriptor_mut(host::__WASI_RIGHT_FD_READDIR, 0)?
+        .as_file_mut()?;
     let host_buf = dec_slice_of_mut::<u8>(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
+    let fe = &wasi_ctx.get_fd_entry(dirfd)?;
-        .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_READLINK, 0)?
+    let resolved = path_get(fe, host::__WASI_RIGHT_PATH_READLINK, 0, 0, &path, false)?;
-        .fd_object;
-    let resolved = path_get(fo, 0, &path, false)?;
 
     let mut buf = dec_slice_of_mut::<u8>(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
+    let old_fe = &wasi_ctx.get_fd_entry(old_dirfd)?;
-        .get_fd_entry(old_dirfd, host::__WASI_RIGHT_PATH_RENAME_SOURCE, 0)?
+    let new_fe = &wasi_ctx.get_fd_entry(new_dirfd)?;
-        .fd_object;
+    let resolved_old = path_get(
-    let new_fo = &wasi_ctx
+        old_fe,
-        .get_fd_entry(new_dirfd, host::__WASI_RIGHT_PATH_RENAME_TARGET, 0)?
+        host::__WASI_RIGHT_PATH_RENAME_SOURCE,
-        .fd_object;
+        0,
-    let resolved_old = path_get(old_fo, 0, old_path, true)?;
+        0,
-    let resolved_new = path_get(new_fo, 0, new_path, true)?;
+        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
+    let fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?;
-        .get_fd_entry(fd, 0, 0)
-        .and_then(|fe| fe.fd_object.descriptor.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)
+        .get_fd_entry(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file())?;
+        .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)
+        .get_fd_entry(fd)?
-        .and_then(|fe| fe.fd_object.descriptor.as_file())?;
+        .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
+    let fe = &wasi_ctx.get_fd_entry(dirfd)?;
-        .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_FILESTAT_GET, 0)?
+    let resolved = path_get(
-        .fd_object;
+        fe,
-    let resolved = path_get(fo, dirflags, path, false)?;
+        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
+    let fe = &wasi_ctx.get_fd_entry(dirfd)?;
-        .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_FILESTAT_SET_TIMES, 0)?
+    let resolved = path_get(
-        .fd_object;
+        fe,
-    let resolved = path_get(fo, dirflags, path, false)?;
+        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
+    let fe = &wasi_ctx.get_fd_entry(dirfd)?;
-        .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_SYMLINK, 0)?
+    let resolved_new = path_get(fe, host::__WASI_RIGHT_PATH_SYMLINK, 0, 0, new_path, true)?;
-        .fd_object;
-    let resolved_new = path_get(fo, 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
+    let fe = &wasi_ctx.get_fd_entry(dirfd)?;
-        .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_UNLINK_FILE, 0)?
+    let resolved = path_get(fe, host::__WASI_RIGHT_PATH_UNLINK_FILE, 0, 0, path, false)?;
-        .fd_object;
-    let resolved = path_get(fo, 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
+    let fe = &wasi_ctx.get_fd_entry(dirfd)?;
-        .get_fd_entry(dirfd, host::__WASI_RIGHT_PATH_REMOVE_DIRECTORY, 0)?
+    let resolved = path_get(
-        .fd_object;
+        fe,
-    let resolved = path_get(fo, 0, path, true)?;
+        host::__WASI_RIGHT_PATH_REMOVE_DIRECTORY,
+        0,
+        0,
+        path,
+        true,
+    )?;
 
     log::debug!("path_remove_directory resolved={:?}", resolved);
 
@@ -997,12 +1042,10 @@ pub(crate) unsafe fn fd_prestat_get(
     );
 
     let fd = dec_fd(fd);
-    // TODO: is this the correct right for this?
+    // TODO: should we validate any rights here?
-    wasi_ctx
+    let fe = &wasi_ctx.get_fd_entry(fd)?;
-        .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 {
+    if fe.file_type != host::__WASI_FILETYPE_DIRECTORY {
         return Err(Error::ENOTDIR);
     }
 
@@ -1020,7 +1063,6 @@ pub(crate) unsafe fn fd_prestat_get(
             },
         },
     )
-        })
 }
 
 pub(crate) unsafe fn fd_prestat_dir_name(
@@ -1038,12 +1080,10 @@ pub(crate) unsafe fn fd_prestat_dir_name(
     );
 
     let fd = dec_fd(fd);
-
+    // TODO: should we validate any rights here?
-    wasi_ctx
+    let fe = &wasi_ctx.get_fd_entry(fd)?;
-        .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 {
+    if fe.file_type != host::__WASI_FILETYPE_DIRECTORY {
         return Err(Error::ENOTDIR);
     }
 
@@ -1056,7 +1096,6 @@ pub(crate) unsafe fn fd_prestat_dir_name(
     trace!("     | (path_ptr,path_len)='{}'", path);
 
     enc_slice_of(memory, path.as_bytes(), path_ptr)
-        })
 }
 
 #[allow(dead_code)] // trouble with sockets

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

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)
+                        .get_fd_entry(wasi_fd)
-                        .map(|fe| &fe.fd_object.descriptor)
+                        .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!(),
         }