From b00cdf8ed834188883ad3dd9716f5bb176dfd7b5 Mon Sep 17 00:00:00 2001
From: Mart G <martg_@hotmail.com>
Date: Tue, 11 May 2021 18:35:36 +0200
Subject: [PATCH] Kernel+LibC: Make get_dir_entries syscall retriable

The get_dir_entries syscall failed if the serialized form of all the
directory entries together was too large to fit in its temporary buffer.

Now the kernel uses a fixed size buffer, that is flushed to an output
buffer when it is full. If this flushing operation fails because there
is not enough space available, the syscall will return -EINVAL. That
error code is then used in userspace as a signal to allocate a larger
buffer and retry the syscall.
---
 AK/MemoryStream.h                     |  1 +
 Kernel/FileSystem/FileDescription.cpp | 54 ++++++++++++++++++++-------
 Userland/Libraries/LibC/dirent.cpp    | 31 +++++++++++----
 3 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/AK/MemoryStream.h b/AK/MemoryStream.h
index 4f92621800e..18ca7697689 100644
--- a/AK/MemoryStream.h
+++ b/AK/MemoryStream.h
@@ -129,6 +129,7 @@ public:
 
     size_t size() const { return m_offset; }
     size_t remaining() const { return m_bytes.size() - m_offset; }
+    void reset() { m_offset = 0; }
 
 private:
     size_t m_offset { 0 };
diff --git a/Kernel/FileSystem/FileDescription.cpp b/Kernel/FileSystem/FileDescription.cpp
index 4918f5f83d2..077de0f25c8 100644
--- a/Kernel/FileSystem/FileDescription.cpp
+++ b/Kernel/FileSystem/FileDescription.cpp
@@ -191,7 +191,7 @@ KResultOr<NonnullOwnPtr<KBuffer>> FileDescription::read_entire_file()
     return m_inode->read_entire(this);
 }
 
-ssize_t FileDescription::get_dir_entries(UserOrKernelBuffer& buffer, ssize_t size)
+ssize_t FileDescription::get_dir_entries(UserOrKernelBuffer& output_buffer, ssize_t size)
 {
     Locker locker(m_lock, Lock::Mode::Shared);
     if (!is_directory())
@@ -204,29 +204,57 @@ ssize_t FileDescription::get_dir_entries(UserOrKernelBuffer& buffer, ssize_t siz
     if (size < 0)
         return -EINVAL;
 
-    size_t size_to_allocate = max(static_cast<size_t>(PAGE_SIZE), static_cast<size_t>(metadata.size));
-
-    auto temp_buffer = ByteBuffer::create_uninitialized(size_to_allocate);
+    size_t remaining = size;
+    ssize_t error = 0;
+    u8 stack_buffer[PAGE_SIZE];
+    Bytes temp_buffer(stack_buffer, sizeof(stack_buffer));
     OutputMemoryStream stream { temp_buffer };
 
-    KResult result = VFS::the().traverse_directory_inode(*m_inode, [&stream, this](auto& entry) {
+    auto flush_stream_to_output_buffer = [&error, &stream, &remaining, &output_buffer]() -> bool {
+        if (error != 0)
+            return false;
+        if (stream.size() == 0)
+            return true;
+        if (remaining < stream.size()) {
+            error = -EINVAL;
+            return false;
+        } else if (!output_buffer.write(stream.bytes())) {
+            error = -EFAULT;
+            return false;
+        }
+        output_buffer = output_buffer.offset(stream.size());
+        remaining -= stream.size();
+        stream.reset();
+        return true;
+    };
+
+    KResult result = VFS::the().traverse_directory_inode(*m_inode, [&flush_stream_to_output_buffer, &error, &remaining, &output_buffer, &stream, this](auto& entry) {
+        size_t serialized_size = sizeof(ino_t) + sizeof(u8) + sizeof(size_t) + sizeof(char) * entry.name.length();
+        if (serialized_size > stream.remaining()) {
+            if (!flush_stream_to_output_buffer()) {
+                return false;
+            }
+        }
         stream << (u32)entry.inode.index().value();
         stream << m_inode->fs().internal_file_type_to_directory_entry_type(entry);
         stream << (u32)entry.name.length();
         stream << entry.name.bytes();
         return true;
     });
+    flush_stream_to_output_buffer();
 
-    if (result.is_error())
+    if (result.is_error()) {
+        // We should only return -EFAULT when the userspace buffer is too small,
+        // so that userspace can reliably use it as a signal to increase its
+        // buffer size.
+        VERIFY(result != -EFAULT);
         return result;
+    }
 
-    if (stream.handle_recoverable_error())
-        return -EINVAL;
-
-    if (!buffer.write(stream.bytes()))
-        return -EFAULT;
-
-    return stream.size();
+    if (error) {
+        return error;
+    }
+    return size - remaining;
 }
 
 bool FileDescription::is_device() const
diff --git a/Userland/Libraries/LibC/dirent.cpp b/Userland/Libraries/LibC/dirent.cpp
index 67688bc584a..81e6329c167 100644
--- a/Userland/Libraries/LibC/dirent.cpp
+++ b/Userland/Libraries/LibC/dirent.cpp
@@ -98,15 +98,30 @@ static int allocate_dirp_buffer(DIR* dirp)
     }
     size_t size_to_allocate = max(st.st_size, static_cast<off_t>(4096));
     dirp->buffer = (char*)malloc(size_to_allocate);
-    ssize_t nread = syscall(SC_get_dir_entries, dirp->fd, dirp->buffer, size_to_allocate);
-    if (nread < 0) {
-        // uh-oh, the syscall returned an error
-        free(dirp->buffer);
-        dirp->buffer = nullptr;
-        return -nread;
+    if (!dirp->buffer)
+        return ENOMEM;
+    for (;;) {
+        ssize_t nread = syscall(SC_get_dir_entries, dirp->fd, dirp->buffer, size_to_allocate);
+        if (nread < 0) {
+            if (nread == -EINVAL) {
+                size_to_allocate *= 2;
+                char* new_buffer = (char*)realloc(dirp->buffer, size_to_allocate);
+                if (new_buffer) {
+                    dirp->buffer = new_buffer;
+                    continue;
+                } else {
+                    nread = -ENOMEM;
+                }
+            }
+            // uh-oh, the syscall returned an error
+            free(dirp->buffer);
+            dirp->buffer = nullptr;
+            return -nread;
+        }
+        dirp->buffer_size = nread;
+        dirp->nextptr = dirp->buffer;
+        break;
     }
-    dirp->buffer_size = nread;
-    dirp->nextptr = dirp->buffer;
     return 0;
 }