Merge pull request #5323 from wasmerio/fix/close-host-files-properly

Close host files when the last FD referencing them is closed
This commit is contained in:
Arshia001
2025-01-10 18:58:20 +03:30
committed by GitHub
39 changed files with 473 additions and 111 deletions

7
Cargo.lock generated
View File

@@ -216,6 +216,12 @@ dependencies = [
"nom",
]
[[package]]
name = "assert-panic"
version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "763b2b82aee23fe46c14c792470080c26538396e9ea589f548298f26b22d7f41"
[[package]]
name = "assert_cmd"
version = "1.0.8"
@@ -7146,6 +7152,7 @@ name = "wasmer-wasix"
version = "0.35.0"
dependencies = [
"anyhow",
"assert-panic",
"async-trait",
"base64",
"bincode",

View File

@@ -351,10 +351,10 @@ impl crate::FileOpener for FileSystem {
pub struct File {
#[cfg_attr(feature = "enable-serde", serde(skip, default = "default_handle"))]
handle: Handle,
#[cfg_attr(feature = "enable-serde", serde(skip_serializing))]
inner_std: fs::File,
#[cfg_attr(feature = "enable-serde", serde(skip))]
inner: tfs::File,
#[cfg_attr(feature = "enable-serde", serde(skip_serializing))]
inner_std: fs::File,
pub host_path: PathBuf,
#[cfg(feature = "enable-serde")]
flags: u16,
@@ -635,6 +635,12 @@ impl AsyncSeek for File {
}
}
impl Drop for File {
fn drop(&mut self) {
tracing::trace!(?self.host_path, "Closing host file");
}
}
/// A wrapper type around Stdout that implements `VirtualFile`.
#[derive(Debug)]
#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]

View File

@@ -165,6 +165,7 @@ tracing-test = "0.2.4"
wasm-bindgen-test = "0.3.0"
env_logger = { version = "0.11.5", default-features = false }
log = "0.4.22"
assert-panic = "1.0.1"
[target.'cfg(target_arch = "wasm32")'.dev-dependencies]
wasm-bindgen-test = "0.3.0"

View File

@@ -24,10 +24,9 @@ use super::{
#[derive(Debug, Clone)]
#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
pub struct Fd {
pub rights: Rights,
pub rights_inheriting: Rights,
pub flags: Fdflags,
pub offset: Arc<AtomicU64>,
#[cfg_attr(feature = "enable-serde", serde(flatten))]
pub inner: FdInner,
/// Flags that determine how the [`Fd`] can be used.
///
/// Used when reopening a [`VirtualFile`] during deserialization.
@@ -36,6 +35,17 @@ pub struct Fd {
pub is_stdio: bool,
}
// This struct contains the bits of Fd that are safe to mutate, so that
// FdList::get_mut can safely return mutable references.
#[derive(Debug, Clone)]
#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
pub struct FdInner {
pub rights: Rights,
pub rights_inheriting: Rights,
pub flags: Fdflags,
pub offset: Arc<AtomicU64>,
}
impl Fd {
/// This [`Fd`] can be used with read system calls.
pub const READ: u16 = 1;

View File

@@ -4,10 +4,10 @@
//! Note, The Unix spec requires newly allocated FDs to always be the
//! lowest-numbered FD available.
use super::fd::Fd;
use super::fd::{Fd, FdInner};
use wasmer_wasix_types::wasi::Fd as WasiFd;
#[derive(Debug, Clone)]
#[derive(Debug)]
pub struct FdList {
fds: Vec<Option<Fd>>,
first_free: Option<usize>,
@@ -57,11 +57,15 @@ impl FdList {
self.fds.get(idx as usize).and_then(|x| x.as_ref())
}
pub fn get_mut(&mut self, idx: WasiFd) -> Option<&mut Fd> {
self.fds.get_mut(idx as usize).and_then(|x| x.as_mut())
pub fn get_mut(&mut self, idx: WasiFd) -> Option<&mut FdInner> {
self.fds
.get_mut(idx as usize)
.and_then(|x| x.as_mut())
.map(|x| &mut x.inner)
}
pub fn insert_first_free(&mut self, fd: Fd) -> WasiFd {
fd.inode.acquire_handle();
match self.first_free {
Some(free) => {
debug_assert!(self.fds[free].is_none());
@@ -106,6 +110,7 @@ impl FdList {
return false;
}
fd.inode.acquire_handle();
self.fds[idx] = Some(fd);
true
}
@@ -115,18 +120,26 @@ impl FdList {
let result = self.fds.get_mut(idx).and_then(|fd| fd.take());
if result.is_some() {
if let Some(fd) = result.as_ref() {
match self.first_free {
None => self.first_free = Some(idx),
Some(x) if x > idx => self.first_free = Some(idx),
_ => (),
}
fd.inode.drop_one_handle();
}
result
}
pub fn clear(&mut self) {
for fd in &self.fds {
if let Some(fd) = fd.as_ref() {
fd.inode.drop_one_handle();
}
}
self.fds.clear();
self.first_free = None;
}
@@ -150,6 +163,27 @@ impl FdList {
}
}
impl Clone for FdList {
fn clone(&self) -> Self {
for fd in &self.fds {
if let Some(fd) = fd.as_ref() {
fd.inode.acquire_handle();
}
}
Self {
fds: self.fds.clone(),
first_free: self.first_free,
}
}
}
impl Drop for FdList {
fn drop(&mut self) {
self.clear();
}
}
impl<'a> Iterator for FdListIterator<'a> {
type Item = (WasiFd, &'a Fd);
@@ -174,7 +208,7 @@ impl<'a> Iterator for FdListIterator<'a> {
}
impl<'a> Iterator for FdListIteratorMut<'a> {
type Item = (WasiFd, &'a mut Fd);
type Item = (WasiFd, &'a mut FdInner);
fn next(&mut self) -> Option<Self::Item> {
loop {
@@ -189,7 +223,7 @@ impl<'a> Iterator for FdListIteratorMut<'a> {
Some(Some(fd)) => {
let wasi_fd = self.idx as WasiFd;
self.idx += 1;
return Some((wasi_fd, fd));
return Some((wasi_fd, &mut fd.inner));
}
}
}
@@ -200,19 +234,22 @@ impl<'a> Iterator for FdListIteratorMut<'a> {
mod tests {
use std::{
borrow::Cow,
sync::{atomic::AtomicU64, Arc, RwLock},
sync::{
atomic::{AtomicI32, AtomicU64},
Arc, RwLock,
},
};
use assert_panic::assert_panic;
use wasmer_wasix_types::wasi::{Fdflags, Rights};
use crate::fs::{Inode, InodeGuard, InodeVal, Kind};
use crate::fs::{fd::FdInner, Inode, InodeGuard, InodeVal, Kind};
use super::{Fd, FdList, WasiFd};
fn useless_fd(n: u16) -> Fd {
Fd {
open_flags: n,
flags: Fdflags::empty(),
open_flags: 0,
inode: InodeGuard {
ino: Inode(0),
inner: Arc::new(InodeVal {
@@ -221,16 +258,24 @@ mod tests {
name: RwLock::new(Cow::Borrowed("")),
stat: RwLock::new(Default::default()),
}),
open_handles: Arc::new(AtomicI32::new(0)),
},
is_stdio: false,
offset: Arc::new(AtomicU64::new(0)),
rights: Rights::empty(),
rights_inheriting: Rights::empty(),
inner: FdInner {
offset: Arc::new(AtomicU64::new(0)),
rights: Rights::empty(),
rights_inheriting: Rights::empty(),
flags: Fdflags::from_bits_preserve(n),
},
}
}
fn is_useless_fd(fd: &Fd, n: u16) -> bool {
fd.open_flags == n
fd.inner.flags.bits() == n
}
fn is_useless_fd_inner(fd_inner: &FdInner, n: u16) -> bool {
fd_inner.flags.bits() == n
}
fn assert_fds_match(l: &FdList, expected: &[(WasiFd, u16)]) {
@@ -346,8 +391,8 @@ mod tests {
assert!(is_useless_fd(l.get(2).unwrap(), 2));
let at_4 = l.get_mut(4).unwrap();
assert!(is_useless_fd(at_4, 4));
*at_4 = useless_fd(5);
assert!(is_useless_fd_inner(at_4, 4));
at_4.flags = Fdflags::from_bits_preserve(5); // Update the "useless FD" number without changing the InodeGuard
assert!(is_useless_fd(l.get(4).unwrap(), 5));
assert!(l.get(10).is_none());
@@ -441,15 +486,72 @@ mod tests {
let next = i.next().unwrap();
assert_eq!(next.0, 0);
assert!(is_useless_fd(next.1, 0));
*next.1 = useless_fd(2);
assert!(is_useless_fd_inner(next.1, 0));
next.1.flags = Fdflags::from_bits_preserve(2); // Update the "useless FD" number without changing the InodeGuard
let next = i.next().unwrap();
assert_eq!(next.0, 1);
assert!(is_useless_fd(next.1, 1));
assert!(is_useless_fd_inner(next.1, 1));
assert!(i.next().is_none());
assert_fds_match(&l, &[(0, 2), (1, 1)]);
}
#[test]
fn open_handles_are_updated_correctly() {
let mut l = FdList::new();
l.insert_first_free(useless_fd(0));
l.insert_first_free(useless_fd(1));
let fd0 = l.get(0).unwrap().clone();
assert_eq!(fd0.inode.handle_count(), 1);
// Try removing an FD, should drop the handle
let fd1 = l.get(1).unwrap().clone();
assert_eq!(fd1.inode.handle_count(), 1);
l.remove(1).unwrap();
assert_eq!(fd1.inode.handle_count(), 0);
// Existing FDs should get a new handle when cloning the list
let mut l2 = l.clone();
assert_eq!(fd0.inode.handle_count(), 2);
{
// Dropping the list should drop open handles
let l3 = l2.clone();
assert_eq!(fd0.inode.handle_count(), 3);
drop(l3);
assert_eq!(fd0.inode.handle_count(), 2);
}
// Clearing the list should drop open handles
l.clear();
assert_eq!(fd0.inode.handle_count(), 1);
// Clear the last handle, should go back to zero
l2.clear();
assert_eq!(fd0.inode.handle_count(), 0);
assert_panic!(
fd0.inode.drop_one_handle(),
&str,
"InodeGuard handle dropped too many times"
);
assert_panic!(drop(fd0.inode.write()), String, contains "PoisonError");
}
#[test]
fn messing_with_inode_causes_panic() {
// We want to pin this behavior down, as not causing a panic
// can lead to inconsistencies
let mut l = FdList::new();
l.insert_first_free(useless_fd(0));
let fd = l.get(0).unwrap();
fd.inode.drop_one_handle();
assert_panic!(drop(l), &str, "InodeGuard handle dropped too many times");
}
}

View File

@@ -17,7 +17,7 @@ use std::{
path::{Component, Path, PathBuf},
pin::Pin,
sync::{
atomic::{AtomicBool, AtomicU64, Ordering},
atomic::{AtomicBool, AtomicI32, AtomicU64, Ordering},
Arc, Mutex, RwLock, Weak,
},
task::{Context, Poll},
@@ -43,7 +43,7 @@ use wasmer_wasix_types::{
},
};
pub use self::fd::{EpollFd, EpollInterest, EpollJoinGuard, Fd, InodeVal, Kind};
pub use self::fd::{EpollFd, EpollInterest, EpollJoinGuard, Fd, FdInner, InodeVal, Kind};
pub(crate) use self::inode_guard::{
InodeValFilePollGuard, InodeValFilePollGuardJoin, InodeValFilePollGuardMode,
InodeValFileReadGuard, InodeValFileWriteGuard, WasiStateFileGuard, POLL_GUARD_MAX_RET,
@@ -121,20 +121,74 @@ impl Inode {
pub struct InodeGuard {
ino: Inode,
inner: Arc<InodeVal>,
// This exists because self.inner doesn't really represent the
// number of FDs referencing this InodeGuard. We need that number
// so we can know when to drop the file handle, which should result
// in the backing file (which may be a host file) getting closed.
open_handles: Arc<AtomicI32>,
}
impl InodeGuard {
pub fn ino(&self) -> Inode {
self.ino
}
pub fn downgrade(&self) -> InodeWeakGuard {
InodeWeakGuard {
ino: self.ino,
open_handles: self.open_handles.clone(),
inner: Arc::downgrade(&self.inner),
}
}
pub fn ref_cnt(&self) -> usize {
Arc::strong_count(&self.inner)
}
pub fn handle_count(&self) -> u32 {
self.open_handles.load(Ordering::SeqCst) as u32
}
pub fn acquire_handle(&self) {
self.open_handles.fetch_add(1, Ordering::SeqCst);
}
pub fn drop_one_handle(&self) {
let prev_handles = self.open_handles.fetch_sub(1, Ordering::SeqCst);
// If this wasn't the last handle, nothing else to do...
if prev_handles > 1 {
return;
}
// ... otherwise, drop the VirtualFile reference
let mut guard = self.inner.write();
// Must have at least one open handle before we can drop.
// This check happens after `inner` is locked so we can
// poison the lock and keep people from using this (possibly
// corrupt) InodeGuard.
if prev_handles != 1 {
panic!("InodeGuard handle dropped too many times");
}
// Re-check the open handles to account for race conditions
if self.open_handles.load(Ordering::SeqCst) != 0 {
return;
}
let ino = self.ino.0;
trace!(%ino, "InodeGuard has no more open handles");
match guard.deref_mut() {
Kind::File { handle, .. } if handle.is_some() => {
let file_ref_count = Arc::strong_count(handle.as_ref().unwrap());
trace!(%file_ref_count, %ino, "dropping file handle");
drop(handle.take().unwrap());
}
_ => (),
}
}
}
impl std::ops::Deref for InodeGuard {
type Target = InodeVal;
@@ -146,6 +200,10 @@ impl std::ops::Deref for InodeGuard {
#[derive(Debug, Clone)]
pub struct InodeWeakGuard {
ino: Inode,
// Needed for when we want to upgrade back. We don't exactly
// care too much when the AtomicI32 is dropped, so this is
// a strong reference to keep things simple.
open_handles: Arc<AtomicI32>,
inner: Weak<InodeVal>,
}
impl InodeWeakGuard {
@@ -155,6 +213,7 @@ impl InodeWeakGuard {
pub fn upgrade(&self) -> Option<InodeGuard> {
Weak::upgrade(&self.inner).map(|inner| InodeGuard {
ino: self.ino,
open_handles: self.open_handles.clone(),
inner,
})
}
@@ -198,7 +257,13 @@ impl WasiInodes {
guard.lookup.retain(|_, v| Weak::strong_count(v) > 0);
}
InodeGuard { ino, inner: val }
let open_handles = Arc::new(AtomicI32::new(0));
InodeGuard {
ino,
open_handles,
inner: val,
}
}
/// Get the `VirtualFile` object at stdout
@@ -687,7 +752,7 @@ impl WasiFs {
/// Opens a user-supplied file in the directory specified with the
/// name and flags given
// dead code because this is an API for external use
// TODO: is this used anywhere?
// TODO: is this used anywhere? Is it even sound?
#[allow(dead_code, clippy::too_many_arguments)]
pub fn open_file_at(
&mut self,
@@ -1307,10 +1372,12 @@ impl WasiFs {
if ret.is_err() && fd == VIRTUAL_ROOT_FD {
Ok(Fd {
rights: ALL_RIGHTS,
rights_inheriting: ALL_RIGHTS,
flags: Fdflags::empty(),
offset: Arc::new(AtomicU64::new(0)),
inner: FdInner {
rights: ALL_RIGHTS,
rights_inheriting: ALL_RIGHTS,
flags: Fdflags::empty(),
offset: Arc::new(AtomicU64::new(0)),
},
open_flags: 0,
inode: self.root_inode.clone(),
is_stdio: false,
@@ -1399,9 +1466,9 @@ impl WasiFs {
},
_ => Filetype::Unknown,
},
fs_flags: fd.flags,
fs_rights_base: fd.rights,
fs_rights_inheriting: fd.rights_inheriting, // TODO(lachlan): Is this right?
fs_flags: fd.inner.flags,
fs_rights_base: fd.inner.rights,
fs_rights_inheriting: fd.inner.rights_inheriting, // TODO(lachlan): Is this right?
})
}
@@ -1445,7 +1512,7 @@ impl WasiFs {
}
_ => {
let fd = self.get_fd(fd)?;
if !fd.rights.contains(Rights::FD_DATASYNC) {
if !fd.inner.rights.contains(Rights::FD_DATASYNC) {
return Err(Errno::Access);
}
@@ -1595,10 +1662,12 @@ impl WasiFs {
Some(__WASI_STDIN_FILENO) | Some(__WASI_STDOUT_FILENO) | Some(__WASI_STDERR_FILENO)
);
let fd = Fd {
rights,
rights_inheriting,
flags,
offset: Arc::new(AtomicU64::new(0)),
inner: FdInner {
rights,
rights_inheriting,
flags,
offset: Arc::new(AtomicU64::new(0)),
},
open_flags,
inode,
is_stdio,
@@ -1621,10 +1690,12 @@ impl WasiFs {
pub fn clone_fd(&self, fd: WasiFd) -> Result<WasiFd, Errno> {
let fd = self.get_fd(fd)?;
Ok(self.fd_map.write().unwrap().insert_first_free(Fd {
rights: fd.rights,
rights_inheriting: fd.rights_inheriting,
flags: fd.flags,
offset: fd.offset.clone(),
inner: FdInner {
rights: fd.inner.rights,
rights_inheriting: fd.inner.rights_inheriting,
flags: fd.inner.flags,
offset: fd.inner.offset.clone(),
},
open_flags: fd.open_flags,
inode: fd.inode,
is_stdio: fd.is_stdio,
@@ -1921,12 +1992,14 @@ impl WasiFs {
false,
raw_fd,
Fd {
rights,
rights_inheriting: Rights::empty(),
flags: fd_flags,
inner: FdInner {
rights,
rights_inheriting: Rights::empty(),
flags: fd_flags,
offset: Arc::new(AtomicU64::new(0)),
},
// since we're not calling open on this, we don't need open flags
open_flags: 0,
offset: Arc::new(AtomicU64::new(0)),
inode,
is_stdio: true,
},

View File

@@ -121,7 +121,7 @@ pub(crate) use crate::{
};
use crate::{
fs::{
fs_error_into_wasi_err, virtual_file_type_to_wasi_file_type, Fd, InodeVal, Kind,
fs_error_into_wasi_err, virtual_file_type_to_wasi_file_type, Fd, FdInner, InodeVal, Kind,
MAX_SYMLINKS,
},
journal::{DynJournal, JournalEffector},
@@ -627,7 +627,7 @@ where
Fut: std::future::Future<Output = Result<T, Errno>>,
{
let fd_entry = env.state.fs.get_fd(sock)?;
if !rights.is_empty() && !fd_entry.rights.contains(rights) {
if !rights.is_empty() && !fd_entry.inner.rights.contains(rights) {
return Err(Errno::Access);
}
@@ -671,7 +671,7 @@ where
let tasks = env.tasks().clone();
let fd_entry = env.state.fs.get_fd(sock)?;
if !rights.is_empty() && !fd_entry.rights.contains(rights) {
if !rights.is_empty() && !fd_entry.inner.rights.contains(rights) {
return Err(Errno::Access);
}
@@ -710,7 +710,7 @@ where
let tasks = env.tasks().clone();
let fd_entry = env.state.fs.get_fd(sock)?;
if !rights.is_empty() && !fd_entry.rights.contains(rights) {
if !rights.is_empty() && !fd_entry.inner.rights.contains(rights) {
return Err(Errno::Access);
}
@@ -747,7 +747,7 @@ where
let tasks = env.tasks().clone();
let fd_entry = env.state.fs.get_fd(sock)?;
if !rights.is_empty() && !fd_entry.rights.contains(rights) {
if !rights.is_empty() && !fd_entry.inner.rights.contains(rights) {
return Err(Errno::Access);
}
@@ -781,7 +781,7 @@ where
{
let env = ctx.data();
let fd_entry = env.state.fs.get_fd(sock)?;
if !rights.is_empty() && !fd_entry.rights.contains(rights) {
if !rights.is_empty() && !fd_entry.inner.rights.contains(rights) {
tracing::warn!(
"wasi[{}:{}]::sock_upgrade(fd={}, rights={:?}) - failed - no access rights to upgrade",
ctx.data().pid(),
@@ -802,7 +802,7 @@ where
drop(guard);
// Start the work using the socket
let work = actor(socket, fd_entry.flags);
let work = actor(socket, fd_entry.inner.flags);
// Block on the work and process it
let res = InlineWaker::block_on(work);

View File

@@ -49,7 +49,7 @@ pub(crate) fn fd_advise_internal(
let fd_entry = state.fs.get_fd(fd)?;
let inode = fd_entry.inode;
if !fd_entry.rights.contains(Rights::FD_ADVISE) {
if !fd_entry.inner.rights.contains(Rights::FD_ADVISE) {
return Err(Errno::Access);
}

View File

@@ -42,7 +42,7 @@ pub(crate) fn fd_allocate_internal(
let fd_entry = state.fs.get_fd(fd)?;
let inode = fd_entry.inode;
if !fd_entry.rights.contains(Rights::FD_ALLOCATE) {
if !fd_entry.inner.rights.contains(Rights::FD_ALLOCATE) {
return Err(Errno::Access);
}
let new_size = offset.checked_add(len).ok_or(Errno::Inval)?;

View File

@@ -13,7 +13,7 @@ pub fn fd_datasync(mut ctx: FunctionEnvMut<'_, WasiEnv>, fd: WasiFd) -> Result<E
let env = ctx.data();
let state = env.state.clone();
let fd_entry = wasi_try_ok!(state.fs.get_fd(fd));
if !fd_entry.rights.contains(Rights::FD_DATASYNC) {
if !fd_entry.inner.rights.contains(Rights::FD_DATASYNC) {
return Ok(Errno::Access);
}

View File

@@ -39,10 +39,10 @@ pub(crate) fn fd_fdstat_set_flags_internal(
let env = ctx.data();
let (_, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) };
let mut fd_map = state.fs.fd_map.write().unwrap();
let fd_entry = wasi_try_ok!(fd_map.get_mut(fd).ok_or(Errno::Badf));
let fd_entry = wasi_try_ok!(fd_map.get(fd).ok_or(Errno::Badf));
let inode = fd_entry.inode.clone();
if !fd_entry.rights.contains(Rights::FD_FDSTAT_SET_FLAGS) {
if !fd_entry.inner.rights.contains(Rights::FD_FDSTAT_SET_FLAGS) {
return Ok(Errno::Access);
}
}
@@ -50,7 +50,38 @@ pub(crate) fn fd_fdstat_set_flags_internal(
let env = ctx.data();
let (_, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) };
let mut fd_map = state.fs.fd_map.write().unwrap();
let fd_entry = wasi_try_ok!(fd_map.get_mut(fd).ok_or(Errno::Badf));
let mut fd_entry = wasi_try_ok!(fd_map.get_mut(fd).ok_or(Errno::Badf));
fd_entry.flags = flags;
Ok(Errno::Success)
}
struct MyStruct {
field1: i32,
field2: String,
}
impl MyStruct {
fn get_mutable_ref(&mut self) -> &mut MyStruct {
self
}
}
fn main_() {
let mut my_struct = MyStruct {
field1: 10,
field2: String::from("Hello"),
};
let my_struct_ref = my_struct.get_mutable_ref();
my_struct_ref.field1 = 20;
my_struct_ref.field2 = String::from("World");
*my_struct_ref = MyStruct {
field1: 20,
field2: "abx".to_owned(),
};
println!(
"field1: {}, field2: {}",
my_struct_ref.field1, my_struct_ref.field2
);
}

View File

@@ -46,7 +46,7 @@ pub(crate) fn fd_fdstat_set_rights_internal(
let env = ctx.data();
let (_, mut state) = unsafe { env.get_memory_and_wasi_state(&ctx, 0) };
let mut fd_map = state.fs.fd_map.write().unwrap();
let fd_entry = fd_map.get_mut(fd).ok_or(Errno::Badf)?;
let mut fd_entry = unsafe { fd_map.get_mut(fd) }.ok_or(Errno::Badf)?;
// ensure new rights are a subset of current rights
if fd_entry.rights | fs_rights_base != fd_entry.rights

View File

@@ -49,7 +49,7 @@ pub(crate) fn fd_filestat_get_internal(
let env = ctx.data();
let (_, state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) };
let fd_entry = state.fs.get_fd(fd)?;
if !fd_entry.rights.contains(Rights::FD_FILESTAT_GET) {
if !fd_entry.inner.rights.contains(Rights::FD_FILESTAT_GET) {
return Err(Errno::Access);
}

View File

@@ -38,7 +38,7 @@ pub(crate) fn fd_filestat_set_size_internal(
let fd_entry = state.fs.get_fd(fd)?;
let inode = fd_entry.inode;
if !fd_entry.rights.contains(Rights::FD_FILESTAT_SET_SIZE) {
if !fd_entry.inner.rights.contains(Rights::FD_FILESTAT_SET_SIZE) {
return Err(Errno::Access);
}

View File

@@ -49,7 +49,11 @@ pub(crate) fn fd_filestat_set_times_internal(
let (_, mut state) = unsafe { env.get_memory_and_wasi_state(&ctx, 0) };
let fd_entry = state.fs.get_fd(fd)?;
if !fd_entry.rights.contains(Rights::FD_FILESTAT_SET_TIMES) {
if !fd_entry
.inner
.rights
.contains(Rights::FD_FILESTAT_SET_TIMES)
{
return Err(Errno::Access);
}

View File

@@ -41,7 +41,7 @@ pub fn fd_read<M: MemorySize>(
let inodes = state.inodes.clone();
let fd_entry = wasi_try_ok!(state.fs.get_fd(fd));
fd_entry.offset.load(Ordering::Acquire) as usize
fd_entry.inner.offset.load(Ordering::Acquire) as usize
};
ctx = wasi_try_ok!(maybe_backoff::<M>(ctx)?);
@@ -137,13 +137,13 @@ pub(crate) fn fd_read_internal<M: MemorySize>(
let is_stdio = fd_entry.is_stdio;
let bytes_read = {
if !is_stdio && !fd_entry.rights.contains(Rights::FD_READ) {
if !is_stdio && !fd_entry.inner.rights.contains(Rights::FD_READ) {
// TODO: figure out the error to return when lacking rights
return Ok(Err(Errno::Access));
}
let inode = fd_entry.inode;
let fd_flags = fd_entry.flags;
let fd_flags = fd_entry.inner.flags;
let (bytes_read, can_update_cursor) = {
let mut guard = inode.write();

View File

@@ -45,8 +45,11 @@ pub(crate) fn fd_renumber_internal(
let new_fd_entry = Fd {
// TODO: verify this is correct
offset: fd_entry.offset.clone(),
rights: fd_entry.rights_inheriting,
inner: FdInner {
offset: fd_entry.inner.offset.clone(),
rights: fd_entry.inner.rights_inheriting,
..fd_entry.inner
},
inode: fd_entry.inode.clone(),
..*fd_entry
};

View File

@@ -59,7 +59,7 @@ pub(crate) fn fd_seek_internal(
let (memory, _) = unsafe { env.get_memory_and_wasi_state(&ctx, 0) };
let fd_entry = wasi_try_ok_ok!(state.fs.get_fd(fd));
if !fd_entry.rights.contains(Rights::FD_SEEK) {
if !fd_entry.inner.rights.contains(Rights::FD_SEEK) {
return Ok(Err(Errno::Access));
}
@@ -133,7 +133,7 @@ pub(crate) fn fd_seek_internal(
return Ok(Err(Errno::Inval));
}
}
fd_entry.offset.load(Ordering::Acquire)
fd_entry.inner.offset.load(Ordering::Acquire)
}
Whence::Set => {
let mut fd_map = state.fs.fd_map.write().unwrap();

View File

@@ -17,7 +17,7 @@ pub fn fd_sync(mut ctx: FunctionEnvMut<'_, WasiEnv>, fd: WasiFd) -> Result<Errno
let env = ctx.data();
let (_, mut state) = unsafe { env.get_memory_and_wasi_state(&ctx, 0) };
let fd_entry = wasi_try_ok!(state.fs.get_fd(fd));
if !fd_entry.rights.contains(Rights::FD_SYNC) {
if !fd_entry.inner.rights.contains(Rights::FD_SYNC) {
return Ok(Errno::Access);
}
let inode = fd_entry.inode;

View File

@@ -21,11 +21,11 @@ pub fn fd_tell<M: MemorySize>(
let fd_entry = wasi_try!(state.fs.get_fd(fd));
if !fd_entry.rights.contains(Rights::FD_TELL) {
if !fd_entry.inner.rights.contains(Rights::FD_TELL) {
return Errno::Access;
}
let offset = fd_entry.offset.load(Ordering::Acquire);
let offset = fd_entry.inner.offset.load(Ordering::Acquire);
Span::current().record("offset", offset);
wasi_try_mem!(offset_ref.write(offset));

View File

@@ -38,7 +38,7 @@ pub fn fd_write<M: MemorySize>(
let inodes = state.inodes.clone();
let fd_entry = wasi_try_ok!(state.fs.get_fd(fd));
fd_entry.offset.load(Ordering::Acquire) as usize
fd_entry.inner.offset.load(Ordering::Acquire) as usize
};
let bytes_written = wasi_try_ok!(fd_write_internal::<M>(
@@ -135,11 +135,11 @@ pub(crate) fn fd_write_internal<M: MemorySize>(
let is_stdio = fd_entry.is_stdio;
let bytes_written = {
if !is_stdio && !fd_entry.rights.contains(Rights::FD_WRITE) {
if !is_stdio && !fd_entry.inner.rights.contains(Rights::FD_WRITE) {
return Ok(Err(Errno::Access));
}
let fd_flags = fd_entry.flags;
let fd_flags = fd_entry.inner.flags;
let mut memory = unsafe { env.memory_view(&ctx) };
let (bytes_written, is_file, can_snapshot) = {
@@ -153,7 +153,7 @@ pub(crate) fn fd_write_internal<M: MemorySize>(
let res = __asyncify_light(
env,
if fd_entry.flags.contains(Fdflags::NONBLOCK) {
if fd_entry.inner.flags.contains(Fdflags::NONBLOCK) {
Some(Duration::ZERO)
} else {
None
@@ -161,10 +161,10 @@ pub(crate) fn fd_write_internal<M: MemorySize>(
async {
let mut handle = handle.write().unwrap();
if !is_stdio {
if fd_entry.flags.contains(Fdflags::APPEND) {
if fd_entry.inner.flags.contains(Fdflags::APPEND) {
// `fdflags::append` means we need to seek to the end before writing.
offset = fd_entry.inode.stat.read().unwrap().st_size;
fd_entry.offset.store(offset, Ordering::Release);
fd_entry.inner.offset.store(offset, Ordering::Release);
}
handle
@@ -431,7 +431,7 @@ pub(crate) fn fd_write_internal<M: MemorySize>(
// fetch_add returns the previous value, we have to add bytes_written again here
+ bytes_written
} else {
fd_entry.offset.load(Ordering::Acquire)
fd_entry.inner.offset.load(Ordering::Acquire)
};
// we set the size but we don't return any errors if it fails as

View File

@@ -55,7 +55,11 @@ pub(crate) fn path_create_directory_internal(
let (memory, state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) };
let working_dir = state.fs.get_fd(fd)?;
if !working_dir.rights.contains(Rights::PATH_CREATE_DIRECTORY) {
if !working_dir
.inner
.rights
.contains(Rights::PATH_CREATE_DIRECTORY)
{
trace!("working directory (fd={fd}) has no rights to create a directory");
return Err(Errno::Access);
}

View File

@@ -69,7 +69,7 @@ pub(crate) fn path_filestat_get_internal(
) -> Result<Filestat, Errno> {
let root_dir = state.fs.get_fd(fd)?;
if !root_dir.rights.contains(Rights::PATH_FILESTAT_GET) {
if !root_dir.inner.rights.contains(Rights::PATH_FILESTAT_GET) {
return Err(Errno::Access);
}
let file_inode = state.fs.get_inode_at_path(

View File

@@ -79,7 +79,11 @@ pub(crate) fn path_filestat_set_times_internal(
let (memory, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) };
let fd_entry = state.fs.get_fd(fd)?;
let fd_inode = fd_entry.inode;
if !fd_entry.rights.contains(Rights::PATH_FILESTAT_SET_TIMES) {
if !fd_entry
.inner
.rights
.contains(Rights::PATH_FILESTAT_SET_TIMES)
{
return Err(Errno::Access);
}
if (fst_flags.contains(Fstflags::SET_ATIM) && fst_flags.contains(Fstflags::SET_ATIM_NOW))

View File

@@ -81,8 +81,8 @@ pub(crate) fn path_link_internal(
let source_fd = state.fs.get_fd(old_fd)?;
let target_fd = state.fs.get_fd(new_fd)?;
if !source_fd.rights.contains(Rights::PATH_LINK_SOURCE)
|| !target_fd.rights.contains(Rights::PATH_LINK_TARGET)
if !source_fd.inner.rights.contains(Rights::PATH_LINK_SOURCE)
|| !target_fd.inner.rights.contains(Rights::PATH_LINK_TARGET)
{
return Err(Errno::Access);
}

View File

@@ -131,10 +131,10 @@ pub(crate) fn path_open_internal(
);
let working_dir = wasi_try_ok_ok!(state.fs.get_fd(dirfd));
let working_dir_rights_inheriting = working_dir.rights_inheriting;
let working_dir_rights_inheriting = working_dir.inner.rights_inheriting;
// ASSUMPTION: open rights apply recursively
if !working_dir.rights.contains(Rights::PATH_OPEN) {
if !working_dir.inner.rights.contains(Rights::PATH_OPEN) {
return Ok(Err(Errno::Access));
}
@@ -183,8 +183,8 @@ pub(crate) fn path_open_internal(
};
let parent_rights = virtual_fs::OpenOptionsConfig {
read: working_dir.rights.contains(Rights::FD_READ),
write: working_dir.rights.contains(Rights::FD_WRITE),
read: working_dir.inner.rights.contains(Rights::FD_READ),
write: working_dir.inner.rights.contains(Rights::FD_WRITE),
// The parent is a directory, which is why these options
// aren't inherited from the parent (append / truncate doesn't work on directories)
create_new: true,
@@ -244,6 +244,8 @@ pub(crate) fn path_open_internal(
if minimum_rights.truncate {
open_flags |= Fd::TRUNCATE;
}
// TODO: I strongly suspect that assigning the handle unconditionally
// breaks opening the same file multiple times.
*handle = Some(Arc::new(std::sync::RwLock::new(wasi_try_ok_ok!(
open_options.open(&path).map_err(fs_error_into_wasi_err)
))));

View File

@@ -31,7 +31,7 @@ pub fn path_readlink<M: MemorySize>(
let (memory, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) };
let base_dir = wasi_try!(state.fs.get_fd(dir_fd));
if !base_dir.rights.contains(Rights::PATH_READLINK) {
if !base_dir.inner.rights.contains(Rights::PATH_READLINK) {
return Errno::Access;
}
let mut path_str = unsafe { get_input_str!(&memory, path, path_len) };

View File

@@ -65,11 +65,11 @@ pub fn path_rename_internal(
{
let source_fd = wasi_try_ok!(state.fs.get_fd(source_fd));
if !source_fd.rights.contains(Rights::PATH_RENAME_SOURCE) {
if !source_fd.inner.rights.contains(Rights::PATH_RENAME_SOURCE) {
return Ok(Errno::Access);
}
let target_fd = wasi_try_ok!(state.fs.get_fd(target_fd));
if !target_fd.rights.contains(Rights::PATH_RENAME_TARGET) {
if !target_fd.inner.rights.contains(Rights::PATH_RENAME_TARGET) {
return Ok(Errno::Access);
}
}

View File

@@ -61,7 +61,7 @@ pub fn path_symlink_internal(
let (memory, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) };
let base_fd = state.fs.get_fd(fd)?;
if !base_fd.rights.contains(Rights::PATH_SYMLINK) {
if !base_fd.inner.rights.contains(Rights::PATH_SYMLINK) {
return Err(Errno::Access);
}

View File

@@ -21,7 +21,7 @@ pub fn path_unlink_file<M: MemorySize>(
let (memory, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) };
let base_dir = wasi_try_ok!(state.fs.get_fd(fd));
if !base_dir.rights.contains(Rights::PATH_UNLINK_FILE) {
if !base_dir.inner.rights.contains(Rights::PATH_UNLINK_FILE) {
return Ok(Errno::Access);
}
let path_str = unsafe { get_input_str_ok!(&memory, path, path_len) };

View File

@@ -179,7 +179,7 @@ pub(crate) fn poll_fd_guard(
.map_err(fs_error_into_wasi_err)?,
_ => {
let fd_entry = state.fs.get_fd(fd)?;
if !fd_entry.rights.contains(Rights::POLL_FD_READWRITE) {
if !fd_entry.inner.rights.contains(Rights::POLL_FD_READWRITE) {
return Err(Errno::Access);
}
let inode = fd_entry.inode;
@@ -256,7 +256,7 @@ where
Ok(a) => a,
Err(err) => return Ok(err),
};
if !fd_entry.rights.contains(Rights::POLL_FD_READWRITE) {
if !fd_entry.inner.rights.contains(Rights::POLL_FD_READWRITE) {
return Ok(Errno::Access);
}
}
@@ -274,7 +274,7 @@ where
Ok(a) => a,
Err(err) => return Ok(err),
};
if !fd_entry.rights.contains(Rights::POLL_FD_READWRITE) {
if !fd_entry.inner.rights.contains(Rights::POLL_FD_READWRITE) {
return Ok(Errno::Access);
}
}

View File

@@ -126,7 +126,7 @@ pub(crate) fn sock_accept_internal(
sock,
Rights::SOCK_ACCEPT,
move |socket, fd| async move {
if fd.flags.contains(Fdflags::NONBLOCK) {
if fd.inner.flags.contains(Fdflags::NONBLOCK) {
fd_flags.set(Fdflags::NONBLOCK, true);
nonblocking = true;
}

View File

@@ -132,7 +132,7 @@ pub(super) fn sock_recv_internal<M: MemorySize>(
.access()
.map_err(mem_error_to_wasi)?;
let nonblocking = fd.flags.contains(Fdflags::NONBLOCK);
let nonblocking = fd.inner.flags.contains(Fdflags::NONBLOCK);
let timeout = socket
.opt_time(TimeType::ReadTimeout)
.ok()

View File

@@ -74,7 +74,7 @@ pub(super) fn sock_recv_from_internal<M: MemorySize>(
sock,
Rights::SOCK_RECV,
|socket, fd| async move {
let nonblocking = fd.flags.contains(Fdflags::NONBLOCK);
let nonblocking = fd.inner.flags.contains(Fdflags::NONBLOCK);
let timeout = socket
.opt_time(TimeType::ReadTimeout)
.ok()
@@ -99,7 +99,7 @@ pub(super) fn sock_recv_from_internal<M: MemorySize>(
sock,
Rights::SOCK_RECV_FROM,
|socket, fd| async move {
let nonblocking = fd.flags.contains(Fdflags::NONBLOCK);
let nonblocking = fd.inner.flags.contains(Fdflags::NONBLOCK);
let timeout = socket
.opt_time(TimeType::ReadTimeout)
.ok()

View File

@@ -39,7 +39,7 @@ pub fn sock_send<M: MemorySize>(
let inodes = state.inodes.clone();
let fd_entry = wasi_try_ok!(state.fs.get_fd(fd));
fd_entry.offset.load(Ordering::Acquire) as usize
fd_entry.inner.offset.load(Ordering::Acquire) as usize
};
wasi_try_ok!(fd_write_internal::<M>(
@@ -100,7 +100,7 @@ pub(crate) fn sock_send_internal<M: MemorySize>(
sock,
Rights::SOCK_SEND,
|socket, fd| async move {
let nonblocking = fd.flags.contains(Fdflags::NONBLOCK);
let nonblocking = fd.inner.flags.contains(Fdflags::NONBLOCK);
let timeout = socket
.opt_time(TimeType::WriteTimeout)
.ok()

View File

@@ -75,7 +75,7 @@ pub(crate) fn sock_send_file_internal(
count -= sub_count;
let fd_entry = wasi_try_ok_ok!(state.fs.get_fd(in_fd));
let fd_flags = fd_entry.flags;
let fd_flags = fd_entry.inner.flags;
let data = {
match in_fd {
@@ -95,12 +95,12 @@ pub(crate) fn sock_send_file_internal(
}
__WASI_STDOUT_FILENO | __WASI_STDERR_FILENO => return Ok(Err(Errno::Inval)),
_ => {
if !fd_entry.rights.contains(Rights::FD_READ) {
if !fd_entry.inner.rights.contains(Rights::FD_READ) {
// TODO: figure out the error to return when lacking rights
return Ok(Err(Errno::Access));
}
let offset = fd_entry.offset.load(Ordering::Acquire) as usize;
let offset = fd_entry.inner.offset.load(Ordering::Acquire) as usize;
let inode = fd_entry.inode;
let data = {
let mut guard = inode.write();

View File

@@ -93,7 +93,7 @@ pub(crate) fn sock_send_to_internal<M: MemorySize>(
sock,
Rights::SOCK_SEND_TO,
|socket, fd| async move {
let nonblocking = fd.flags.contains(Fdflags::NONBLOCK);
let nonblocking = fd.inner.flags.contains(Fdflags::NONBLOCK);
let timeout = socket
.opt_time(TimeType::WriteTimeout)
.ok()

View File

@@ -0,0 +1,102 @@
/// This test makes sure writing to the same file, when referred to
/// by multiple FDs (some of which are shared), works correctly. The
/// trace logs generated by Wasmer are also inspected to make sure
/// the file is closed properly.
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/wait.h>
void error_exit(const char *message)
{
perror(message);
exit(-1);
}
int main(int argc, char *argv[])
{
int fd1 = open("./output", O_CREAT | O_EXCL | O_WRONLY);
if (fd1 < 0)
{
error_exit("open");
}
int fd2 = dup(fd1);
if (fd2 < 0 || fd2 == fd1)
{
error_exit("parent dup");
}
if (!write(fd2, "parent 2\n", 9))
{
error_exit("parent write 2");
}
pid_t pid = fork();
if (pid == -1)
{
error_exit("fork");
}
else if (pid == 0)
{
if (close(fd2))
{
error_exit("child close parent fd2");
}
fd2 = dup(fd1);
if (fd2 < 0 || fd2 == fd1)
{
error_exit("child dup");
}
if (!write(fd1, "child 1\n", 8))
{
error_exit("child write 1");
}
if (close(fd1))
{
error_exit("child close 1");
}
if (!write(fd2, "child 2\n", 8))
{
error_exit("child write 2");
}
if (close(fd2))
{
error_exit("child close 2");
}
exit(0);
}
else
{
if (close(fd2))
{
error_exit("parent close 2");
}
int status;
waitpid(pid, &status, 0);
if (status)
{
error_exit("child exit code");
}
if (!write(fd1, "parent 1\n", 9))
{
error_exit("parent write 1");
}
// These prints can be inspected to make sure the file was
// closed as a result of the last close() call, see run.sh
printf("closing last fd\n");
if (close(fd1))
{
error_exit("parent close 1");
}
printf("last fd closed\n");
}
}

13
tests/wasix/shared-fd/run.sh Executable file
View File

@@ -0,0 +1,13 @@
#!/bin/bash
set -eu
rm -f output*
RUST_LOG=virtual_fs=trace $WASMER run main.wasm --dir . &> output-wasmer-log
cat output | grep "parent 1" >/dev/null && \
cat output | grep "parent 2" >/dev/null && \
cat output | grep "child 1" >/dev/null && \
cat output | grep "child 2" >/dev/null && \
awk '/closing last fd/{m1=1} /Closing host file.*shared-fd\/output/{if(m1) m2=1} /last fd closed/{if(m2) print "found all lines"}' output-wasmer-log | grep "found all lines" >/dev/null