Fix the return value from wasi-common's fd_readdir.

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<DirEntry> {
         unsafe {
-            if self.buf.is_empty() {
+            if self.buf.len() < mem::size_of::<wasi::Dirent>() {
                 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::<wasi::Dirent>() + 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<DirEntry> {
+/// Return the entries plus a bool indicating EOF.
+unsafe fn exec_fd_readdir(fd: wasi::Fd, cookie: wasi::Dircookie) -> (Vec<DirEntry>, 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,54 @@ 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) {
+    // 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 +211,5 @@ fn main() {
 
     // Run the tests.
     unsafe { test_fd_readdir(dir_fd) }
+    unsafe { test_fd_readdir_lots(dir_fd) }
 }

crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs

@@ -4,6 +4,7 @@ use crate::sys::{clock, poll};
 use crate::wasi::types;
 use crate::wasi::wasi_snapshot_preview1::WasiSnapshotPreview1;
 use crate::{path, sched, Error, Result, WasiCtx};
+use std::cmp::min;
 use std::convert::TryInto;
 use std::io::{self, SeekFrom};
 use std::ops::Deref;
@@ -304,15 +305,34 @@ 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)