Fix fd_readdir on BSD-style nixes (#81)

* Fix fd_readdir on BSD-style nixes

The fix was tested on Darwin-XNU and FreeBSD. The change introduces
thread-safe cache of (RawFd, *mut libc::DIR) pairs so that
libc::fdopendir syscall is called only once when invoking fd_readdir
for the first time, and then the pointer to the directory stream,
*mut libc::DIR, is reused until the matching raw file descriptor
is closed.

This fix allows then correct use (and matching to the implementation
on Linux kernels) of libc::seekdir and libc::rewinddir to seek through
and rewind the existing directory stream, *mut libc::DIR, which
otherwise seems to be reset/invalidated every time libc::fdopendir
is called (unlike on Linux, where this behaviour is not observed).

* Store dir stream as part of the FdEntry's Descriptor

* Move bsd specifics into separate module

* Add todo comments and fix formatting

* Refactor int conversions

* Emphasise in debug logs that we're looking at fd_readdir entry

* Change visibility of FdEntry and related to public-private

* Rewrite creating DirStream for the first time
This commit is contained in:
Jakub Konka
2019-09-14 21:01:39 +02:00
committed by GitHub
parent 500e32a3b2
commit c98b3d10ec
15 changed files with 484 additions and 219 deletions

View File

@@ -0,0 +1,156 @@
use super::osfile::OsFile;
use crate::sys::host_impl;
use crate::{host, Error, Result};
use nix::libc::{self, c_long, c_void};
use std::convert::TryInto;
use std::fs::File;
use std::os::unix::prelude::AsRawFd;
pub(crate) fn fd_readdir(
os_file: &mut OsFile,
host_buf: &mut [u8],
cookie: host::__wasi_dircookie_t,
) -> Result<usize> {
use crate::sys::unix::bsd::osfile::DirStream;
use libc::{fdopendir, memcpy, readdir, rewinddir, seekdir, telldir};
use nix::errno::Errno;
use std::mem::ManuallyDrop;
use std::sync::Mutex;
let dir_stream = match os_file.dir_stream {
Some(ref mut dir_stream) => dir_stream,
None => {
let file = os_file.file.try_clone()?;
let dir_ptr = unsafe { fdopendir(file.as_raw_fd()) };
os_file.dir_stream.get_or_insert(Mutex::new(DirStream {
file: ManuallyDrop::new(file),
dir_ptr,
}))
}
};
let dir_stream = dir_stream.lock().unwrap();
let host_buf_ptr = host_buf.as_mut_ptr();
let host_buf_len = host_buf.len();
if cookie != host::__WASI_DIRCOOKIE_START {
unsafe { seekdir(dir_stream.dir_ptr, cookie as c_long) };
} else {
unsafe { rewinddir(dir_stream.dir_ptr) };
}
let mut left = host_buf_len;
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 mut entry: host::__wasi_dirent_t =
host_impl::dirent_from_host(&unsafe { *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 host::__wasi_dircookie_t;
log::debug!("fd_readdir entry = {:?}", entry);
let name_len = entry.d_namlen.try_into()?;
let required_space = std::mem::size_of_val(&entry) + name_len;
if required_space > left {
break;
}
unsafe {
let ptr = host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut c_void
as *mut host::__wasi_dirent_t;
*ptr = entry;
}
host_buf_offset += std::mem::size_of_val(&entry);
let name_ptr = unsafe { *host_entry }.d_name.as_ptr();
unsafe {
memcpy(
host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut _,
name_ptr as *const _,
name_len,
)
};
host_buf_offset += name_len;
left -= required_space;
}
Ok(host_buf_len - left)
}
#[cfg(any(target_os = "macos", target_os = "ios"))]
pub(crate) fn fd_advise(
file: &File,
advice: host::__wasi_advice_t,
offset: host::__wasi_filesize_t,
len: host::__wasi_filesize_t,
) -> Result<()> {
use nix::errno::Errno;
match advice {
host::__WASI_ADVICE_DONTNEED => return Ok(()),
// unfortunately, the advisory syscall in macOS doesn't take any flags of this
// sort (unlike on Linux), hence, they are left here as a noop
host::__WASI_ADVICE_SEQUENTIAL
| host::__WASI_ADVICE_WILLNEED
| host::__WASI_ADVICE_NOREUSE
| host::__WASI_ADVICE_RANDOM
| host::__WASI_ADVICE_NORMAL => {}
_ => return Err(Error::EINVAL),
}
// From macOS man pages:
// F_RDADVISE Issue an advisory read async with no copy to user.
//
// The F_RDADVISE command operates on the following structure which holds information passed from
// the user to the system:
//
// struct radvisory {
// off_t ra_offset; /* offset into the file */
// int ra_count; /* size of the read */
// };
let advisory = libc::radvisory {
ra_offset: offset.try_into()?,
ra_count: len.try_into()?,
};
let res = unsafe { libc::fcntl(file.as_raw_fd(), libc::F_RDADVISE, &advisory) };
Errno::result(res).map(|_| ()).map_err(Error::from)
}
// TODO
// It seems that at least some BSDs do support `posix_fadvise`,
// so we should investigate further.
#[cfg(not(any(target_os = "macos", target_os = "ios")))]
pub(crate) fn fd_advise(
_file: &File,
advice: host::__wasi_advice_t,
_offset: host::__wasi_filesize_t,
_len: host::__wasi_filesize_t,
) -> Result<()> {
match advice {
host::__WASI_ADVICE_DONTNEED
| host::__WASI_ADVICE_SEQUENTIAL
| host::__WASI_ADVICE_WILLNEED
| host::__WASI_ADVICE_NOREUSE
| host::__WASI_ADVICE_RANDOM
| host::__WASI_ADVICE_NORMAL => {}
_ => return Err(Error::EINVAL),
}
Ok(())
}

2
src/sys/unix/bsd/mod.rs Normal file
View File

@@ -0,0 +1,2 @@
pub(crate) mod hostcalls_impl;
pub(crate) mod osfile;

View File

@@ -0,0 +1,52 @@
use std::fs;
use std::mem::ManuallyDrop;
use std::ops::{Deref, DerefMut};
use std::os::unix::prelude::{AsRawFd, RawFd};
use std::sync::Mutex;
#[derive(Debug)]
pub(crate) struct DirStream {
pub(crate) file: ManuallyDrop<fs::File>,
pub(crate) dir_ptr: *mut libc::DIR,
}
impl Drop for DirStream {
fn drop(&mut self) {
unsafe { libc::closedir(self.dir_ptr) };
}
}
#[derive(Debug)]
pub(crate) struct OsFile {
pub(crate) file: fs::File,
pub(crate) dir_stream: Option<Mutex<DirStream>>,
}
impl From<fs::File> for OsFile {
fn from(file: fs::File) -> Self {
Self {
file,
dir_stream: None,
}
}
}
impl AsRawFd for OsFile {
fn as_raw_fd(&self) -> RawFd {
self.file.as_raw_fd()
}
}
impl Deref for OsFile {
type Target = fs::File;
fn deref(&self) -> &Self::Target {
&self.file
}
}
impl DerefMut for OsFile {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.file
}
}

View File

@@ -3,10 +3,25 @@ use crate::{host, Error, Result};
use std::io;
use std::os::unix::prelude::{AsRawFd, FileTypeExt, FromRawFd, RawFd};
cfg_if::cfg_if! {
if #[cfg(target_os = "linux")] {
pub(crate) use super::linux::osfile::*;
} else if #[cfg(any(
target_os = "macos",
target_os = "netbsd",
target_os = "freebsd",
target_os = "openbsd",
target_os = "ios",
target_os = "dragonfly"
))] {
pub(crate) use super::bsd::osfile::*;
}
}
impl AsRawFd for Descriptor {
fn as_raw_fd(&self) -> RawFd {
match self {
Descriptor::File(f) => f.as_raw_fd(),
Descriptor::OsFile(file) => file.as_raw_fd(),
Descriptor::Stdin => io::stdin().as_raw_fd(),
Descriptor::Stdout => io::stdout().as_raw_fd(),
Descriptor::Stderr => io::stderr().as_raw_fd(),

View File

@@ -5,13 +5,28 @@ use crate::helpers::systemtime_to_timestamp;
use crate::hostcalls_impl::PathGet;
use crate::sys::host_impl;
use crate::{host, Error, Result};
use nix::libc::{self, c_long, c_void};
use nix::libc;
use std::convert::TryInto;
use std::ffi::CString;
use std::fs::{File, Metadata};
use std::os::unix::fs::FileExt;
use std::os::unix::prelude::{AsRawFd, FromRawFd};
cfg_if::cfg_if! {
if #[cfg(target_os = "linux")] {
pub(crate) use super::super::linux::hostcalls_impl::*;
} else if #[cfg(any(
target_os = "macos",
target_os = "netbsd",
target_os = "freebsd",
target_os = "openbsd",
target_os = "ios",
target_os = "dragonfly"
))] {
pub(crate) use super::super::bsd::hostcalls_impl::*;
}
}
pub(crate) fn fd_pread(
file: &File,
buf: &mut [u8],
@@ -41,94 +56,6 @@ pub(crate) fn fd_fdstat_set_flags(fd: &File, fdflags: host::__wasi_fdflags_t) ->
}
}
#[cfg(target_os = "linux")]
pub(crate) fn fd_advise(
file: &File,
advice: host::__wasi_advice_t,
offset: host::__wasi_filesize_t,
len: host::__wasi_filesize_t,
) -> Result<()> {
{
use nix::fcntl::{posix_fadvise, PosixFadviseAdvice};
let offset = offset.try_into()?;
let len = len.try_into()?;
let host_advice = match advice {
host::__WASI_ADVICE_DONTNEED => PosixFadviseAdvice::POSIX_FADV_DONTNEED,
host::__WASI_ADVICE_SEQUENTIAL => PosixFadviseAdvice::POSIX_FADV_SEQUENTIAL,
host::__WASI_ADVICE_WILLNEED => PosixFadviseAdvice::POSIX_FADV_WILLNEED,
host::__WASI_ADVICE_NOREUSE => PosixFadviseAdvice::POSIX_FADV_NOREUSE,
host::__WASI_ADVICE_RANDOM => PosixFadviseAdvice::POSIX_FADV_RANDOM,
host::__WASI_ADVICE_NORMAL => PosixFadviseAdvice::POSIX_FADV_NORMAL,
_ => return Err(Error::EINVAL),
};
posix_fadvise(file.as_raw_fd(), offset, len, host_advice)?;
}
Ok(())
}
#[cfg(not(any(target_os = "linux", target_os = "macos")))]
pub(crate) fn fd_advise(
_file: &File,
advice: host::__wasi_advice_t,
_offset: host::__wasi_filesize_t,
_len: host::__wasi_filesize_t,
) -> Result<()> {
match advice {
host::__WASI_ADVICE_DONTNEED
| host::__WASI_ADVICE_SEQUENTIAL
| host::__WASI_ADVICE_WILLNEED
| host::__WASI_ADVICE_NOREUSE
| host::__WASI_ADVICE_RANDOM
| host::__WASI_ADVICE_NORMAL => {}
_ => return Err(Error::EINVAL),
}
Ok(())
}
#[cfg(target_os = "macos")]
pub(crate) fn fd_advise(
file: &File,
advice: host::__wasi_advice_t,
offset: host::__wasi_filesize_t,
len: host::__wasi_filesize_t,
) -> Result<()> {
use nix::errno::Errno;
match advice {
host::__WASI_ADVICE_DONTNEED => return Ok(()),
// unfortunately, the advisory syscall in macOS doesn't take any flags of this
// sort (unlike on Linux), hence, they are left here as a noop
host::__WASI_ADVICE_SEQUENTIAL
| host::__WASI_ADVICE_WILLNEED
| host::__WASI_ADVICE_NOREUSE
| host::__WASI_ADVICE_RANDOM
| host::__WASI_ADVICE_NORMAL => {}
_ => return Err(Error::EINVAL),
}
// From macOS man pages:
// F_RDADVISE Issue an advisory read async with no copy to user.
//
// The F_RDADVISE command operates on the following structure which holds information passed from
// the user to the system:
//
// struct radvisory {
// off_t ra_offset; /* offset into the file */
// int ra_count; /* size of the read */
// };
let advisory = libc::radvisory {
ra_offset: offset.try_into()?,
ra_count: len.try_into()?,
};
let res = unsafe { libc::fcntl(file.as_raw_fd(), libc::F_RDADVISE, &advisory) };
Errno::result(res).map(|_| ()).map_err(Error::from)
}
pub(crate) fn path_create_directory(resolved: PathGet) -> Result<()> {
use nix::libc::mkdirat;
let path_cstr = CString::new(resolved.path().as_bytes()).map_err(|_| Error::EILSEQ)?;
@@ -249,66 +176,6 @@ pub(crate) fn path_open(
Ok(unsafe { File::from_raw_fd(new_fd) })
}
pub(crate) fn fd_readdir(
fd: &File,
host_buf: &mut [u8],
cookie: host::__wasi_dircookie_t,
) -> Result<usize> {
use libc::{dirent, fdopendir, memcpy, readdir_r, rewinddir, seekdir};
let host_buf_ptr = host_buf.as_mut_ptr();
let host_buf_len = host_buf.len();
let dir = unsafe { fdopendir(fd.as_raw_fd()) };
if dir.is_null() {
return Err(host_impl::errno_from_nix(nix::errno::Errno::last()));
}
if cookie != host::__WASI_DIRCOOKIE_START {
unsafe { seekdir(dir, cookie as c_long) };
} else {
// If cookie set to __WASI_DIRCOOKIE_START, rewind the dir ptr
// to the start of the stream.
unsafe { rewinddir(dir) };
}
let mut entry_buf = unsafe { std::mem::uninitialized::<dirent>() };
let mut left = host_buf_len;
let mut host_buf_offset: usize = 0;
while left > 0 {
let mut host_entry: *mut dirent = std::ptr::null_mut();
let res = unsafe { readdir_r(dir, &mut entry_buf, &mut host_entry) };
if res == -1 {
return Err(host_impl::errno_from_nix(nix::errno::Errno::last()));
}
if host_entry.is_null() {
break;
}
let entry: host::__wasi_dirent_t = host_impl::dirent_from_host(&unsafe { *host_entry })?;
let name_len = entry.d_namlen as usize;
let required_space = std::mem::size_of_val(&entry) + name_len;
if required_space > left {
break;
}
unsafe {
let ptr =
host_buf_ptr.add(host_buf_offset) as *mut c_void as *mut host::__wasi_dirent_t;
*ptr = entry;
}
host_buf_offset += std::mem::size_of_val(&entry);
let name_ptr = unsafe { *host_entry }.d_name.as_ptr();
unsafe {
memcpy(
host_buf_ptr.add(host_buf_offset) as *mut _,
name_ptr as *const _,
name_len,
)
};
host_buf_offset += name_len;
left -= required_space;
}
Ok(host_buf_len - left)
}
pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result<usize> {
use nix::errno::Errno;
let path_cstr = CString::new(resolved.path().as_bytes()).map_err(|_| Error::EILSEQ)?;

View File

@@ -0,0 +1,104 @@
use super::osfile::OsFile;
use crate::sys::host_impl;
use crate::{host, Error, Result};
use nix::libc::{self, c_long, c_void};
use std::convert::TryInto;
use std::fs::File;
use std::os::unix::prelude::AsRawFd;
pub(crate) fn fd_readdir(
os_file: &mut OsFile,
host_buf: &mut [u8],
cookie: host::__wasi_dircookie_t,
) -> Result<usize> {
use libc::{dirent, fdopendir, memcpy, readdir_r, rewinddir, seekdir};
let host_buf_ptr = host_buf.as_mut_ptr();
let host_buf_len = host_buf.len();
let dir = unsafe { fdopendir(os_file.as_raw_fd()) };
if dir.is_null() {
return Err(host_impl::errno_from_nix(nix::errno::Errno::last()));
}
if cookie != host::__WASI_DIRCOOKIE_START {
unsafe { seekdir(dir, cookie as c_long) };
} else {
// If cookie set to __WASI_DIRCOOKIE_START, rewind the dir ptr
// to the start of the stream.
unsafe { rewinddir(dir) };
}
let mut entry_buf = unsafe { std::mem::uninitialized::<dirent>() };
let mut left = host_buf_len;
let mut host_buf_offset: usize = 0;
while left > 0 {
let mut host_entry: *mut dirent = std::ptr::null_mut();
// TODO
// `readdir_r` syscall is being deprecated so we should look into
// replacing it with `readdir` call instead.
// Also, `readdir_r` returns a positive int on failure, and doesn't
// set the errno.
let res = unsafe { readdir_r(dir, &mut entry_buf, &mut host_entry) };
if res == -1 {
return Err(host_impl::errno_from_nix(nix::errno::Errno::last()));
}
if host_entry.is_null() {
break;
}
let entry: host::__wasi_dirent_t = host_impl::dirent_from_host(&unsafe { *host_entry })?;
log::debug!("fd_readdir entry = {:?}", entry);
let name_len = entry.d_namlen.try_into()?;
let required_space = std::mem::size_of_val(&entry) + name_len;
if required_space > left {
break;
}
unsafe {
let ptr = host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut c_void
as *mut host::__wasi_dirent_t;
*ptr = entry;
}
host_buf_offset += std::mem::size_of_val(&entry);
let name_ptr = unsafe { *host_entry }.d_name.as_ptr();
unsafe {
memcpy(
host_buf_ptr.offset(host_buf_offset.try_into()?) as *mut _,
name_ptr as *const _,
name_len,
)
};
host_buf_offset += name_len;
left -= required_space;
}
Ok(host_buf_len - left)
}
pub(crate) fn fd_advise(
file: &File,
advice: host::__wasi_advice_t,
offset: host::__wasi_filesize_t,
len: host::__wasi_filesize_t,
) -> Result<()> {
{
use nix::fcntl::{posix_fadvise, PosixFadviseAdvice};
let offset = offset.try_into()?;
let len = len.try_into()?;
let host_advice = match advice {
host::__WASI_ADVICE_DONTNEED => PosixFadviseAdvice::POSIX_FADV_DONTNEED,
host::__WASI_ADVICE_SEQUENTIAL => PosixFadviseAdvice::POSIX_FADV_SEQUENTIAL,
host::__WASI_ADVICE_WILLNEED => PosixFadviseAdvice::POSIX_FADV_WILLNEED,
host::__WASI_ADVICE_NOREUSE => PosixFadviseAdvice::POSIX_FADV_NOREUSE,
host::__WASI_ADVICE_RANDOM => PosixFadviseAdvice::POSIX_FADV_RANDOM,
host::__WASI_ADVICE_NORMAL => PosixFadviseAdvice::POSIX_FADV_NORMAL,
_ => return Err(Error::EINVAL),
};
posix_fadvise(file.as_raw_fd(), offset, len, host_advice)?;
}
Ok(())
}

View File

@@ -0,0 +1,2 @@
pub(crate) mod hostcalls_impl;
pub(crate) mod osfile;

View File

@@ -0,0 +1,32 @@
use std::fs;
use std::ops::{Deref, DerefMut};
use std::os::unix::prelude::{AsRawFd, RawFd};
#[derive(Debug)]
pub(crate) struct OsFile(fs::File);
impl From<fs::File> for OsFile {
fn from(file: fs::File) -> Self {
Self(file)
}
}
impl AsRawFd for OsFile {
fn as_raw_fd(&self) -> RawFd {
self.0.as_raw_fd()
}
}
impl Deref for OsFile {
type Target = fs::File;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl DerefMut for OsFile {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

View File

@@ -2,6 +2,18 @@ pub(crate) mod fdentry_impl;
pub(crate) mod host_impl;
pub(crate) mod hostcalls_impl;
#[cfg(any(
target_os = "macos",
target_os = "netbsd",
target_os = "freebsd",
target_os = "openbsd",
target_os = "ios",
target_os = "dragonfly"
))]
mod bsd;
#[cfg(target_os = "linux")]
mod linux;
use crate::Result;
use std::fs::File;
use std::path::Path;

View File

@@ -2,12 +2,42 @@ use crate::fdentry::Descriptor;
use crate::{host, Error, Result};
use std::fs::File;
use std::io;
use std::ops::{Deref, DerefMut};
use std::os::windows::prelude::{AsRawHandle, FromRawHandle, RawHandle};
#[derive(Debug)]
pub(crate) struct OsFile(File);
impl From<File> for OsFile {
fn from(file: File) -> Self {
Self(file)
}
}
impl AsRawHandle for OsFile {
fn as_raw_handle(&self) -> RawHandle {
self.0.as_raw_handle()
}
}
impl Deref for OsFile {
type Target = File;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl DerefMut for OsFile {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}
impl AsRawHandle for Descriptor {
fn as_raw_handle(&self) -> RawHandle {
match self {
Descriptor::File(f) => f.as_raw_handle(),
Descriptor::OsFile(file) => file.as_raw_handle(),
Descriptor::Stdin => io::stdin().as_raw_handle(),
Descriptor::Stdout => io::stdout().as_raw_handle(),
Descriptor::Stderr => io::stderr().as_raw_handle(),

View File

@@ -5,7 +5,7 @@ use crate::ctx::WasiCtx;
use crate::fdentry::FdEntry;
use crate::helpers::systemtime_to_timestamp;
use crate::hostcalls_impl::{fd_filestat_set_times_impl, PathGet};
use crate::sys::fdentry_impl::determine_type_rights;
use crate::sys::fdentry_impl::{determine_type_rights, OsFile};
use crate::sys::host_impl;
use crate::sys::hostcalls_impl::fs_helpers::PathGetExt;
use crate::{host, Error, Result};
@@ -167,7 +167,7 @@ pub(crate) fn path_open(
}
pub(crate) fn fd_readdir(
fd: &File,
fd: &mut OsFile,
host_buf: &mut [u8],
cookie: host::__wasi_dircookie_t,
) -> Result<usize> {