Merge pull request #3452 from LeandroLP23/fix/kernel-windows-directory-rewind

fix(libkernel): rewind Windows directory descriptors on lseek

core/libs/prx/libkernel/CMakeLists.txt

@@ -908,6 +908,15 @@ if(BUILD_TESTING)
         set_tests_properties(guest_directory_buffer PROPERTIES ENVIRONMENT_MODIFICATION
             "PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
     endif()
+    if(WIN32)
+        add_test_executable(guest_directory_rewind_tests ${LIBS_INCLUDE_DIR}/tests/GuestDirectoryRewind.cpp)
+        target_include_directories(guest_directory_rewind_tests PRIVATE ${LIBS_INCLUDE_DIR})
+        target_link_libraries(guest_directory_rewind_tests PRIVATE libkernel libc)
+        configure_windows_unwind(guest_directory_rewind_tests)
+        add_test(NAME guest_directory_rewind COMMAND guest_directory_rewind_tests)
+        set_tests_properties(guest_directory_rewind PROPERTIES TIMEOUT 15 ENVIRONMENT_MODIFICATION
+            "PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
+    endif()
     if(NOT WIN32)
         add_test_executable(guest_directory_entry_tests ${LIBS_INCLUDE_DIR}/tests/GuestDirectoryEntries.cpp)
         target_include_directories(guest_directory_entry_tests PRIVATE ${LIBS_INCLUDE_DIR})

core/libs/prx/libkernel/File/include/DirectoryDescriptor.hpp

@@ -1,6 +1,7 @@
 #ifndef CORE_LIBS_PRX_LIBKERNEL_FILE_DIRECTORYDESCRIPTOR_HPP
 #define CORE_LIBS_PRX_LIBKERNEL_FILE_DIRECTORYDESCRIPTOR_HPP
 
+#include <cstdint>
 #include <filesystem>
 #include <optional>
 
@@ -11,6 +12,7 @@ namespace File {
 int OpenDirectoryDescriptor(const std::filesystem::path& path);
 std::optional<std::filesystem::path> DirectoryDescriptorPath(int fd);
 void ForgetDirectoryDescriptor(int fd);
+std::optional<std::int64_t> SeekDirectoryDescriptor(int fd, std::int64_t offset, int whence);
 int ReadDirectoryDescriptor(int fd, char* buf, int nbytes);
 
 }

core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp

@@ -7,11 +7,17 @@
 
 #ifdef _WIN32
 
+#include "prx/libc/include/General.hpp"
+
+#include <cerrno>
 #include <cstdint>
+#include <atomic>
 #include <cstring>
+#include <limits>
 #include <fcntl.h>
 #include <io.h>
 #include <map>
+#include <memory>
 #include <mutex>
 #include <string>
 #include <utility>
@@ -23,6 +29,7 @@ constexpr std::uint8_t GuestDirectoryType = 4;
 constexpr std::uint8_t GuestRegularType = 8;
 
 struct DirectoryState {
+    std::mutex mutex;
     std::filesystem::path path;
     std::vector<std::pair<std::string, std::uint8_t>> entries;
     std::size_t cursor = 0;
@@ -30,7 +37,26 @@ struct DirectoryState {
 };
 
 std::mutex g_mutex;
-std::map<int, DirectoryState> g_directories;
+std::map<int, std::shared_ptr<DirectoryState>> g_directories;
+std::atomic<std::size_t> g_directoryCount{0};
+
+std::shared_ptr<DirectoryState> Find(int fd) {
+    std::lock_guard lock(g_mutex);
+    const auto found = g_directories.find(fd);
+    return found == g_directories.end() ? nullptr : found->second;
+}
+
+void Load(DirectoryState& state) {
+    if (state.loaded) return;
+    state.loaded = true;
+    state.entries.emplace_back(".", GuestDirectoryType);
+    state.entries.emplace_back("..", GuestDirectoryType);
+    std::error_code error;
+    for (const auto& entry : std::filesystem::directory_iterator(state.path, error)) {
+        std::error_code typeError;
+        state.entries.emplace_back(entry.path().filename().string(), entry.is_directory(typeError) ? GuestDirectoryType : GuestRegularType);
+    }
+}
 
 }
 
@@ -40,37 +66,63 @@ int OpenDirectoryDescriptor(const std::filesystem::path& path) {
     const int fd = ::_open("NUL", _O_RDONLY | _O_BINARY);
     if (fd < 0) return -1;
     std::lock_guard lock(g_mutex);
-    g_directories[fd] = DirectoryState{path};
+    const auto state = std::make_shared<DirectoryState>();
+    state->path = path;
+    g_directories[fd] = state;
+    g_directoryCount = g_directories.size();
     return fd;
 }
 
 std::optional<std::filesystem::path> DirectoryDescriptorPath(int fd) {
-    std::lock_guard lock(g_mutex);
-    const auto found = g_directories.find(fd);
-    if (found == g_directories.end()) return std::nullopt;
-    return found->second.path;
+    const auto state = Find(fd);
+    if (!state) return std::nullopt;
+    return state->path;
 }
 
 void ForgetDirectoryDescriptor(int fd) {
+    if (g_directoryCount == 0) return;
     std::lock_guard lock(g_mutex);
     g_directories.erase(fd);
+    g_directoryCount = g_directories.size();
+}
+
+std::optional<std::int64_t> SeekDirectoryDescriptor(int fd, std::int64_t offset, int whence) {
+    constexpr int SeekSet = 0;
+    constexpr int SeekCurrent = 1;
+    if (g_directoryCount == 0) return std::nullopt;
+    const auto found = Find(fd);
+    if (!found) return std::nullopt;
+    auto& state = *found;
+    std::lock_guard lock(state.mutex);
+    if (whence != SeekSet && whence != SeekCurrent) NotImplemented_nid_no_patch("lseek from the end of a directory descriptor");
+    const auto cursor = static_cast<std::int64_t>(state.cursor);
+    if (whence == SeekCurrent && offset == 0) return cursor;
+    if (whence == SeekCurrent && offset > std::numeric_limits<std::int64_t>::max() - cursor) {
+        errno = EOVERFLOW;
+        return std::int64_t{-1};
+    }
+    const std::int64_t target = whence == SeekSet ? offset : cursor + offset;
+    if (target < 0) {
+        errno = EINVAL;
+        return std::int64_t{-1};
+    }
+    if (target == 0) {
+        state.entries.clear();
+        state.loaded = false;
+        state.cursor = 0;
+        return std::int64_t{0};
+    }
+    Load(state);
+    state.cursor = static_cast<std::size_t>(target);
+    return target;
 }
 
 int ReadDirectoryDescriptor(int fd, char* buf, int nbytes) {
-    std::lock_guard lock(g_mutex);
-    const auto found = g_directories.find(fd);
-    if (found == g_directories.end()) return SCE_KERNEL_ERROR_ENOTDIR;
-    auto& state = found->second;
-    if (!state.loaded) {
-        state.loaded = true;
-        state.entries.emplace_back(".", GuestDirectoryType);
-        state.entries.emplace_back("..", GuestDirectoryType);
-        std::error_code error;
-        for (const auto& entry : std::filesystem::directory_iterator(state.path, error)) {
-            std::error_code typeError;
-            state.entries.emplace_back(entry.path().filename().string(), entry.is_directory(typeError) ? GuestDirectoryType : GuestRegularType);
-        }
-    }
+    const auto found = Find(fd);
+    if (!found) return SCE_KERNEL_ERROR_ENOTDIR;
+    auto& state = *found;
+    std::lock_guard lock(state.mutex);
+    Load(state);
     std::size_t used = 0;
     while (state.cursor < state.entries.size()) {
         const auto& [name, type] = state.entries[state.cursor];

core/libs/prx/libkernel/File/src/Open.cpp

@@ -215,6 +215,12 @@ std::int64_t APS5_VABI sceKernelLseek(int d, std::int64_t offset, int whence) {
     if (whence < 0 || whence > 2) {
         throw std::invalid_argument(std::string(__func__) + ": invalid whence=" + std::to_string(whence));
     }
+#ifdef _WIN32
+    if (const auto directory = File::SeekDirectoryDescriptor(d, offset, whence)) {
+        if (*directory < 0) return SceIoErrorFromErrno(errno);
+        return *directory;
+    }
+#endif
     std::int64_t result = NativeLseek(d, offset, whence);
     if (result < 0) return SceIoErrorFromErrno(errno);
     return result;

core/libs/tests/GuestDirectoryRewind.cpp

@@ -0,0 +1,116 @@
+#include "prx/libc/include/general/VabiMacros.hpp"
+#include "prx/libkernel/File/include/FileFlags.hpp"
+
+#include <array>
+#include <chrono>
+#include <cstdint>
+#include <cstdio>
+#include <cstdlib>
+#include <filesystem>
+#include <fstream>
+#include <limits>
+#include <set>
+#include <string>
+
+extern "C" {
+int APS5_VABI sceKernelOpen(const char*, int, std::uint16_t);
+int APS5_VABI sceKernelClose(int);
+int APS5_VABI sceKernelGetdents(int, char*, int);
+std::int64_t APS5_VABI sceKernelLseek(int, std::int64_t, int);
+}
+
+static void Check(bool value, int line) {
+    if (!value) {
+        std::fprintf(stderr, "Directory rewind check failed at line %d\n", line);
+        std::abort();
+    }
+}
+#define Require(value) Check((value), __LINE__)
+
+
+static std::set<std::string> ReadAll(int directory) {
+    std::set<std::string> names;
+    std::array<char, 4096> buffer{};
+    for (;;) {
+        const int bytes = sceKernelGetdents(directory, buffer.data(), static_cast<int>(buffer.size()));
+        Require(bytes >= 0);
+        if (bytes == 0) return names;
+        for (int offset = 0; offset < bytes;) {
+            names.emplace(buffer.data() + offset + 8);
+            offset += static_cast<unsigned char>(buffer[offset + 4]) | (static_cast<unsigned char>(buffer[offset + 5]) << 8);
+        }
+    }
+}
+
+int main() {
+    const auto root = std::filesystem::temp_directory_path() /
+        ("anyps5-directory-rewind-" + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count()));
+    Require(std::filesystem::create_directory(root));
+    { std::ofstream file(root / "a"); Require(static_cast<bool>(file)); }
+    { std::ofstream file(root / "b"); Require(static_cast<bool>(file)); }
+
+    const int directory = sceKernelOpen(root.string().c_str(), SCE_KERNEL_O_RDONLY | SCE_KERNEL_O_DIRECTORY, 0);
+    Require(directory >= 0);
+    const std::set<std::string> expected{".", "..", "a", "b"};
+    Require(ReadAll(directory) == expected);
+    Require(ReadAll(directory).empty());
+
+    Require(sceKernelLseek(directory, 0, 0) == 0);
+    Require(ReadAll(directory) == expected);
+
+    { std::ofstream file(root / "c"); Require(static_cast<bool>(file)); }
+    Require(sceKernelLseek(directory, 0, 0) == 0);
+    Require(ReadAll(directory) == (std::set<std::string>{".", "..", "a", "b", "c"}));
+
+    std::array<char, 24> two{};
+    Require(sceKernelLseek(directory, 0, 0) == 0);
+    Require(sceKernelGetdents(directory, two.data(), static_cast<int>(two.size())) == 24);
+    const auto position = sceKernelLseek(directory, 0, 1);
+    Require(position == 2);
+    Require(sceKernelLseek(directory, 0, 1) == 2);
+    const auto rest = ReadAll(directory);
+    Require(sceKernelLseek(directory, position, 0) == position);
+    Require(ReadAll(directory) == rest);
+    Require(!rest.contains(".") && !rest.contains(".."));
+
+    Require(sceKernelLseek(directory, 100, 0) == 100);
+    Require(sceKernelGetdents(directory, two.data(), static_cast<int>(two.size())) == 0);
+    Require(sceKernelLseek(directory, 0, 1) == 100);
+    Require(sceKernelLseek(directory, -98, 1) == 2);
+    Require(ReadAll(directory) == rest);
+
+    Require(sceKernelLseek(directory, 3, 0) == 3);
+    Require(sceKernelLseek(directory, -1, 0) == static_cast<int>(0x80020016u));
+    Require(sceKernelLseek(directory, -4, 1) == static_cast<int>(0x80020016u));
+    Require(sceKernelLseek(directory, 0, 1) == 3);
+    Require(sceKernelLseek(directory, std::numeric_limits<std::int64_t>::max(), 0) == std::numeric_limits<std::int64_t>::max());
+    Require(sceKernelLseek(directory, 1, 1) == static_cast<int>(0x80020054u));
+    Require(sceKernelLseek(directory, 0, 1) == std::numeric_limits<std::int64_t>::max());
+
+    Require(sceKernelLseek(directory, 0, 0) == 0);
+    Require(sceKernelGetdents(directory, two.data(), static_cast<int>(two.size())) == 24);
+    Require(sceKernelLseek(directory, 0, 1) == 2);
+    const auto afterQuery = ReadAll(directory);
+    Require(afterQuery == rest);
+
+    std::array<char, 8> tiny{};
+    Require(sceKernelLseek(directory, 0, 0) == 0);
+    Require(sceKernelGetdents(directory, tiny.data(), static_cast<int>(tiny.size())) < 0);
+    { std::ofstream file(root / "d"); Require(static_cast<bool>(file)); }
+    Require(sceKernelLseek(directory, 0, 1) == 0);
+    Require(ReadAll(directory) == (std::set<std::string>{".", "..", "a", "b", "c"}));
+    Require(sceKernelLseek(directory, 0, 0) == 0);
+    Require(sceKernelGetdents(directory, tiny.data(), static_cast<int>(tiny.size())) < 0);
+    Require(sceKernelLseek(directory, 0, 0) == 0);
+    Require(ReadAll(directory) == (std::set<std::string>{".", "..", "a", "b", "c", "d"}));
+
+    Require(sceKernelClose(directory) == 0);
+
+    const int reused = sceKernelOpen((root / "a").string().c_str(), SCE_KERNEL_O_RDWR, 0);
+    Require(reused == directory);
+    Require(sceKernelLseek(reused, 5, 0) == 5);
+    Require(sceKernelLseek(reused, 0, 1) == 5);
+    Require(sceKernelLseek(reused, 0, 2) == 0);
+    Require(sceKernelClose(reused) == 0);
+    std::filesystem::remove_all(root);
+}

docs/dev/TechnicalDebt.md

@@ -366,6 +366,7 @@ Throughout the project, every function at every stage either **does exactly what
 - [libSceHttp2 asynchronous requests](../../core/libs/prx/libSceHttp2/Export.cpp) (libSceHttp2) never reach the network: `sceHttp2SendRequestAsync` accepts the request, queues a completion whose result is the library's network error and triggers the user event named by the kqueue option (equeue at offset 0, event id at 8, user data at 16, layout inferred from PPSA17221); `sceHttp2WaitAsync` returns that completion. The completion's `event_type` value is unknown and left 0. PPSA17221 completes its request object only after `sceHttp2SendRequestAsync` returns, and its handler for a synchronous failure calls `shared_from_this` before that, so a send must not fail synchronously. A kqueue option or user event that is missing, an extra option, and a wait with no completed operation throw
 - [system](../../core/libs/prx/libkernel/System/src/Process.cpp) (libkernel) runs no command. It follows FreeBSD 11's `lib/libc/stdlib/system.c` for a sandbox without `/bin/sh`: a null command returns 1 unconditionally, as that version does, and any command returns the wait status of a child that could not exec the shell and called `_exit(127)` (`0x7f00`). Whether the console's application sandbox has no shell, and whether `vfork` succeeds there (FreeBSD returns -1 when it fails), is unverified
 - [sceKernelWriteThrottlingStatus](../../core/libs/prx/libkernel/File/src/Stdio.cpp) (libkernel) reports no write throttling: the host SSD has no write budget, so the first qword of the 32-byte status is set to `UINT32_MAX` and the rest is zeroed. PPSA17221 reads only that qword: it multiplies it by 64 KiB and compares the product with byte thresholds up to 0x4AF00000 to pick one of five tiers. When the call fails the title assumes 0x4B000001 bytes, which selects the same top tier as `UINT32_MAX`. What the value measures on hardware (a remaining budget or a rate) and the meaning of the other three qwords are unknown
+- [Windows directory descriptors](../../core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp) (libkernel) take one snapshot of the directory, on the first `getdents`, `getdirentries` or non-zero `lseek` (`SEEK_SET` or `SEEK_CUR`), and keep it until it is discarded; `lseek` to offset 0 only discards the snapshot, and the next read lists the directory again, so every rewind costs O(entries) on the next read. `lseek` takes the offset as an entry index and `SEEK_CUR` with offset 0 only reports the index. `SEEK_END` throws, where the NUL descriptor used to return 0, because FreeBSD defines it relative to the directory size, which is not modelled. A negative result returns `EINVAL` and a `SEEK_CUR` offset that overflows returns `EOVERFLOW`, as FreeBSD `vn_seek` reports them. `getdirentries` still reports 0 through `basep` instead of the position of the block it read, so only 0 and values returned by `lseek` are valid offsets; FreeBSD offsets are filesystem cookies
 - [/dev/random and /dev/urandom](../../core/libs/prx/libkernel/File/src/RandomDevice.cpp) (libkernel) opened for reading return a descriptor backed by the host null device whose reads are filled from `std::random_device`; both paths behave the same, never block, and positioned reads ignore the offset (FreeBSD `random(4)`). Only the exact paths are recognised, a `dup`ed descriptor reads as an empty file, `fstat`, `lseek`, `mmap` and `select` see the null device, and opening them for writing throws
 - [Wide-character conversions](../../core/libs/prx/libc/src/Multibyte.cpp) (libc) - The wide `wcsto*` functions parse the ASCII prefix of the guest string, as the C locale accepts only ASCII white space and digits. [`wcsrtombs`](../../core/libs/prx/libc/src/Multibyte.cpp) uses the guest's single-byte C locale through `wcrtomb`; other encodings and shift states are not supported
 - [realpath](../../core/libs/prx/libc/src/RealPath.cpp) (libc) follows FreeBSD 11's [`realpath.c`](https://github.com/freebsd/freebsd-src/blob/stable/11/lib/libc/stdlib/realpath.c) except for symlinks. The console's guest filesystem has none, so a host symlink under the guest root is followed for existence and type but not expanded, and `..` drops the previous component lexically, as `Resolve` does for every guest path. Through a host link `linkdir -> sub/deep`, `linkdir/../x` gives `x` beside the link, not beside its target.