diff --git a/crates/test-programs/wasi-tests/src/bin/fd_readdir.rs b/crates/test-programs/wasi-tests/src/bin/fd_readdir.rs index 8790ea0218..6e4345107a 100644 --- a/crates/test-programs/wasi-tests/src/bin/fd_readdir.rs +++ b/crates/test-programs/wasi-tests/src/bin/fd_readdir.rs @@ -26,15 +26,20 @@ impl<'a> Iterator for ReadDir<'a> { fn next(&mut self) -> Option { unsafe { - if self.buf.is_empty() { + if self.buf.len() < mem::size_of::() { return None; } // Read the data let dirent_ptr = self.buf.as_ptr() as *const wasi::Dirent; let dirent = dirent_ptr.read_unaligned(); + + if self.buf.len() < mem::size_of::() + dirent.d_namlen as usize { + return None; + } + let name_ptr = dirent_ptr.offset(1) as *const u8; - // NOTE Linux syscall returns a NULL-terminated name, but WASI doesn't + // NOTE Linux syscall returns a NUL-terminated name, but WASI doesn't let namelen = dirent.d_namlen as usize; let slice = slice::from_raw_parts(name_ptr, namelen); let name = str::from_utf8(slice).expect("invalid utf8").to_owned(); @@ -48,21 +53,24 @@ impl<'a> Iterator for ReadDir<'a> { } } -unsafe fn exec_fd_readdir(fd: wasi::Fd, cookie: wasi::Dircookie) -> Vec { +/// Return the entries plus a bool indicating EOF. +unsafe fn exec_fd_readdir(fd: wasi::Fd, cookie: wasi::Dircookie) -> (Vec, bool) { let mut buf: [u8; BUF_LEN] = [0; BUF_LEN]; let bufused = wasi::fd_readdir(fd, buf.as_mut_ptr(), BUF_LEN, cookie).expect("failed fd_readdir"); let sl = slice::from_raw_parts(buf.as_ptr(), min(BUF_LEN, bufused)); let dirs: Vec<_> = ReadDir::from_slice(sl).collect(); - dirs + let eof = bufused < BUF_LEN; + (dirs, eof) } unsafe fn test_fd_readdir(dir_fd: wasi::Fd) { let stat = wasi::fd_filestat_get(dir_fd).expect("failed filestat"); // Check the behavior in an empty directory - let mut dirs = exec_fd_readdir(dir_fd, 0); + let (mut dirs, eof) = exec_fd_readdir(dir_fd, 0); + assert!(eof, "expected to read the entire directory"); dirs.sort_by_key(|d| d.name.clone()); assert_eq!(dirs.len(), 2, "expected two entries in an empty directory"); let mut dirs = dirs.into_iter(); @@ -105,9 +113,11 @@ unsafe fn test_fd_readdir(dir_fd: wasi::Fd) { ); let stat = wasi::fd_filestat_get(file_fd).expect("failed filestat"); + wasi::fd_close(file_fd).expect("closing a file"); // Execute another readdir - let mut dirs = exec_fd_readdir(dir_fd, 0); + let (mut dirs, eof) = exec_fd_readdir(dir_fd, 0); + assert!(eof, "expected to read the entire directory"); assert_eq!(dirs.len(), 3, "expected three entries"); // Save the data about the last entry. We need to do it before sorting. let lastfile_cookie = dirs[1].dirent.d_next; @@ -130,9 +140,56 @@ unsafe fn test_fd_readdir(dir_fd: wasi::Fd) { assert_eq!(dir.dirent.d_ino, stat.ino); // check if cookie works as expected - let dirs = exec_fd_readdir(dir_fd, lastfile_cookie); + let (dirs, eof) = exec_fd_readdir(dir_fd, lastfile_cookie); + assert!(eof, "expected to read the entire directory"); assert_eq!(dirs.len(), 1, "expected one entry"); assert_eq!(dirs[0].name, lastfile_name, "name of the only entry"); + + wasi::path_unlink_file(dir_fd, "file").expect("removing a file"); +} + +unsafe fn test_fd_readdir_lots(dir_fd: wasi::Fd) { + let stat = wasi::fd_filestat_get(dir_fd).expect("failed filestat"); + + // Add a file and check the behavior + for count in 0..1000 { + let file_fd = wasi::path_open( + dir_fd, + 0, + &format!("file.{}", count), + wasi::OFLAGS_CREAT, + wasi::RIGHTS_FD_READ + | wasi::RIGHTS_FD_WRITE + | wasi::RIGHTS_FD_READDIR + | wasi::RIGHTS_FD_FILESTAT_GET, + 0, + 0, + ) + .expect("failed to create file"); + assert_gt!( + file_fd, + libc::STDERR_FILENO as wasi::Fd, + "file descriptor range check", + ); + wasi::fd_close(file_fd).expect("closing a file"); + } + + // Count the entries to ensure that we see the correct number. + let mut total = 0; + let mut cookie = 0; + loop { + let (dirs, eof) = exec_fd_readdir(dir_fd, cookie); + total += dirs.len(); + if eof { + break; + } + cookie = dirs[dirs.len()-1].dirent.d_next; + } + assert_eq!(total, 1002, "expected 1000 entries plus . and .."); + + for count in 0..1000 { + wasi::path_unlink_file(dir_fd, &format!("file.{}", count)).expect("removing a file"); + } } fn main() { @@ -156,4 +213,5 @@ fn main() { // Run the tests. unsafe { test_fd_readdir(dir_fd) } + unsafe { test_fd_readdir_lots(dir_fd) } } diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index f5b80eb5a1..769fc3601b 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -7,6 +7,7 @@ use crate::{path, sched, Error, Result, WasiCtx}; use std::convert::TryInto; use std::io::{self, SeekFrom}; use std::ops::Deref; +use std::cmp::min; use tracing::{debug, trace}; use wiggle::{GuestPtr, GuestSliceMut}; @@ -304,15 +305,32 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { let name_raw = name.as_bytes(); let name_len = name_raw.len().try_into()?; let offset = dirent_len.checked_add(name_len).ok_or(Error::Overflow)?; - if (buf_len - bufused) < offset { - break; - } else { - buf.as_array(dirent_len).copy_from_slice(&dirent_raw)?; - buf = buf.add(dirent_len)?; - buf.as_array(name_len).copy_from_slice(name_raw)?; - buf = buf.add(name_len)?; - bufused += offset; + + // Copy as many bytes of the dirent as we can, up to the end of the buffer. + let dirent_copy_len = min(dirent_len, buf_len - bufused); + buf.as_array(dirent_copy_len).copy_from_slice(&dirent_raw[..dirent_copy_len as usize])?; + + // If the dirent struct wasn't copied entirely, return that we + // filled the buffer, which tells libc that we're not at EOF. + if dirent_copy_len < dirent_len { + return Ok(buf_len); } + + buf = buf.add(dirent_copy_len)?; + + // Copy as many bytes of the name as we can, up to the end of the buffer. + let name_copy_len = min(name_len, buf_len - bufused); + buf.as_array(name_copy_len).copy_from_slice(&name_raw[..name_copy_len as usize])?; + + // If the dirent struct wasn't copied entirely, return that we + // filled the buffer, which tells libc that we're not at EOF. + if name_copy_len < name_len { + return Ok(buf_len); + } + + buf = buf.add(name_copy_len)?; + + bufused += offset; } Ok(bufused)