Skip to content

fix(files): isolate guest descriptors from host resources - #2170

Closed
MichalSy wants to merge 1 commit into
boykopovar:mainfrom
MichalSy:codex/guest-file-descriptor-isolation
Closed

MichalSy wants to merge 1 commit into
boykopovar:mainfrom
MichalSy:codex/guest-file-descriptor-isolation

Conversation

@MichalSy

@MichalSy MichalSy commented Oct 9, 2026 •

Copy link
Copy Markdown

What

Guest file descriptors currently contain host descriptor numbers. A guest can therefore read, seek or close a private runtime file by supplying its host descriptor. This change gives guest files their own descriptor table, shared by libc, libkernel, POSIX file operations and AIO.

Descriptors 0-2 retain their guest meanings through owned duplicates of the host standard streams. Ordinary guest descriptors occupy 3-32767, preserving the existing signed 16-bit FILE prefix. Operations hold a native ownership lease, so a concurrent guest close retires the logical descriptor while an in-flight operation can finish. FILE wrappers retain the original descriptor identity; after close and slot reuse, a stale stream reports EBADF without accessing or closing the replacement. Pipe adoption publishes both guest descriptors atomically and releases native resources on failure.

Existing platform backends remain in place, including the Windows/Linux ftruncate error handling. Existing tests now obtain file descriptors through the guest API. The same new isolation fixture runs against both the ordinary libraries and the actual NID-patched libc.prx/libkernel.prx.

The overlap bot reported eight other PRs. Their current heads were read and fetched for three-way merge simulations against b14cd621. Each of the eight produces a textual conflict; none implements the guest descriptor namespace, ownership leases or stale-FILE identity mechanism introduced here.

PR Other change Integration when combining
1804 Linux routing for guest wide printf arguments Keep guest-stdout routing and its wide-conversion logic together in vfprintf.
1807 Sandbox random-word export Preserve both independent CMake test-registration blocks.
2019 gethostname Preserve both independent CMake test-registration blocks.
2030 fsync result/errno translation Acquire the guest descriptor lease, then preserve that PR's native flush/error mapping.
2033 FreeBSD fopen/freopen mode parsing Keep its parser; derive the registry access mode from the parsed native mode and preserve FILE identity/ownership.
2109 Wide-format UTF-8 validation Keep its EILSEQ catch while writing through the owned guest stdout stream.
2129 sceKernelFdatasync Its added export currently uses a raw host descriptor. A combined version must resolve the logical guest descriptor and hold its lease; tests must use guest-owned descriptors.
2144 Scalar kernel I/O errors Preserve its native error/buffer handling and our logical lookup/lease. Its pipe tests must obtain guest descriptors. Both changes cover invalid-descriptor EBADF behavior, using different mechanisms.

These are not functional prerequisites for this standalone branch, so Depends on remains none. Merge order matters: after any of these lands, rebase and validate the combined behavior rather than selecting one side of a conflict. The simulations above only examine merges; no combined-branch build or runtime pass is claimed.

Previously reviewed scalar POSIX I/O errors and Windows positioned I/O also touch file APIs, without implementing the ownership registry. File/pipe polling will need logical-descriptor lookup if both changes land.

Tested

  • Windows, WinLibs GCC 15.2.0 r7, Release: full all and libs build; 13 selected tests passed covering ordinary and patched isolation, standard streams, file positioning, filesystem operations, vector I/O, directory buffers/close, AIO, guest memory and narrow/wide formatting. The new socket-pair test also passed. After cleanup and the final upstream rebase, the full build and all 13 selected tests passed again. One intermediate parallel run timed out after the isolation fixture printed its success message; the immediate isolated run and the repeated parallel selection passed without code or timeout changes.
  • Linux, Ubuntu 26.04.1 under WSL, GCC 15.2.0, Release: built libc, libkernel, their NID-patched libraries and the relevant test targets. All 26 selected tests passed, including private host descriptor rejection, stale FILE identities, ownership, close races, blocked reads, pipe rollback, stdio errno, directory entries and the shared isolation fixture against actual patched libraries. No full Linux graphics build or game/homebrew run was performed.
  • python tools/check_conventions.py --base upstream/main and the whitespace check passed. No full CTest suite was run.

The runtime/build results above were obtained at 2b12fd51, based on upstream 51f69aa2. The subsequent conflict-free rebase onto d70b8998 brings upstream AGC profiling/video-output build changes; libc, libkernel, the NID patcher and the file test sources are unchanged. The conventions check including the PR title and description was rerun on b14cd621.

