From 8d8b1b8fc70ea58e261a022e9e32abd3c8d0e928 Mon Sep 17 00:00:00 2001 From: Arshia Ghafoori Date: Wed, 15 May 2024 00:03:50 +0330 Subject: [PATCH 1/2] Fix calculation of stat.st_size in the presence of combined seeks and writes --- .../src/syscalls/wasi/fd_filestat_get.rs | 2 +- lib/wasix/src/syscalls/wasi/fd_write.rs | 25 ++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/wasix/src/syscalls/wasi/fd_filestat_get.rs b/lib/wasix/src/syscalls/wasi/fd_filestat_get.rs index 46a3dc33e05..e893a35185e 100644 --- a/lib/wasix/src/syscalls/wasi/fd_filestat_get.rs +++ b/lib/wasix/src/syscalls/wasi/fd_filestat_get.rs @@ -39,7 +39,7 @@ pub(crate) fn fd_filestat_get_internal( fd: WasiFd, ) -> Result { let env = ctx.data(); - let (_, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) }; + 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) { return Err(Errno::Access); diff --git a/lib/wasix/src/syscalls/wasi/fd_write.rs b/lib/wasix/src/syscalls/wasi/fd_write.rs index 079527e5deb..d42fe2c4053 100644 --- a/lib/wasix/src/syscalls/wasi/fd_write.rs +++ b/lib/wasix/src/syscalls/wasi/fd_write.rs @@ -141,7 +141,7 @@ pub(crate) fn fd_write_internal( let fd_flags = fd_entry.flags; let mut memory = unsafe { env.memory_view(&ctx) }; - let (bytes_written, can_update_cursor, can_snapshot) = { + let (bytes_written, is_file, can_snapshot) = { let (mut memory, _) = unsafe { env.get_memory_and_wasi_state(&ctx, 0) }; let mut guard = fd_entry.inode.write(); match guard.deref_mut() { @@ -414,20 +414,33 @@ pub(crate) fn fd_write_internal( // reborrow and update the size if !is_stdio { - if can_update_cursor && should_update_cursor { + let offset = if is_file && should_update_cursor { + let bytes_written = bytes_written as u64; let mut fd_map = state.fs.fd_map.write().unwrap(); let fd_entry = wasi_try_ok_ok!(fd_map.get_mut(&fd).ok_or(Errno::Badf)); fd_entry .offset - .fetch_add(bytes_written as u64, Ordering::AcqRel); - } + .fetch_add(bytes_written, Ordering::AcqRel) + // fetch_add returns the previous value, we have to add bytes_written again here + + bytes_written + } else { + fd_entry.offset.load(Ordering::Acquire) + }; // we set the size but we don't return any errors if it fails as // pipes and sockets will not do anything with this let (mut memory, _, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) }; - // Cast is valid because we don't support 128 bit systems... - fd_entry.inode.stat.write().unwrap().st_size += bytes_written as u64; + if is_file { + let mut stat = fd_entry.inode.stat.write().unwrap(); + // If we wrote before the end, the current size is still correct. + // Otherwise, we only got as far as the current cursor. So, the + // max of the two is the correct new size. + stat.st_size = stat.st_size.max(offset); + } else { + // Cast is valid because we don't support 128 bit systems... + fd_entry.inode.stat.write().unwrap().st_size += bytes_written as u64; + } } bytes_written }; From 8ecdf5959619c1772d65748e2331e0358f813366 Mon Sep 17 00:00:00 2001 From: Arshia Ghafoori Date: Wed, 15 May 2024 12:39:07 +0330 Subject: [PATCH 2/2] Add test for file write and seek --- tests/wasi-fyi/fs_write-and-seek.rs | 39 +++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/wasi-fyi/fs_write-and-seek.rs diff --git a/tests/wasi-fyi/fs_write-and-seek.rs b/tests/wasi-fyi/fs_write-and-seek.rs new file mode 100644 index 00000000000..adab7909e51 --- /dev/null +++ b/tests/wasi-fyi/fs_write-and-seek.rs @@ -0,0 +1,39 @@ +use std::fs::{metadata, OpenOptions}; +use std::io::{Seek, SeekFrom, Write}; + +fn main() { + let mut file = OpenOptions::new() + .write(true) + .read(true) + .create(true) + .open("file") + .unwrap(); + + write!(file, "hell").unwrap(); + + // We wrote 4 bytes + let md = metadata("file").unwrap(); + assert_eq!(md.len(), 4); + + assert_eq!(file.seek(SeekFrom::Start(0)).unwrap(), 0); + + write!(file, "eh").unwrap(); + + // We overwrote the first 2 bytes, should still have 4 bytes + let md = metadata("file").unwrap(); + assert_eq!(md.len(), 4); + + assert_eq!(file.seek(SeekFrom::Start(0)).unwrap(), 0); + + write!(file, "hello").unwrap(); + + // Now we wrote past the end, should have 5 bytes + let md = metadata("file").unwrap(); + assert_eq!(md.len(), 5); + + write!(file, " world!").unwrap(); + + // We wrote past the end entirely, should have 5 + 7 = 12 bytes + let md = metadata("file").unwrap(); + assert_eq!(md.len(), 12); +}