Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Force some more permission checks with 0-length writes #8822

Merged
merged 3 commits into from
Jun 17, 2024
Merged

Force some more permission checks with 0-length writes #8822

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec9e220
Force some more permission checks with 0-length writes
alexcrichton Jun 17, 2024
e80df1e
Update crates/test-programs/src/bin/preview1_file_write.rs
alexcrichton Jun 17, 2024
4d748d7
Allow a second error for Windows as well
alexcrichton Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions crates/test-programs/src/bin/preview1_file_write.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,30 @@ unsafe fn test_file_long_write(dir_fd: wasi::Fd, filename: &str) {
assert_eq!(buffer, &content[end_cursor..], "contents of end read chunk");

wasi::fd_close(file_fd).expect("closing the file");

// Open a file for writing
let filename = "test-zero-write-fails.txt";
let file_fd = wasi::path_open(
dir_fd,
0,
filename,
wasi::OFLAGS_CREAT,
wasi::RIGHTS_FD_WRITE,
0,
0,
)
.expect("creating a file for writing");
wasi::fd_close(file_fd).expect("closing the file");
let file_fd = wasi::path_open(dir_fd, 0, filename, 0, wasi::RIGHTS_FD_READ, 0, 0)
.expect("creating a file for writing");
alexcrichton marked this conversation as resolved.
Show resolved Hide resolved
let res = wasi::fd_write(
file_fd,
&[wasi::Ciovec {
buf: 3 as *const u8,
buf_len: 0,
}],
);
assert_eq!(res, Err(wasi::ERRNO_BADF));
}

fn main() {
Expand Down
25 changes: 10 additions & 15 deletions crates/wasi-preview1-component-adapter/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,8 +1047,8 @@ pub unsafe extern "C" fn fd_pread(
nread: *mut Size,
) -> Errno {
cfg_filesystem_available! {
// Advance to the first non-empty buffer.
while iovs_len != 0 && (*iovs_ptr).buf_len == 0 {
// Skip leading non-empty buffers.
while iovs_len > 1 && (*iovs_ptr).buf_len == 0 {
iovs_ptr = iovs_ptr.add(1);
iovs_len -= 1;
}
Expand Down Expand Up @@ -1162,8 +1162,8 @@ pub unsafe extern "C" fn fd_pwrite(
nwritten: *mut Size,
) -> Errno {
cfg_filesystem_available! {
// Advance to the first non-empty buffer.
while iovs_len != 0 && (*iovs_ptr).buf_len == 0 {
// Skip leading non-empty buffers.
while iovs_len > 1 && (*iovs_ptr).buf_len == 0 {
iovs_ptr = iovs_ptr.add(1);
iovs_len -= 1;
}
Expand Down Expand Up @@ -1194,8 +1194,8 @@ pub unsafe extern "C" fn fd_read(
mut iovs_len: usize,
nread: *mut Size,
) -> Errno {
// Advance to the first non-empty buffer.
while iovs_len != 0 && (*iovs_ptr).buf_len == 0 {
// Skip leading non-empty buffers.
while iovs_len > 1 && (*iovs_ptr).buf_len == 0 {
iovs_ptr = iovs_ptr.add(1);
iovs_len -= 1;
}
Expand Down Expand Up @@ -1243,9 +1243,6 @@ pub unsafe extern "C" fn fd_read(
if let StreamType::File(file) = &streams.type_ {
file.position
.set(file.position.get() + data.len() as filesystem::Filesize);
if len == 0 {
return Err(ERRNO_INTR);
}
}

let len = data.len();
Expand Down Expand Up @@ -1618,8 +1615,8 @@ pub unsafe extern "C" fn fd_write(
return ERRNO_IO;
}

// Advance to the first non-empty buffer.
while iovs_len != 0 && (*iovs_ptr).buf_len == 0 {
// Skip leading empty buffers.
while iovs_len > 1 && (*iovs_ptr).buf_len == 0 {
iovs_ptr = iovs_ptr.add(1);
iovs_len -= 1;
}
Expand Down Expand Up @@ -2506,11 +2503,12 @@ impl BlockingMode {
match self {
BlockingMode::Blocking => {
let total = bytes.len();
while !bytes.is_empty() {
loop {
let len = bytes.len().min(4096);
let (chunk, rest) = bytes.split_at(len);
bytes = rest;
match output_stream.blocking_write_and_flush(chunk) {
Ok(()) if bytes.is_empty() => break,
Ok(()) => {}
Err(streams::StreamError::Closed) => return Err(ERRNO_IO),
Err(streams::StreamError::LastOperationFailed(e)) => {
Expand All @@ -2531,9 +2529,6 @@ impl BlockingMode {
};

let len = bytes.len().min(permit as usize);
if len == 0 {
return Ok(0);
}

match output_stream.write(&bytes[..len]) {
Ok(_) => {}
Expand Down
14 changes: 8 additions & 6 deletions crates/wasi/src/filesystem.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,32 +335,34 @@ impl HostOutputStream for FileOutputStream {
}
}

if buf.is_empty() {
return Ok(());
}

let m = self.mode;
let result = self.file._spawn_blocking(move |f| {
match m {
FileOutputMode::Position(mut p) => {
let mut total = 0;
let mut buf = buf;
while !buf.is_empty() {
loop {
let nwritten = f.write_at(buf.as_ref(), p)?;
// afterwards buf contains [nwritten, len):
let _ = buf.split_to(nwritten);
p += nwritten as u64;
total += nwritten;
if buf.is_empty() {
break;
}
}
Ok(total)
}
FileOutputMode::Append => {
let mut total = 0;
let mut buf = buf;
while !buf.is_empty() {
loop {
let nwritten = f.append(buf.as_ref())?;
let _ = buf.split_to(nwritten);
total += nwritten;
if buf.is_empty() {
break;
}
}
Ok(total)
}
Expand Down
5 changes: 4 additions & 1 deletion crates/wasi/src/host/io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,14 @@ where
}

let mut bytes = bytes::Bytes::from(bytes);
while !bytes.is_empty() {
loop {
let permit = s.write_ready().await?;
let len = bytes.len().min(permit);
let chunk = bytes.split_to(len);
s.write(chunk)?;
if bytes.is_empty() {
break;
}
}

s.flush()?;
Expand Down
42 changes: 16 additions & 26 deletions crates/wasi/src/preview1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,34 +1047,36 @@ fn read_string<'a>(memory: &'a GuestMemory<'_>, ptr: GuestPtr<str>) -> Result<St
Ok(memory.as_cow_str(ptr)?.into_owned())
}

// Find first non-empty buffer.
// Returns the first non-empty buffer in `ciovs` or a single empty buffer if
// they're all empty.
fn first_non_empty_ciovec(
memory: &GuestMemory<'_>,
ciovs: types::CiovecArray,
) -> Result<Option<GuestPtr<[u8]>>> {
) -> Result<GuestPtr<[u8]>> {
for iov in ciovs.iter() {
let iov = memory.read(iov?)?;
if iov.buf_len == 0 {
continue;
}
return Ok(Some(iov.buf.as_array(iov.buf_len)));
return Ok(iov.buf.as_array(iov.buf_len));
}
Ok(None)
Ok(GuestPtr::new((0, 0)))
}

// Find first non-empty buffer.
// Returns the first non-empty buffer in `iovs` or a single empty buffer if
// they're all empty.
fn first_non_empty_iovec(
memory: &GuestMemory<'_>,
iovs: types::IovecArray,
) -> Result<Option<GuestPtr<[u8]>>> {
) -> Result<GuestPtr<[u8]>> {
for iov in iovs.iter() {
let iov = memory.read(iov?)?;
if iov.buf_len == 0 {
continue;
}
return Ok(Some(iov.buf.as_array(iov.buf_len)));
return Ok(iov.buf.as_array(iov.buf_len));
}
Ok(None)
Ok(GuestPtr::new((0, 0)))
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -1612,9 +1614,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
drop(t);
let pos = position.load(Ordering::Relaxed);
let file = self.table().get(&fd)?.file()?;
let Some(iov) = first_non_empty_iovec(memory, iovs)? else {
return Ok(0);
};
let iov = first_non_empty_iovec(memory, iovs)?;
let bytes_read = match (file.as_blocking_file(), memory.as_slice_mut(iov)?) {
// Try to read directly into wasm memory where possible
// when the current thread can block and additionally wasm
Expand Down Expand Up @@ -1653,9 +1653,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
Descriptor::Stdin { stream, .. } => {
let stream = stream.borrowed();
drop(t);
let Some(buf) = first_non_empty_iovec(memory, iovs)? else {
return Ok(0);
};
let buf = first_non_empty_iovec(memory, iovs)?;
let read = BlockingMode::Blocking
.read(&mut self.as_wasi_impl(), stream, buf.len().try_into()?)
.await?;
Expand Down Expand Up @@ -1690,9 +1688,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
let fd = fd.borrowed();
let blocking_mode = *blocking_mode;
drop(t);
let Some(buf) = first_non_empty_iovec(memory, iovs)? else {
return Ok(0);
};
let buf = first_non_empty_iovec(memory, iovs)?;

let stream = self
.as_wasi_impl()
Expand Down Expand Up @@ -1758,9 +1754,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
let append = *append;
drop(t);
let f = self.table().get(&fd)?.file()?;
let Some(buf) = first_non_empty_ciovec(memory, ciovs)? else {
return Ok(0);
};
let buf = first_non_empty_ciovec(memory, ciovs)?;

let write = move |f: &cap_std::fs::File, buf: &[u8]| {
if append {
Expand Down Expand Up @@ -1798,9 +1792,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
Descriptor::Stdout { stream, .. } | Descriptor::Stderr { stream, .. } => {
let stream = stream.borrowed();
drop(t);
let Some(buf) = first_non_empty_ciovec(memory, ciovs)? else {
return Ok(0);
};
let buf = first_non_empty_ciovec(memory, ciovs)?;
let n = BlockingMode::Blocking
.write(memory, &mut self.as_wasi_impl(), stream, buf)
.await?
Expand Down Expand Up @@ -1830,9 +1822,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
let fd = fd.borrowed();
let blocking_mode = *blocking_mode;
drop(t);
let Some(buf) = first_non_empty_ciovec(memory, ciovs)? else {
return Ok(0);
};
let buf = first_non_empty_ciovec(memory, ciovs)?;
let stream = self
.as_wasi_impl()
.write_via_stream(fd, offset)
Expand Down