Skip to content

Commit

Permalink
Force some more permission checks with 0-length writes
Browse files Browse the repository at this point in the history 
When a 0-length write is performed try to send the write all the way to
the underlying file descriptor to at least check that it's valid to
write.

Closes bytecodealliance#8818
  • Loading branch information
@alexcrichton
alexcrichton committed Jun 17, 2024
1 parent 0f48f93 commit ec9e220
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 48 deletions.
24 changes: 24 additions & 0 deletions crates/test-programs/src/bin/preview1_file_write.rs
View file
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");
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
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
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
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
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

0 comments on commit ec9e220

Please sign in to comment.