Merge pull request #4973 from ducloc080202/fix/cq3-positioned-io

fix(libkernel): serialize positioned file transfers on Windows

core/libs/prx/libkernel/CMakeLists.txt

@@ -292,6 +292,15 @@ if(BUILD_TESTING)
         set_tests_properties(guest_aio PROPERTIES ENVIRONMENT_MODIFICATION
             "PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
     endif()
+    add_test_executable(guest_positioned_race_tests ${LIBS_INCLUDE_DIR}/tests/GuestPositionedRace.cpp)
+    target_include_directories(guest_positioned_race_tests PRIVATE ${LIBS_INCLUDE_DIR})
+    target_link_libraries(guest_positioned_race_tests PRIVATE libkernel libc)
+    configure_windows_unwind(guest_positioned_race_tests)
+    add_test(NAME guest_positioned_race COMMAND guest_positioned_race_tests)
+    if(WIN32)
+        set_tests_properties(guest_positioned_race PROPERTIES TIMEOUT 60 ENVIRONMENT_MODIFICATION
+            "PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
+    endif()
     add_test_executable(guest_once_tests ${LIBS_INCLUDE_DIR}/tests/GuestOnce.cpp)
     target_include_directories(guest_once_tests PRIVATE ${LIBS_INCLUDE_DIR})
     target_link_libraries(guest_once_tests PRIVATE libkernel libc)

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

@@ -3,6 +3,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <mutex>
 
 #include "SceTypes.hpp"
 #include "prx/libc/include/General.hpp"
@@ -27,6 +28,8 @@ bool DescriptorIsOpen_nid_no_patch(int descriptor);
 
 namespace File {
 int HostStandardError();
+[[nodiscard]] std::unique_lock<std::mutex> LockPositionedTransfer(int descriptor);
+void SharePositionedTransfer(int from, int to);
 }
 
 #endif

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

@@ -565,6 +565,7 @@ void ShareDescriptor(int from, int to) {
     auto status = Materialize(from).status;
     g_entries[to] = Entry{false, std::move(status)};
 #ifdef _WIN32
+    File::SharePositionedTransfer(from, to);
     GuestFileLockShare_nid_no_patch(from, to);
 #endif
 }

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

@@ -44,6 +44,7 @@ static int NativeOpen(const std::filesystem::path& p, int nativeFlags, std::uint
     return descriptor;
 }
 static std::int64_t NativeLseek(int fd, std::int64_t offset, int whence) {
+    const auto positioned = File::LockPositionedTransfer(fd);
     const auto result = File::SeekSharedMemory_nid_no_patch(fd, offset, whence);
     if (result != -2) return result;
     const auto position = ::_lseeki64(fd, offset, whence);
@@ -55,6 +56,7 @@ static int NativeRead(int fd, void* buf, std::size_t n) {
     if (n > static_cast<std::size_t>(std::numeric_limits<unsigned int>::max())) {
         throw std::runtime_error("sceKernelRead: nbytes exceeds platform limit");
     }
+    const auto positioned = File::LockPositionedTransfer(fd);
     const int result = ::_read(fd, buf, static_cast<unsigned int>(n));
     if (result < 0 && _doserrno == ERROR_NO_DATA) errno = EAGAIN;
     return result;
@@ -63,6 +65,7 @@ static int NativeWrite(int fd, const void* buf, std::size_t n) {
     if (n > static_cast<std::size_t>(std::numeric_limits<unsigned int>::max())) {
         throw std::runtime_error("sceKernelWrite: nbytes exceeds platform limit");
     }
+    const auto positioned = File::LockPositionedTransfer(fd);
     const auto shared = File::WriteSharedMemory_nid_no_patch(fd, buf, n);
     if (shared != -2) return static_cast<int>(shared);
     const int result = ::_write(fd, buf, static_cast<unsigned int>(n));

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

@@ -26,9 +26,12 @@
 #include <cstdarg>
 #include <deque>
 #include <filesystem>
+#include <memory>
+#include <mutex>
 #include <optional>
 #include <stdexcept>
 #include <string>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -170,6 +173,27 @@ static int NativeSync(int descriptor) {
 static int NativeFlock(int descriptor, int operation) {
     return File::Flock(descriptor, operation);
 }
+struct PositionedLock {
+    std::mutex mutex;
+};
+std::mutex g_positionedTableLock;
+std::unordered_map<int, std::shared_ptr<PositionedLock>> g_positionedLocks;
+std::unique_lock<std::mutex> File::LockPositionedTransfer(int descriptor) {
+    std::shared_ptr<PositionedLock> entry;
+    {
+        std::lock_guard table(g_positionedTableLock);
+        auto& slot = g_positionedLocks[descriptor];
+        if (!slot) slot = std::make_shared<PositionedLock>();
+        entry = slot;
+    }
+    return std::unique_lock<std::mutex>(entry->mutex);
+}
+void File::SharePositionedTransfer(int from, int to) {
+    std::lock_guard table(g_positionedTableLock);
+    auto& source = g_positionedLocks[from];
+    if (!source) source = std::make_shared<PositionedLock>();
+    g_positionedLocks[to] = source;
+}
 std::int64_t NativePositioned_nid_no_patch(int descriptor, void* buf, std::size_t nbytes, std::int64_t offset, bool write) {
     if (nbytes > static_cast<std::size_t>(std::numeric_limits<DWORD>::max())) {
         throw std::runtime_error("NativePositioned: nbytes exceeds platform limit");
@@ -183,6 +207,7 @@ std::int64_t NativePositioned_nid_no_patch(int descriptor, void* buf, std::size_
         errno = EBADF;
         return -1;
     }
+    const auto positioned = File::LockPositionedTransfer(descriptor);
     LARGE_INTEGER position{};
     if (::GetFileType(handle) != FILE_TYPE_DISK || !::SetFilePointerEx(handle, LARGE_INTEGER{}, &position, FILE_CURRENT)) {
         errno = ESPIPE;
@@ -209,6 +234,7 @@ static std::int64_t NativeTransfer(int descriptor, void* buf, std::size_t nbytes
     if (nbytes > static_cast<std::size_t>(std::numeric_limits<int>::max())) {
         throw std::runtime_error("NativeTransfer: nbytes exceeds platform limit");
     }
+    const auto positioned = File::LockPositionedTransfer(descriptor);
     if (write) {
         const auto result = File::WriteSharedMemory_nid_no_patch(descriptor, buf, nbytes);
         if (result != -2) return result;

core/libs/tests/GuestPositionedRace.cpp

@@ -0,0 +1,77 @@
+#include "SceTypes.hpp"
+#include "prx/libc/include/general/VabiMacros.hpp"
+#include "prx/libkernel/File/include/FileFlags.hpp"
+#include "prx/libkernel/KernelErrors.hpp"
+#include <array>
+#include <atomic>
+#include <chrono>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <filesystem>
+#include <fstream>
+#include <string>
+#include <thread>
+extern "C" {
+int APS5_VABI sceKernelOpen(const char* path, int flags, std::uint16_t mode);
+int APS5_VABI sceKernelClose(int d);
+std::int64_t APS5_VABI sceKernelRead(int d, void* buf, std::size_t nbytes);
+std::int64_t APS5_VABI sceKernelPread(int d, void* buf, std::size_t nbytes, std::int64_t offset);
+std::int64_t APS5_VABI sceKernelLseek(int d, std::int64_t offset, int whence);
+}
+static void Check(bool value, int line) {
+    if (!value) {
+        std::fprintf(stderr, "Positioned race check failed at line %d\n", line);
+        std::abort();
+    }
+}
+#define Require(value) Check((value), __LINE__)
+int main() {
+    const auto root = std::filesystem::path("anyps5-positioned-race-" +
+        std::to_string(std::chrono::steady_clock::now().time_since_epoch().count()));
+    Require(std::filesystem::create_directory(root));
+    const auto file = root / "blocks.bin";
+    constexpr std::int64_t kBlocks = 8;
+    constexpr std::size_t kBlockSize = 4096;
+    {
+        std::ofstream stream(file, std::ios::binary);
+        Require(static_cast<bool>(stream));
+        for (std::int64_t block = 0; block < kBlocks; ++block) {
+            stream << std::string(kBlockSize, static_cast<char>('A' + block));
+        }
+    }
+    const int fd = sceKernelOpen(file.string().c_str(), SCE_KERNEL_O_RDONLY, 0);
+    Require(fd >= 0);
+    std::array<char, 64> expected{};
+    expected.fill('A');
+    std::atomic<bool> stop{false};
+    std::atomic<bool> misread{false};
+    std::array<char, 64> seeked{};
+    seeked.fill('H');
+    std::thread seeker([&] {
+        std::array<char, 64> sink{};
+        while (!stop.load()) {
+            if (sceKernelLseek(fd, (kBlocks - 1) * kBlockSize, 0) != (kBlocks - 1) * kBlockSize) {
+                misread = true;
+                return;
+            }
+            if (sceKernelRead(fd, sink.data(), sink.size()) != static_cast<std::int64_t>(sink.size()) ||
+                std::memcmp(sink.data(), seeked.data(), seeked.size()) != 0) {
+                misread = true;
+                return;
+            }
+        }
+    });
+    for (int i = 0; i < 5000 && !misread.load(); ++i) {
+        std::array<char, 64> bytes{};
+        const auto got = sceKernelPread(fd, bytes.data(), bytes.size(), 0);
+        if (got != static_cast<std::int64_t>(bytes.size()) || std::memcmp(bytes.data(), expected.data(), expected.size()) != 0) {
+            misread = true;
+        }
+    }
+    stop = true;
+    seeker.join();
+    Require(!misread);
+    Require(sceKernelClose(fd) == 0);
+    Require(std::filesystem::remove_all(root) > 0);
+}

docs/dev/debt/libkernel.md

@@ -111,7 +111,7 @@
 - [sceKernelAddReadEvent and sceKernelAddWriteEvent](../../../core/libs/prx/libkernel/Equeue/Equeue.cpp) (libkernel): a write event reports the socket's send buffer size (`SO_SNDBUF`) in `data`, not the free space left in it, as Windows has no query for that space. Only guest sockets are supported; other descriptors and a low watermark above 1 throw
 - [close](../../../core/libs/prx/libkernel/File/src/Stdio.cpp) and [setsockopt](../../../core/libs/prx/libkernel/Socket/src/Udp.cpp) (libkernel) reach a [libSceNet](../../../core/libs/prx/libSceNet/Export.cpp) socket through hooks libSceNet registers, as `select` polls them; the other libkernel socket calls do not. libSceNet numbers its sockets from 32 in its own table, so a CRT file descriptor and a socket can share a number; `close` then closes the socket. On the console both are one kernel descriptor space
 - [sceKernelPread, sceKernelPwrite, sceKernelPreadv, sceKernelPwritev](../../../core/libs/prx/libkernel/File/src/Stdio.cpp) (libkernel) on Windows transfer at the given offset with an overlapped `ReadFile`/`WriteFile` and restore the descriptor's file pointer afterwards, so a concurrent `read`/`write` on the same descriptor can see the moved pointer in between, unlike on the console. Handles that are not disk files return `ESPIPE`. `readv` on a pipe or console handle stops after the first iovec read, a short read the console is assumed to allow; each iovec is transferred separately, so a failure after the first transfer returns the bytes already done
-- [sceKernelAioSubmitReadCommands, sceKernelAioSubmitWriteCommands and their Multiple variants](../../../core/libs/prx/libkernel/Aio/src/Aio.cpp) (libkernel) on Windows transfer through the same positioned path as `sceKernelPread`/`sceKernelPwrite` and restore the descriptor's file pointer afterwards, so a concurrent `read`/`write` on the same descriptor can see the moved pointer in between, unlike on the console
+- [sceKernelAioSubmitReadCommands, sceKernelAioSubmitWriteCommands and their Multiple variants](../../../core/libs/prx/libkernel/Aio/src/Aio.cpp) (libkernel) on Windows transfer through the same positioned path as `sceKernelPread`/`sceKernelPwrite` and restore the descriptor's file pointer afterwards; positioned transfers, plain transfers and seeks of one descriptor are serialized per descriptor (shared across duplicated descriptors), so a concurrent transfer observes a stable file pointer
 - [sceKernelAioSetParam](../../../core/libs/prx/libkernel/Aio/src/Aio.cpp) (libkernel) writes the five values into the 20-byte scheduling struct the first argument points to, following the declarations in [Orbital](https://github.com/AlexAltea/orbital/blob/cc408367da6a6ebbf5ac1ac559fefeadc73b2831/tools/ida/db_types.json), [SharpProspero](https://github.com/SvenGDK/SharpProspero/blob/5dfdb4b0b166e83dc9ae275001afb695f12c7174/src/SharpProspero/Interop/Kernel/KernelAio.cs) and [ampr_emu](https://github.com/drakmor/ampr_emu/blob/cfa85df379f6eeeb165d7badf9b648e266fe77b7/ps5/include/sce/_fs.h). Whether the console checks the values is unknown, so a null struct, a scheduling window or delayed count limit outside 0..128, a split flag other than 0 or 1 and a split size or chunk size above 0x1000000 (SharpProspero's maximums) throw. The scheduling settings are not used: `sceKernelAioInitializeImpl` ignores the struct and requests run synchronously on submit
 - [sceKernelGetFsSandboxRandomWord](../../../core/libs/prx/libkernel/System/src/Sandbox.cpp) (libkernel) returns a fixed directory name, `gkbHtXc3Wq`. On the console the name is the process's randomized path (syscall 602, `randomized_path`), and the sandbox exposes system files under it, such as `common/lib` and `common/cert`. Nothing is mounted under the name here, so `/gkbHtXc3Wq/common/...` resolves under the guest root like any other path and does not exist
 - [getcontext and setcontext](../../../core/libs/prx/libkernel/System/src/UserContext.cpp) (libkernel) and [makecontext](../../../core/libs/prx/libSceLibcInternal/UserContext.cpp) (libSceLibcInternal) use FreeBSD 11's amd64 `ucontext_t` (`mcontext` at 0x10, `mc_len` 800, `uc_link` at 0x330, `uc_stack` at 0x338, 0x370 bytes), the layout homebrew built against FreeBSD's headers passes: PPSA99039's OpenSSL `ASYNC` (`crypto/async/arch/async_posix.c`) captures into a 0x370-byte stack buffer. `getcontext` writes the 0x330 bytes before `uc_link`, as FreeBSD's `UC_COPY_SIZE`. The console's own layout for these functions is not verified; the exception handler's context is a different one (see `sceKernelRaiseException`)