T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix bug in fd_readdir impl on BSD

Browse Source 
This commit fixes a subtle bug in `fd_readdir` implementation on
BSD-style nixes. In particular, the bug only resurfaced when testing in
release mode, and I can only assume it was due to a unlucky combination
of compiler optimizations and at first sight correct casting of `d_name`
pointer from `const* i8` to `const* u8`. This is now fixed by first
converting `d_name` to `std::str` by using `std::ffi::CStr::to_str`, and
then using the resultant `std::str` to copy the properly validated `u8`
contents into the Wasm buffer.

Furthermore, this commit fixes incorrect handling of the `readdir` loop
break condition. Although undocumented in BSD man pages, the signalling
is the same as on Linux where a null ptr returned by `readdir` signals
the end of the dir stream when the errno code has not changed since
before calling `readdir` inside a loop. Upon a fault such as an invalid
file descriptor, the errno *will* change after executing `readdir`.
This commit is contained in:
Jakub Konka
2019-11-08 10:49:15 +01:00
parent ac40eff871
commit 4982878a95
1 changed files with 30 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
wasi-common/src/sys/unix/bsd/hostcalls_impl.rs
View File

@@ -147,6 +147,7 @@ pub(crate) fn fd_readdir(
use crate::sys::unix::bsd::osfile::DirStream;
use libc::{fdopendir, readdir, rewinddir, seekdir, telldir};
use nix::errno::Errno;
use std::ffi::CStr;
use std::mem::ManuallyDrop;
use std::sync::Mutex;
@@ -176,45 +177,53 @@ pub(crate) fn fd_readdir(
let mut host_buf_offset: usize = 0;
loop {
let host_entry = unsafe { readdir(dir_stream.dir_ptr) };
if host_entry.is_null() {
// FIXME
// Currently, these are verified to be correct on macOS.
// Need to still verify these on other BSD-based OSes.
match Errno::last() {
Errno::EBADF => return Err(Error::EBADF),
Errno::EFAULT => return Err(Error::EFAULT),
Errno::EIO => return Err(Error::EIO),
_ => break, // not an error
let errno = Errno::last();
let host_entry_ptr = unsafe { readdir(dir_stream.dir_ptr) };
if host_entry_ptr.is_null() {
if errno != Errno::last() {
// TODO Is this correct?
// According to POSIX man (for Linux though!), there was an error
// if the errno value has changed at some point during the sequence
// of readdir calls
return Err(host_impl::errno_from_nix(Errno::last()));
} else {
// Not an error
break;
}
}
let mut entry: wasi::__wasi_dirent_t =
host_impl::dirent_from_host(&unsafe { *host_entry })?;
let host_entry = unsafe { *host_entry_ptr };
let mut wasi_entry: wasi::__wasi_dirent_t = host_impl::dirent_from_host(&host_entry)?;
// Set d_next manually:
// * on macOS d_seekoff is not set for some reason
// * on FreeBSD d_seekoff doesn't exist; there is d_off but it is
// not equivalent to the value read from telldir call
entry.d_next = unsafe { telldir(dir_stream.dir_ptr) } as wasi::__wasi_dircookie_t;
wasi_entry.d_next = unsafe { telldir(dir_stream.dir_ptr) } as wasi::__wasi_dircookie_t;
log::debug!("fd_readdir entry = {:?}", entry);
log::debug!("fd_readdir host_entry = {:?}", host_entry);
log::debug!("fd_readdir wasi_entry = {:?}", wasi_entry);
let name_len = host_entry.d_namlen.try_into()?;
let required_space = std::mem::size_of_val(&wasi_entry) + name_len;
let name_len = entry.d_namlen.try_into()?;
let required_space = std::mem::size_of_val(&entry) + name_len;
if required_space > left {
break;
}
let name = unsafe { CStr::from_ptr(host_entry.d_name.as_ptr()) }.to_str()?;
log::debug!("fd_readdir entry name = {}", name);
unsafe {
let ptr = host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut c_void
as *mut wasi::__wasi_dirent_t;
*ptr = entry;
*ptr = wasi_entry;
}
host_buf_offset += std::mem::size_of_val(&entry);
let name_ptr = unsafe { *host_entry }.d_name.as_ptr();
host_buf_offset += std::mem::size_of_val(&wasi_entry);
unsafe {
std::ptr::copy_nonoverlapping(
name_ptr as *const _,
host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut _,
name.as_ptr(),
host_buf_ptr.offset(host_buf_offset.try_into()?),
name_len,
)
};