Checklist

  • Based on current main; no duplicate implementation of this descriptor-isolation change was found
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw; no new silent stubs
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source: none added
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#2144 core/libs/prx/libkernel/File/src/Open.cpp L37-43, L135-138, L140-145, L148-156, L159-167
core/libs/tests/GuestFileVector.cpp L13-22, L31, L129
#2230 core/libs/prx/libkernel/CMakeLists.txt L503
#2235 core/libs/prx/libc/CMakeLists.txt L48
#2247 core/libs/CMakeLists.txt L133
#2268 core/libs/CMakeLists.txt L133
#2271 core/libs/prx/libc/CMakeLists.txt L48
#2280 core/libs/prx/libc/CMakeLists.txt L48
#2361 core/libs/prx/libc/include/FileStream.hpp L3-5, L34, L69-107
core/libs/prx/libc/src/FileAndHeap.cpp L65-66, L79-109

QRMOE added a commit to QRMOE/AnyPS5 that referenced this pull request Oct 9, 2026
AI-assisted: yes. Moves hostname test registration to avoid boykopovar#2170 overlap. The full current-main build and focused Windows tests pass; implementation and test source are unchanged.
@popeee99

popeee99 commented Oct 9, 2026

Copy link
Copy Markdown

Independent allocation-failure validation on b14cd621f4055f501378762f9fb86430c5146420 (Linux under WSL, GCC 15.2).

The existing ownership tests cover invalid arguments, stale identities and rollback, but I did not find allocation-failure injection. I compiled the unchanged GuestFileDescriptors.cpp with a standalone harness that injects std::bad_alloc at all three standard-stream initialization allocations, single adoption, both pair-adoption allocations, and replacement.

All cases passed: guest ENOMEM, no partial pair publication, owned descriptors cleaned exactly once, original identity/resource retained after failed replacement, and successful retries. Cleanup re-enters GuestFileAcquire_nid_no_patch(1), so the pair rollback also checks that cleanup runs outside the table mutex.

Each mode passed five fresh-process runs in both Release and ASan/UBSan builds (40 runs total), with no sanitizer diagnostics. The existing GuestFileDescriptorOwnership.cpp test also passed. As a harness control, removing the pair rollback's unlock in an isolated source copy caused the expected five-second timeout; the unchanged implementation passed the bounded checks.

This is focused registry validation, not a full library, Windows, NID-patched-library or game test. No defect found in these tested paths; this does not establish correctness of the entire implementation. AI-assisted investigation and harness.

Standalone harness and reproduction commands

Check out the exact PR head, then save the following as allocation_failure.cpp outside the checkout:

