From 0245ba7d883db3fc21e71e1111512bf2c00bf5e0 Mon Sep 17 00:00:00 2001 From: Yunus Shaikh <75296019+yunusshaikh7@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:25:00 -0700 Subject: [PATCH] fix(libc): open fopen streams with delete sharing on Windows fopen opened its stream with _wfopen, which shares read and write access but not delete access. While the guest held a stream open, rename of its file failed with EIO and remove with EBUSY. POSIX rename and remove work on open files, as on Linux and FreeBSD, and since #3677 the descriptors sceKernelOpen returns share delete access. fopen now creates the handle with CreateFileW sharing read, write and delete, with the access and creation disposition of the mode, and wraps it with _open_osfhandle and _wfdopen, keeping append mode. A failed CreateFileW sets errno through the error mapping #3677 added to libkernel's NativeOpen, which moves to libc as WindowsOpenError_nid_no_patch so that both use it. freopen still reopens through _wfreopen; TechnicalDebt records it. --- core/libs/prx/libc/include/General.hpp | 3 ++ core/libs/prx/libc/src/FileAndHeap.cpp | 34 ++++++++++++++++++++-- core/libs/prx/libc/src/General.cpp | 35 +++++++++++++++++++++++ core/libs/prx/libkernel/File/src/Open.cpp | 34 +--------------------- core/libs/tests/FileStreams.cpp | 23 +++++++++++++++ docs/dev/TechnicalDebt.md | 1 + 6 files changed, 94 insertions(+), 36 deletions(-) diff --git a/core/libs/prx/libc/include/General.hpp b/core/libs/prx/libc/include/General.hpp index 2a2a9ed92..7de9ed967 100644 --- a/core/libs/prx/libc/include/General.hpp +++ b/core/libs/prx/libc/include/General.hpp @@ -24,6 +24,9 @@ extern "C" void RecordWrittenPath_nid_no_patch(const std::filesystem::path& path extern "C" std::vector WrittenPaths_nid_no_patch(); extern "C" void SyncWrittenPaths_nid_no_patch(); extern "C" void BlockPathAlias_nid_no_patch(const char* guestPrefix); +#ifdef _WIN32 +extern "C" int WindowsOpenError_nid_no_patch(unsigned long error); +#endif #define APS5_INVALID_ARG_EX throw std::invalid_argument(std::string(__func__) + ": invalid argument") diff --git a/core/libs/prx/libc/src/FileAndHeap.cpp b/core/libs/prx/libc/src/FileAndHeap.cpp index 87c352e2b..f4548f666 100644 --- a/core/libs/prx/libc/src/FileAndHeap.cpp +++ b/core/libs/prx/libc/src/FileAndHeap.cpp @@ -11,6 +11,14 @@ #include #include #include +#ifdef _WIN32 +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include +#include +#include +#endif #include "prx/libc/include/FileStream.hpp" #include "prx/libc/include/ApplicationHeap.hpp" @@ -45,9 +53,29 @@ static std::optional ParseFileMode(const char* mode) { static std::FILE* OpenNative(const std::filesystem::path& path, const std::string& nativeMode) { #ifdef _WIN32 - std::wstring wideMode; - for (const char character : nativeMode) wideMode.push_back(static_cast(character)); - return ::_wfopen(path.c_str(), wideMode.c_str()); + const char access = nativeMode[0]; + const bool update = nativeMode.find('+') != std::string::npos; + const DWORD desired = update ? GENERIC_READ | GENERIC_WRITE : access == 'r' ? GENERIC_READ : GENERIC_WRITE; + const DWORD disposition = access == 'r' ? OPEN_EXISTING + : nativeMode.find('x') != std::string::npos ? CREATE_NEW : access == 'w' ? CREATE_ALWAYS : OPEN_ALWAYS; + SECURITY_ATTRIBUTES security{sizeof(SECURITY_ATTRIBUTES), nullptr, TRUE}; + const auto handle = CreateFileW(path.c_str(), desired, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, &security, + disposition, FILE_ATTRIBUTE_NORMAL, nullptr); + if (handle == INVALID_HANDLE_VALUE) { + errno = WindowsOpenError_nid_no_patch(GetLastError()); + return nullptr; + } + const int descriptor = ::_open_osfhandle(reinterpret_cast(handle), access == 'a' ? _O_APPEND : 0); + if (descriptor < 0) { + CloseHandle(handle); + return nullptr; + } + std::wstring streamMode; + for (const char character : nativeMode) + if (character != 'x') streamMode.push_back(static_cast(character)); + std::FILE* stream = ::_wfdopen(descriptor, streamMode.c_str()); + if (!stream) ::_close(descriptor); + return stream; #else return std::fopen(path.c_str(), nativeMode.c_str()); #endif diff --git a/core/libs/prx/libc/src/General.cpp b/core/libs/prx/libc/src/General.cpp index fefc33af4..a2a57370b 100644 --- a/core/libs/prx/libc/src/General.cpp +++ b/core/libs/prx/libc/src/General.cpp @@ -401,6 +401,41 @@ extern "C" char* APS5_VABI getcwd_nid_postfix(char* buffer, std::size_t size) { catch (const std::filesystem::filesystem_error& error) { errno = DirectoryFailure(error.code()); return nullptr; } } +#ifdef _WIN32 +extern "C" int WindowsOpenError_nid_no_patch(unsigned long error) { + switch (error) { + case ERROR_FILE_NOT_FOUND: + case ERROR_PATH_NOT_FOUND: + case ERROR_INVALID_DRIVE: + case ERROR_BAD_NETPATH: + case ERROR_BAD_NET_NAME: + case ERROR_BAD_PATHNAME: + case ERROR_FILENAME_EXCED_RANGE: + return ENOENT; + case ERROR_ACCESS_DENIED: + case ERROR_SHARING_VIOLATION: + case ERROR_LOCK_VIOLATION: + case ERROR_WRITE_PROTECT: + case ERROR_NETWORK_ACCESS_DENIED: + case ERROR_CANNOT_MAKE: + case ERROR_DRIVE_LOCKED: + return EACCES; + case ERROR_FILE_EXISTS: + case ERROR_ALREADY_EXISTS: + return EEXIST; + case ERROR_TOO_MANY_OPEN_FILES: + return EMFILE; + case ERROR_NOT_ENOUGH_MEMORY: + case ERROR_OUTOFMEMORY: + return ENOMEM; + case ERROR_DISK_FULL: + return ENOSPC; + default: + return EINVAL; + } +} +#endif + extern "C" void NotImplemented_nid_no_patch(const char* funcName) { throw std::runtime_error(std::string(funcName) + " not implemented"); } diff --git a/core/libs/prx/libkernel/File/src/Open.cpp b/core/libs/prx/libkernel/File/src/Open.cpp index bfcf004c5..58e3c3c70 100644 --- a/core/libs/prx/libkernel/File/src/Open.cpp +++ b/core/libs/prx/libkernel/File/src/Open.cpp @@ -23,38 +23,6 @@ #include #include #include -static int OpenError(DWORD error) { - switch (error) { - case ERROR_FILE_NOT_FOUND: - case ERROR_PATH_NOT_FOUND: - case ERROR_INVALID_DRIVE: - case ERROR_BAD_NETPATH: - case ERROR_BAD_NET_NAME: - case ERROR_BAD_PATHNAME: - case ERROR_FILENAME_EXCED_RANGE: - return ENOENT; - case ERROR_ACCESS_DENIED: - case ERROR_SHARING_VIOLATION: - case ERROR_LOCK_VIOLATION: - case ERROR_WRITE_PROTECT: - case ERROR_NETWORK_ACCESS_DENIED: - case ERROR_CANNOT_MAKE: - case ERROR_DRIVE_LOCKED: - return EACCES; - case ERROR_FILE_EXISTS: - case ERROR_ALREADY_EXISTS: - return EEXIST; - case ERROR_TOO_MANY_OPEN_FILES: - return EMFILE; - case ERROR_NOT_ENOUGH_MEMORY: - case ERROR_OUTOFMEMORY: - return ENOMEM; - case ERROR_DISK_FULL: - return ENOSPC; - default: - return EINVAL; - } -} static int NativeOpen(const std::filesystem::path& p, int nativeFlags, std::uint16_t mode) { const int access = nativeFlags & (_O_RDONLY | _O_WRONLY | _O_RDWR); const DWORD desired = access == _O_RDONLY ? GENERIC_READ : access == _O_WRONLY ? GENERIC_WRITE : GENERIC_READ | GENERIC_WRITE; @@ -65,7 +33,7 @@ static int NativeOpen(const std::filesystem::path& p, int nativeFlags, std::uint SECURITY_ATTRIBUTES security{sizeof(SECURITY_ATTRIBUTES), nullptr, TRUE}; const auto handle = CreateFileW(p.c_str(), desired, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, &security, disposition, attributes, nullptr); if (handle == INVALID_HANDLE_VALUE) { - errno = OpenError(GetLastError()); + errno = WindowsOpenError_nid_no_patch(GetLastError()); return -1; } const int descriptor = ::_open_osfhandle(reinterpret_cast(handle), nativeFlags & (_O_APPEND | _O_RDONLY | _O_BINARY)); diff --git a/core/libs/tests/FileStreams.cpp b/core/libs/tests/FileStreams.cpp index aa4b41128..c06b0a06a 100644 --- a/core/libs/tests/FileStreams.cpp +++ b/core/libs/tests/FileStreams.cpp @@ -19,6 +19,8 @@ int APS5_VABI fseek_nid_postfix(FileStream* stream, std::int64_t offset, int ori std::int64_t APS5_VABI ftell_nid_postfix(FileStream* stream); int APS5_VABI fputs_nid_postfix(const char* str, FileStream* stream); int APS5_VABI fflush_nid_postfix(FileStream* stream); +int APS5_VABI remove_nid_postfix(const char* path); +int APS5_VABI rename_nid_postfix(const char* from, const char* to); } static void Require(bool condition) { @@ -91,5 +93,26 @@ int main(int argc, char** argv) { Require(fclose_nid_postfix(stream) == 0); Require(std::filesystem::file_size(hostName) == 0); std::filesystem::remove(hostName); + const std::string heldName = std::string(argv[1]) + ".held"; + const std::string movedName = std::string(argv[1]) + ".moved"; + stream = fopen_nid_postfix(heldName.c_str(), "w+b"); + Require(stream != nullptr && fwrite_nid_postfix(payload, 1, sizeof(payload), stream) == sizeof(payload)); + Require(fflush_nid_postfix(stream) == 0); + Require(rename_nid_postfix(heldName.c_str(), movedName.c_str()) == 0 && !std::filesystem::exists(heldName)); + Require(remove_nid_postfix(movedName.c_str()) == 0 && !std::filesystem::exists(movedName)); + Require(fseek_nid_postfix(stream, 0, SEEK_SET) == 0); + std::array heldBuffer{}; + Require(fread_nid_postfix(heldBuffer.data(), 1, heldBuffer.size(), stream) == heldBuffer.size()); + Require(std::strcmp(heldBuffer.data(), payload) == 0 && fclose_nid_postfix(stream) == 0); + stream = fopen_nid_postfix(heldName.c_str(), "ab"); + Require(stream != nullptr && fputs_nid_postfix("ab", stream) >= 0 && fclose_nid_postfix(stream) == 0); + stream = fopen_nid_postfix(heldName.c_str(), "ab"); + Require(stream != nullptr && fputs_nid_postfix("cd", stream) >= 0 && fclose_nid_postfix(stream) == 0); + errno = 0; + Require(fopen_nid_postfix(heldName.c_str(), "wbx") == nullptr && errno == EEXIST); + stream = fopen_nid_postfix(heldName.c_str(), "rb"); + std::array appended{}; + Require(stream != nullptr && fread_nid_postfix(appended.data(), 1, 4, stream) == 4 && std::strcmp(appended.data(), "abcd") == 0); + Require(fclose_nid_postfix(stream) == 0 && remove_nid_postfix(heldName.c_str()) == 0); std::cout << "PASS: stream objects, file operations, EOF and error handling\n"; } diff --git a/docs/dev/TechnicalDebt.md b/docs/dev/TechnicalDebt.md index 0fa78fec4..158208540 100644 --- a/docs/dev/TechnicalDebt.md +++ b/docs/dev/TechnicalDebt.md @@ -431,6 +431,7 @@ Throughout the project, every function at every stage either **does exactly what - [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 - [sceKernelSync](../../core/libs/prx/libc/src/General.cpp) (libkernel) flushes only the host files and directories recorded through `RecordWrittenPath_nid_no_patch` (the libc, libkernel, save data and app content file functions) and their changed parent directories. A host write in a library that does not record its path is not flushed by `sceKernelSync` - [sceKernelSync](../../core/libs/prx/libc/src/General.cpp) (libkernel) on Windows skips a recorded file it cannot open for writing (read-only, or held open by a handle that denies write sharing), since `FlushFileBuffers` needs a writable handle. On Linux, it skips a recorded file it cannot open for reading. Such a file's data may not reach the disk before a host crash; the guest reads the same data either way +- [`fopen` streams](../../core/libs/prx/libc/src/FileAndHeap.cpp) (libc) on Windows share delete access, like the descriptors [`sceKernelOpen`](../../core/libs/prx/libkernel/File/src/Open.cpp) opens, so a file the guest holds open can be removed or renamed. A stream reopened with `freopen` still goes through `_wfreopen`, which does not share delete access - [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`) - [makecontext](../../core/libs/prx/libSceLibcInternal/UserContext.cpp) (libSceLibcInternal) follows FreeBSD 11's `lib/libc/amd64/gen/makecontext.c`: at most six arguments, a stack of at least `MINSIGSTKSZ` (2048) bytes, and `exit(0)` when the function returns with no `uc_link`. The console's version is not verified - [getcontext and setcontext](../../core/libs/prx/libkernel/System/src/UserContext.cpp) (libkernel) keep the 512-byte FXSAVE image in `mc_fpstate` (x87, MXCSR, XMM0-15), so the upper halves of YMM0-15 are neither saved nor restored, and `getcontext` leaves `mc_xfpustate` and `mc_xfpustate_len` 0. `getcontext` sets `mc_fpformat` and `mc_ownedfp` to FreeBSD's `_MC_FPFMT_XMM` and `_MC_FPOWNED_FPU`, `mc_cs` and `mc_ss` to the host selectors, and `mc_flags`, `mc_fsbase` and `mc_gsbase` to 0. `setcontext` loads `mc_fpstate` whatever `mc_fpformat` and `mc_ownedfp` say, and restores neither the segment selectors, `mc_fsbase` and `mc_gsbase` nor the extended state, which FreeBSD restores when `mc_flags` asks for them