Skip to content

fix(libkernel): reprotect bundled guest module images on Windows - #1803

Merged
adriadam10 merged 3 commits into
boykopovar:mainfrom
tv7:fix/mprotect-guest-module-images
Oct 10, 2026
Merged

adriadam10 merged 3 commits into
boykopovar:mainfrom
tv7:fix/mprotect-guest-module-images

Conversation

@tv7

@tv7 tv7 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

What

Fixes item 4 of #1794. On Windows, sceKernelMprotect accepted only the executable's image and threw Memory protection of a foreign image is not supported for any other image. Bundled title modules are relinked into DLLs named <module>.guest.prx, so a title that reprotects pages of its own bundled module could not start. Dying Light 2's bundled libNRD does this at start-up (prot=0x12 over 0x11278 bytes, then 0x11 over 0x178540 bytes).

DoMprotect now also accepts images whose file name ends in .guest.prx. It registers their committed pages in GuestAllocations the same way as the executable's: RegisterMainImage's Windows walk becomes a per-image registration, and the executable is one caller. Any other image (system DLLs, AnyPS5's own .prx libraries) is still refused.

Tested

Windows 11, MinGW GCC 15.2.0, RTX 3070:

Linux is unchanged. I did not build or run it.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

🤖 Generated with Claude Code

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a module-lifetime issue in the new registration. registerImage adds committed .guest.prx pages to the global allocation registry as non-releasable and records the base in registeredImages, but guest modules can later be unloaded: sceKernelStopUnloadModule calls dlclose_nid_postfix, whose Windows Module destructor calls FreeLibrary. Nothing removes those registry ranges or the base from registeredImages on unload. This leaves dead image ranges registered, can make later mappings at the released address fail, and can expose stale addresses to GPU builds. Please add unload cleanup coordinated with outstanding allocation leases and cover mprotect → unload → reload/address reuse in a Windows test before approval.

@tv7

tv7 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, you're right: nothing undid the registration when a module was unloaded. Fixed in the new commit:

  • GuestAllocations records each registered image's extent and gets GuestAllocationsUnregisterImage_nid_postfix. It waits for outstanding leases on that extent through RequireUnpinned (same path, pin waiter and timeout as unmap/protect), reports the extent to the invalidator, then drops the image's non-releasable ranges and its registeredImages entry.
  • The Windows Module destructor in DynamicLoader.cpp unregisters the image before FreeLibrary. If the wait fails (lease still held after APS5_PIN_WAIT_MS), the module stays loaded, the destructor prints the error, and it returns, so GPU work never sees a freed image.
  • If FreeLibrary only drops a reference and the image stays mapped, the next sceKernelMprotect on it registers it again from the current page protections.

The test (guest_memory, Windows) now loads the fixture through dlopen_nid_postfix and checks:
mprotect → registered → dlclose while a lease is held (it blocks, image still mapped) → lease dropped → dlclose returns, range no longer registered → a new allocation at the freed address registers → reload → mprotect registers again → dlclose → unregistered.

With the unregister call removed, the test fails at the "dlclose blocks while leased" check.

Full build + ctest: 477/477 pass (Windows, MinGW, RTX 3070).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

~Module in DynamicLoader.cpp catches the UnregisterImage failure, prints it and leaves the module loaded. Failures need to stay explicit here; please unregister in dlclose_nid_postfix before the Module is destroyed and let the exception propagate.

@tv7

tv7 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@oneandonlydean done in 0040990:

  • ~Module no longer touches the registry; it only frees the library.
  • dlclose_nid_postfix calls UnregisterImage before erasing the handle, so a failure (image still leased) propagates and the handle, symbols and protection stay valid. The handle is erased and the library freed only after unregistering succeeds.
  • If another open handle refers to the same image (LoadLibrary refcount), the image stays registered until the last handle closes.
  • guest_memory: dlclose under a held lease now has to throw and leave the module usable and protected (APS5_PIN_WAIT_MS=1000 in that test), then the existing close-after-release, reload and address-reuse checks run.

Full build + ctest 477/477 on Windows (GCC 15.2 UCRT, RTX 3070); check_conventions.py passes.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Conflicts with main in core/libs/tests/GuestMemory.cpp; please rebase. The dlclose change is right.

tv7 and others added 3 commits October 10, 2026 10:23
On Windows, sceKernelMprotect accepted only the executable's image and threw
"Memory protection of a foreign image is not supported" for every other
image. Bundled title modules are relinked into DLLs named <module>.guest.prx,
so a title that reprotects its own module pages failed. Dying Light 2's
bundled libNRD does this at start-up (boykopovar#1794).

DoMprotect now also accepts images whose file name ends in .guest.prx, and
registers their committed pages like the executable's. Other images are
still refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A .guest.prx image registered by sceKernelMprotect stayed in the guest
allocation registry after dlclose freed it, so its dead ranges blocked
later mappings at that address and could reach GPU builds.

The Windows module destructor now unregisters the image before
FreeLibrary. Unregistering waits for outstanding leases on the image like
unmap and protect do; if the wait fails the module stays loaded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…te failures

Move the image unregistration out of the Module destructor into
dlclose_nid_postfix. It runs while the handle is still registered, so a
failure (the image stays leased by GPU work) propagates to the caller and
leaves the module loaded and usable. Images shared by another open
handle stay registered until the last handle closes.

guest_memory now checks that dlclose under a held lease throws and keeps
the module, its symbols and its protection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@adriadam10
adriadam10 force-pushed the fix/mprotect-guest-module-images branch from 0040990 to b1a14c6 Compare October 10, 2026 08:28

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased onto current main as requested (single conflict in core/libs/tests/GuestMemory.cpp, main(): kept the _putenv_s under _WIN32 and the two new checks from main). Built and ran guest_memory and the libc/libkernel tests on Linux: green. The Windows case needs the Windows job; merging once CI is green.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@github-actions

github-actions Bot commented Oct 10, 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
#861 core/libs/tests/GuestMemory.cpp L536, L1269-1270
#1905 core/libs/prx/libc/include/GuestAllocations.hpp L60-61
#1961 core/libs/prx/libc/include/GuestAllocations.hpp L60-61
#2254 core/libs/tests/GuestMemory.cpp L21, L1269

@adriadam10
adriadam10 merged commit 8fded89 into boykopovar:main Oct 10, 2026
14 checks passed
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.

4 participants