#include "prx/libc/include/GuestFileDescriptors.hpp"
#include <array>
#include <cerrno>
#include <cstdio>
#include <cstdlib>
#include <new>
#include <fcntl.h>
#include <unistd.h>
using namespace GuestFiles;
static int failAfter = -1;
static int injected = 0;
void* operator new(std::size_t size) {
    if (failAfter == 0) { failAfter = -1; ++injected; throw std::bad_alloc(); }
    if (failAfter > 0) --failAfter;
    if (void* p = std::malloc(size ? size : 1)) return p;
    throw std::bad_alloc();
}
void operator delete(void* p) noexcept { std::free(p); }
void operator delete(void* p, std::size_t) noexcept { std::free(p); }
static void check(bool ok, const char* message) {
    if (!ok) { std::fprintf(stderr,"FAIL: %s (errno=%d)\n",message,errno); std::exit(1); }
}
using Inventory = std::array<bool,256>;
static Inventory inventory() {
    Inventory result{};
    for (int fd=0;fd<256;++fd) result[fd] = ::fcntl(fd,F_GETFD) != -1;
    return result;
}
static int cleaned = 0;
static void cleanup(int fd) noexcept {
    check(::fcntl(fd,F_GETFD) != -1,"cleanup receives live native fd");
    check(static_cast<bool>(GuestFileAcquire_nid_no_patch(1)),"cleanup can re-enter registry");
    ++cleaned;
    errno = 47;
}
static int native() {
    int fd = ::open("/dev/null",O_RDWR|O_CLOEXEC);
    check(fd >= 0 && fd < 256,"fixture native fd");
    return fd;
}
static void initialize(int allocation) {
    const auto before=inventory();
    failAfter=allocation;
    const int result=GuestFileInitializeStandards_nid_no_patch();
    const int error=errno;
    failAfter=-1;
    check(injected==1 && result==-1 && error==12,"standard initialization returns ENOMEM");
    check(inventory()==before,"failed standards setup releases duplicates and preserves host stdio");
    check(GuestFileInitializeStandards_nid_no_patch()==0,"standards initialization retries");
    for(int fd=0;fd<3;++fd) check(static_cast<bool>(GuestFileAcquire_nid_no_patch(fd)),"standard identity after retry");
}
static void rollback() {
    check(GuestFileInitializeStandards_nid_no_patch()==0,"initialize standards");
    const auto before=inventory();
    int original=native();
    failAfter=0;
    auto entry=GuestFileAdoptOwned_nid_no_patch(original,2,cleanup);
    int error=errno;
    failAfter=-1;
    check(!entry && error==12 && cleaned==1,"single allocation failure reports ENOMEM and cleans once");
    check(inventory()==before,"single rollback native inventory");
    for(int allocation=0;allocation<2;++allocation) {
        const int first=native(),second=native();
        const int previous=cleaned;
        failAfter=allocation;
        auto pair=GuestFileAdoptPairOwned_nid_no_patch(first,second,0,1,cleanup);
        error=errno;
        failAfter=-1;
        check(!pair[0] && !pair[1] && error==12,"pair allocation failure publishes neither lease");
        check(cleaned==previous+2 && inventory()==before,"pair rollback closes each owned fd exactly once");
        check(!GuestFileAcquire_nid_no_patch(3) && !GuestFileAcquire_nid_no_patch(4),"pair publishes neither logical slot");
    }
    auto initial=GuestFileAdoptOwned_nid_no_patch(native(),2,cleanup);
    check(initial && GuestFileLogicalDescriptor_nid_no_patch(initial)==3,"lowest ordinary slot remains reusable");
    const Identity identity=initial;
    const int firstNative=GuestFileNativeDescriptor_nid_no_patch(initial);
    const auto retained=inventory();
    const int replacementNative=native(),previous=cleaned;
    failAfter=0;
    auto replacement=GuestFileReplaceOwnedMatching_nid_no_patch(identity,replacementNative,1,cleanup);
    error=errno;
    failAfter=-1;
    check(!replacement && error==12,"replacement failure reports ENOMEM");
    check(cleaned==previous+1 && inventory()==retained,"replacement failure closes only incoming fd");
    check(GuestFileMatches_nid_no_patch(identity),"original identity remains current");
    char byte=0;
    check(::read(firstNative,&byte,1)==0,"original native resource remains usable");
    replacement=GuestFileReplaceOwnedMatching_nid_no_patch(identity,native(),1,cleanup);
    check(replacement && GuestFileLogicalDescriptor_nid_no_patch(replacement)==3,"replacement retry succeeds at original slot");
    check(!GuestFileMatches_nid_no_patch(identity),"successful replacement retires old identity");
    initial.reset();
    const Identity finalIdentity=replacement;
    replacement.reset();
    check(GuestFileCloseMatching_nid_no_patch(finalIdentity)==0,"close replacement");
    check(inventory()==before,"final native inventory restored");
    check(injected==4,"all four rollback failure points were injected");
}
int main(int argc,char** argv) {
    check(argc==2,"mode required");
    if(argv[1][0]=='s') initialize(std::atoi(argv[1]+1)); else rollback();
    std::printf("PASS %s injected=%d cleanups=%d\n",argv[1],injected,cleaned);
}

From the PR checkout, with PROBE set to the harness's absolute path:

git rev-parse HEAD
# b14cd621f4055f501378762f9fb86430c5146420
PROBE=/absolute/path/to/allocation_failure.cpp
SRC=core/libs/prx/libc/src/GuestFileDescriptors.cpp
g++ -std=c++20 -O2 -Wall -Wextra -Werror -pthread -Icore/libs "$PROBE" "$SRC" -o /tmp/2170-release
g++ -std=c++20 -O1 -g -fsanitize=address,undefined -fno-omit-frame-pointer -Wall -Wextra -Werror -pthread -Icore/libs "$PROBE" "$SRC" -o /tmp/2170-sanitized
for binary in /tmp/2170-release /tmp/2170-sanitized; do
  for mode in s0 s1 s2 rollback; do
    for run in 1 2 3 4 5; do
      timeout 5s "$binary" "$mode" || exit 1
    done
  done
done
g++ -std=c++20 -O2 -Wall -Wextra -Werror -pthread -Icore/libs core/libs/tests/GuestFileDescriptorOwnership.cpp "$SRC" -o /tmp/2170-ownership
timeout 5s /tmp/2170-ownership

Every sN run printed PASS sN injected=1 cleanups=0; every rollback run printed PASS rollback injected=4 cleanups=8. The ownership test printed its PASS message. All returned zero. Run with stdin/stdout/stderr open; the fixture keeps its native descriptors below 256 and compares their inventory before and after rollback.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: this replaces how every file descriptor works (+2275 lines, real conflicts with about 15 open PRs) without a title that fails because of host descriptor numbers. If a title breaks on that, please open an issue with the title ID and the log first, so the fix can be sized to it.

adriadam10 pushed a commit that referenced this pull request Oct 10, 2026
AI-assisted: yes. Moves hostname test registration to avoid #2170 overlap. The full current-main build and focused Windows tests pass; implementation and test source are unchanged.
@MichalSy
MichalSy deleted the codex/guest-file-descriptor-isolation branch October 12, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants