fix(libkernel): keep directory lseek off the shared lock and report offset overflow

lseek on a non-directory descriptor no longer takes the directory table lock when no directory is open, and a listing is read under the descriptor's own lock. SEEK_CUR with offset 0 only reports the position, and an overflowing SEEK_CUR offset reports EOVERFLOW.

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

@@ -11,11 +11,13 @@
 
 #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>
@@ -25,8 +27,10 @@ namespace {
 
 constexpr std::uint8_t GuestDirectoryType = 4;
 constexpr std::uint8_t GuestRegularType = 8;
+constexpr int GuestEoverflow = 84;
 
 struct DirectoryState {
+    std::mutex mutex;
     std::filesystem::path path;
     std::vector<std::pair<std::string, std::uint8_t>> entries;
     std::size_t cursor = 0;
@@ -34,7 +38,14 @@ 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;
@@ -56,33 +67,39 @@ 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;
-    std::lock_guard lock(g_mutex);
-    const auto found = g_directories.find(fd);
-    if (found == g_directories.end()) return std::nullopt;
-    auto& state = found->second;
+    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 = EINVAL;
+        errno = GuestEoverflow;
         return std::int64_t{-1};
     }
     const std::int64_t target = whence == SeekSet ? offset : cursor + offset;
@@ -102,10 +119,10 @@ std::optional<std::int64_t> SeekDirectoryDescriptor(int fd, std::int64_t offset,
 }
 
 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;
+    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()) {

core/libs/tests/GuestDirectoryRewind.cpp

@@ -8,7 +8,9 @@
 #include <cstdlib>
 #include <filesystem>
 #include <fstream>
+#include <limits>
 #include <set>
+#include <stdexcept>
 #include <string>
 
 extern "C" {
@@ -26,6 +28,16 @@ static void Check(bool value, int line) {
 }
 #define Require(value) Check((value), __LINE__)
 
+template <typename TFunction>
+static bool ThrowsRuntimeError(TFunction function) {
+    try {
+        function();
+    } catch (const std::runtime_error&) {
+        return true;
+    }
+    return false;
+}
+
 static std::set<std::string> ReadAll(int directory) {
     std::set<std::string> names;
     std::array<char, 4096> buffer{};
@@ -64,12 +76,51 @@ int main() {
     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 > 0);
+    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(ThrowsRuntimeError([&] { sceKernelLseek(directory, -1, 0); }));
+    Require(ThrowsRuntimeError([&] { sceKernelLseek(directory, -4, 1); }));
+    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(ThrowsRuntimeError([&] { sceKernelLseek(directory, 1, 1); }));
+    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,7 +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) snapshot the directory on the first `getdents` or `getdirentries`; `lseek` on them takes `SEEK_SET` and `SEEK_CUR` offsets as entry indexes and `SEEK_END` throws. Offset 0 rewinds and takes a new snapshot, as FreeBSD `getdirentries(2)` allows. `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
+- [Windows directory descriptors](../../core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp) (libkernel) snapshot the directory on the first `getdents` or `getdirentries` and on `lseek` with `SEEK_SET` or `SEEK_CUR` to a non-zero index; `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 throws like any other `lseek` failure (`EINVAL`) and a `SEEK_CUR` offset that overflows throws with `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.