Skip to content

feat(libkernel): guest memory protection and VirtualQuery on macOS hosts - #10898

Merged
adriadam10 merged 7 commits into
boykopovar:mainfrom
imellag:feat/libkernel-memory-protection-macos
Oct 27, 2026
Merged

adriadam10 merged 7 commits into
boykopovar:mainfrom
imellag:feat/libkernel-memory-protection-macos

Conversation

@imellag

@imellag imellag commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What

This started as mugurc's f66a4df741 from his macOS series (ci/macos-series-2, listed on #1), without the mlock check that #10889 adds. #10889's change, which this branch carried as its first commit, is now in main; adriadam10 rebased the branch onto it as 896ae02ab5 and dropped that commit. mugurc is co-author of the first commit; I offered on #1 to open it for him. The later commits address the reviews.

On macOS hosts:

  • mprotect. The guest rounds a range to its 16 KiB pages. A Mach-O image is loaded at a multiple of the host page size only (4 KiB under Rosetta), so DoMprotect counts the pages from where the ELF address 0 of the image holding the range was loaded (GuestAllocationsGuestPages, which counts a segment up to the end of its last host page). A rounded start smaller than a guest page that lies in the gap between two segments, where no host page exists, is left out (TrimToRegistered). The host protection and the write watch's record of it change together under the watch's lock (GuestWriteWatchProtect), so a collection on another thread cannot leave a page read-only while the watch records it writable. Both branches of DoMprotect go through it: the guest image and mapping spans, and guest thread stacks.
  • sceKernelVirtualQuery reads the region with mach_vm_region instead of /proc/self/maps, and limits it to the write watch run that holds the address, taking a watched page's write permission from the watch.
  • sceKernelMlock, sceKernelMunlock, sceKernelMunlockall. macOS has no munlockall: it fails there with ENOSYS and would overwrite errno, so sceKernelMunlockall unlocks the ranges libkernel locked, one by one. XNU counts every wire of a page, and the locked ranges merge overlapping locks, so mlock now wires only the pages libkernel has not wired yet, plus recorded ones a mapping has replaced since (their wire count is 0). One munlock then unwires a page however many times the guest locked it, as on FreeBSD and Linux. munlock and munlockall update the record under the same lock as the host call.
  • Tests. GuestKernelStackProtect reads page protections with mach_vm_region and keeps its main stack checks on Linux, where they can run. GuestMlock checks the wire count on macOS, also after a fixed remap of a locked range, and WriteWatch checks GuestWriteWatchProtect.

The libc and libkernel debt entries describe the macOS rounding, sceKernelVirtualQuery's extent, which write watch calls are wired, and how libkernel counts wires on macOS. Linux and Windows behave as before: the new code is under __APPLE__, and mlock there still calls the host once per request.

Tested

Apple M4 Pro, macOS 27.0.1, Apple clang 21, x86-64 Release under Rosetta 2 (-DCMAKE_OSX_ARCHITECTURES=x86_64, Ninja, BUILD_TESTING=ON).

main alone doesn't build libkernel on macOS. The tree is main 61fc572cee plus the open macOS build PRs (#10366, #10419, #10552, #7631 with the two casts I reported there, #7804, #4630, #10540, #10659, #10660, #10669, #10676) and the parts of mugurc's series that #7201 also carries, with #10889 and #9882 (merged as described below).

Same-tree A/B, ctest -R '^guest_' -j6, two runs each, at 3895f10311 (see the comment thread for the lists):

  • without this PR: 20 of 672 fail;
  • with it: 15 fail. guest_credentials, guest_kernel_is_stack, guest_kernel_stack_protect, guest_mlock and guest_system_configuration pass, and nothing fails only with it.

At 7729b6d75d, on the same tree with #10530, #10876, #10903, #10907, #9668 and #10345 added (test-side or unrelated library fixes), ctest -R '^(guest_|write_watch)' -j6 (682 tests) has 13 failures. They are the same tests that fail without this PR, minus the SSL transport pair that #10530 fixes. In one of the two runs guest_posix_scalar_io also timed out; it passes 10 out of 10 alone and uses none of the changed code. The focused set (guest_memory, guest_mlock, guest_mprotect_unmapped, guest_kernel_stack_protect, guest_system_configuration, guest_kernel_is_stack, guest_credentials and the ten write_watch_* tests) passes 3 runs out of 3. With the wire-once part reverted, guest_mlock fails at its new wire-count check.

At 32f4ad23d1 (rebased onto #10889's 04638bf776; the stack branch of DoMprotect now goes through GuestWriteWatchProtect too; the tree carries #7631's version of that branch, whose macOS mprotect I routed the same way), the same tree has 13 failures in ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6 (674 tests), all of them tests that fail without this PR, and the focused set passes 3 runs out of 3.

At f8fac50a31 (on macOS the stack branch makes one GuestWriteWatchProtect call and sets errno only on failure; GuestAllocationsGuestPages rounds a segment's start down to the host page as FindImageContaining does), the same tree has 14 failures in ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6 (674 tests). The same 14 fail with this commit's two code changes reverted, and none of them uses the changed code. The focused set of 17 passes 3 runs out of 3.

The current head 119e879525 is f8fac50a31 rebased onto main 14c99b52ac. git range-diff shows changes only in context lines of docs/dev/debt/libkernel.md, where main rewrote the connect, mlockall, munlockall and socket entries; this PR's mlockall, munlockall entry is now main's line plus "; macOS is under Locked memory on macOS". The net diff of the 9 code files is unchanged. maximilianfv reran the focused set at this head: 17 of 17, 3 runs out of 3.

Linux: the changed .cpp files, except the macOS-only WriteWatch.cpp test, pass g++ -std=gnu++20 -fsyntax-only with GCC 14 (gcc:14 image, x86-64). Not built or run on Windows.

check_conventions.py --base upstream/main passes. pr_overlap.py reports two conflicts:

#7201 also implements, for macOS: the GuestKernelStackProtect.cpp page query (both come from mugurc's series), the sceKernelVirtualQuery fallback through GuestWriteWatchProtection, sceKernelMunlockall as one munlock per recorded range, and a write watch update in both branches of DoMprotect (through GuestWriteWatchProtectionChanged). Whichever of the two lands second drops its copy. #5021 and #7201 add their own mlock preflight in LockHostPages; #10889 records that overlap.

Checklist

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 1835 core/libs/prx/libc/src/GuestAllocations.cpp
PR 4449 core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp
PR 4630 docs/dev/debt/libkernel.md
PR 5021 core/libs/prx/libc/include/GuestAllocations.hpp
core/libs/prx/libc/src/GuestAllocations.cpp
core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp
core/libs/prx/libkernel/DirectMemory/Export.cpp
docs/dev/debt/libc.md
docs/dev/debt/libkernel.md
PR 6149 docs/dev/debt/libkernel.md
PR 6321 core/libs/prx/libc/src/GuestAllocations.cpp
PR 6713 docs/dev/debt/libc.md
PR 6936 docs/dev/debt/libkernel.md
PR 7081 docs/dev/debt/libc.md
PR 7088 docs/dev/debt/libkernel.md
PR 7105 core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp
core/libs/prx/libkernel/DirectMemory/Export.cpp
docs/dev/debt/libkernel.md
PR 7121 docs/dev/debt/libkernel.md
PR 7149 core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp
docs/dev/debt/libkernel.md
PR 7192 core/libs/prx/libc/include/GuestWriteWatch.hpp
core/libs/prx/libc/src/GuestWriteWatch.cpp
docs/dev/debt/libkernel.md
PR 7386 docs/dev/debt/libc.md
PR 7447 docs/dev/debt/libc.md
docs/dev/debt/libkernel.md
PR 7509 docs/dev/debt/libkernel.md
PR 7648 docs/dev/debt/libc.md
PR 7715 docs/dev/debt/libc.md
PR 7804 docs/dev/debt/libkernel.md
PR 8150 docs/dev/debt/libc.md
PR 8605 docs/dev/debt/libkernel.md
PR 8937 docs/dev/debt/libc.md
PR 9156 docs/dev/debt/libkernel.md
PR 9242 docs/dev/debt/libkernel.md
PR 9284 docs/dev/debt/libkernel.md
PR 9293 docs/dev/debt/libkernel.md
PR 9297 docs/dev/debt/libkernel.md
PR 9302 docs/dev/debt/libkernel.md
PR 9371 docs/dev/debt/libc.md
PR 9389 docs/dev/debt/libkernel.md
PR 9447 docs/dev/debt/libc.md
PR 9491 docs/dev/debt/libkernel.md
PR 9526 docs/dev/debt/libc.md
PR 9557 docs/dev/debt/libkernel.md
PR 9566 docs/dev/debt/libkernel.md
PR 9575 docs/dev/debt/libkernel.md
PR 9578 docs/dev/debt/libkernel.md
PR 9929 docs/dev/debt/libc.md
PR 10319 docs/dev/debt/libkernel.md
PR 10324 docs/dev/debt/libc.md
PR 10433 docs/dev/debt/libc.md
PR 10442 docs/dev/debt/libkernel.md
PR 10558 docs/dev/debt/libkernel.md
PR 10620 docs/dev/debt/libc.md
PR 10725 docs/dev/debt/libkernel.md
PR 11002 docs/dev/debt/libkernel.md
PR 11042 core/libs/prx/libkernel/DirectMemory/Export.cpp
core/libs/tests/GuestMlock.cpp
docs/dev/debt/libkernel.md
PR 11215 docs/dev/debt/libkernel.md
PR 11267 docs/dev/debt/libkernel.md
PR 11332 docs/dev/debt/libc.md

@MarcZX10

Copy link
Copy Markdown
Contributor

I checked the exact current heads against each other with git merge-tree: #9882 and this branch conflict in core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp; the other touched files auto-merge. Your Tested section accurately mentions that whichever lands second needs a rebase, but the checklist currently marks Depends on: #10889 as complete and leaves #9882 only in prose. Please coordinate/record the intended order on the checklist (Depends on: #9882 if this PR is to land after it, or otherwise an explicit order agreement with #9882’s author), then rebase onto the first PR if needed. The overlap is real and needs an agreed merge order under CONTRIBUTING.md. I ran git diff --check and python3 tools/check_conventions.py --base upstream/main --head HEAD; both pass. I did not run an Apple build locally, and the PR reports that guest_kernel_stack_protect fails in both its before/after macOS trees, so I’m holding review pending clarity on that test result and the merge order.

@imellag

imellag commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. Both points:

Order with #9882. #9882 is older, measured on a PS5 and already approved, so this PR lands after it. The checklist now says Depends on: #10889 and #9882. Their only conflict is one line in DoMprotect: #9882 makes the start check conditional (requireMappedStart), and this PR compares the first span with pointer, which TrimToRegistered may move on macOS. Merged, the line becomes:

if (requireMappedStart && (spans.empty() || spans.front().first != pointer)) return SCE_KERNEL_ERROR_EINVAL;

I merged #9882's head b46ae368c3 into this branch locally with that resolution and tested it on the same macOS tree. 16 of 672 guest_ tests fail, against 17 with this PR alone. guest_mprotect_unmapped (#9882's test), guest_memory, guest_mlock, guest_kernel_stack_protect, guest_system_configuration, guest_kernel_is_stack and guest_credentials pass. I'll rebase onto main once #9882 is merged.

guest_kernel_stack_protect. It failed at line 528 because its main stack checks run the test again in a child through /proc/self/exe, which macOS doesn't have. The child exited with 127 before testing anything, so the failure said nothing about this PR. The new commit 3895f10311 makes the child run the path _NSGetExecutablePath reports on macOS; Linux keeps /proc/self/exe. With it the test passes 3 runs out of 3. It also passes with this PR's library changes reverted, because what it needs from this PR is the test's own mach_vm_region page query.

AI-assisted: yes

@MarcZX10

Copy link
Copy Markdown
Contributor

Thanks for clarifying the macOS child re-exec failure and recording the #9882 order. The integrated run still reports 16 failures among 672 guest_ tests with #9882 + this PR, versus 17 with this PR alone, but does not identify the failures or give the same-tree baseline with #9882 and this PR reverted. Could you list the 16 failing test names/results and run that baseline (or otherwise show which failures predate this change)? That comparison is needed to tell whether these memory-management changes introduce any regressions. I will keep review pending that evidence.

@imellag

imellag commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Here is the same-tree baseline. The tree is main 61fc572cee plus the open macOS build PRs, the parts of mugurc's series that #7201 also carries, #10889, #9882 (head b46ae368c3, merged as described above), and four test-side PRs that don't touch memory management: #10876, #10903, #10907, and #9668 + #10345. I ran ctest -R '^guest_' -j6 twice on each side and toggled only this PR (both commits). Without it, DoMprotect is #9882's version, and GuestKernelStackProtect.cpp is main's.

Without this PR: 20 of 672 fail in both runs.
With it: 15 fail in both runs, and a 16th, guest_socket_read_write, fails in one of the two.

Fail in both trees (15):

  • guest_aio, guest_bad_optional_access, guest_floating_point, guest_host_standard_error (timeout), guest_itimer, guest_json_numeric_locale, guest_kernel_app_archive_mmap, guest_math_exact;
  • guest_module_info, guest_poll, guest_sce_net, guest_socket_pair, guest_socket_port_zero, guest_ssl_transport_kernel, guest_ssl_transport_net.

Fail only without this PR (5): guest_credentials, guest_kernel_is_stack, guest_kernel_stack_protect, guest_mlock, guest_system_configuration.

Fail only with this PR: none in both runs. guest_socket_read_write failed in one run with it and passed in the other. It also failed in an earlier run of this stack without this PR, and passes 5 out of 5 when run alone, so it is flaky under -j6 on this machine. It doesn't use any of the changed code (DoMprotect, sceKernelVirtualQuery, sceKernelMunlockall).

The 15 shared failures fail identically on both sides, and most already have open PRs: #10543 and #10552 for aio and module_info, #8311 for bad_optional_access, #9654 for host_standard_error, #10505 for itimer and kernel_app_archive_mmap, #10888 for math_exact, and #9509, #9625, #9733 and #10544 for the socket tests. json_numeric_locale fails because libc++'s istream sets failbit on an underflowing number in the JSON parser, which #10378 rewrites. ssl_transport_* stop at a server certificate without a loaded CA passed the initial verify options.

AI-assisted: yes

@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.

Reviewed the updated head 3895f10311c7457838140693751631fd789d059e. The author has now provided two same-tree guest_ runs with and without this change: 20 failures without it versus 15 in both runs with it, plus one unrelated flaky socket test in one changed-tree run. No failure is exclusive to this change in both runs; the five tests that pass only with this change are consistent with the intended memory-protection/query behavior. The PR also records the required order after #9882, the repaired macOS child re-exec test, and the limits of the reported console evidence. I independently ran git diff --check upstream/main...HEAD and python3 tools/check_conventions.py --base upstream/main --head HEAD; both passed. Hosted relinker, ASan, write-watch, and register-context checks pass on this head; Linux/Windows product jobs are skipped by path policy.

@maximilianfv maximilianfv 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.

Requesting changes. On this Mac the A/B is clean: the four tests the body names, plus guest_kernel_stack_protect, go from abort to pass, and nothing new fails in the guest_ suite. But I reproduced one defect: macOS sceKernelMunlockall leaves pages wired (item 1). Items 2 to 4 are code issues.

Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)

Head 3895f10311. The tree is mugurc's macOS series (27e968974) merged with main at 2742004ed2, plus other open macOS fixes. That series already carries this PR's mprotect, sceKernelVirtualQuery and sceKernelMunlockall code, as mugurc's own commit. I set up the two states like this:

  • Without: I reverse-applied the code hunks of 7d41eab9a5. I kept the __APPLE__ include block in GuestAllocations.cpp, because other series code needs it. I used GuestKernelStackProtect.cpp from fa633d11ad.
  • With: I re-applied those hunks and used GuestKernelStackProtect.cpp from 3895f10311.

Both states have the series' own mlock check in place of #10889's. Release build, ctest -R '^guest_' -E 'audio_in|voice' -j6, 664 tests.

Without this PR With this PR
guest_mlock abort, GuestMlock.cpp:112: *__error_nid_postfix() == untouched after munlockall_nid_postfix() Passed
guest_credentials abort, GuestCredentials.cpp:70: getresuid_nid_postfix(...) == 0 Passed
guest_kernel_is_stack abort, GuestKernelIsStack.cpp:50: sceKernelIsStack(...) == SCE_OK Passed
guest_system_configuration abort in CheckVmmapSysctl: *__error_nid_postfix() == 13 Passed
guest_kernel_stack_protect abort: ChildPasses fails at the /proc/self/exe re-exec Passed, but its children skip every main stack check (item 5)
guest_memory, guest_mprotect_unmapped Passed Passed
guest_ suite 44 of 664 fail 38 of 664 fail, none new

The focused set passed 3 runs out of 3 with this PR. Two network tests are flaky both ways and are not affected by this PR:

  • guest_socket_read_write failed in both full runs. Alone it passed 34 of 40 runs without this PR and 33 of 40 with it.
  • guest_refused_connect passed 1 of 8 runs alone without this PR and 3 of 8 with it.

Not built on Linux or Windows.

Findings

  1. core/libs/prx/libkernel/DirectMemory/Export.cpp:794: sceKernelMunlockall leaves pages wired on macOS.

    • XNU counts every mlock on a page (user_wired_count), and RecordLockedRange merges overlapping locks into one entry. So one munlock per recorded range only takes one count off.

    • Reproduced on this tree:

      Step user_wired_count (mach_vm_region)
      sceKernelMapFlexibleMemory 64 KiB, then mlock_nid_postfix twice on it 2
      munlockall_nid_postfix(), which returns 0 1
    • Linux munlockall unwires everything. So does FreeBSD's: as docs/dev/debt/libkernel.md:249 states, sys_munlockall runs vm_map_unwire over the whole map with VM_MAP_WIRE_HOLESOK.

    • Fix: count only the wires libkernel added, and undo exactly that many. Either keep a per-page lock count next to g_lockedRanges and call munlock that many times, or have LockHostPages skip ::mlock on pages that are already recorded. Don't read user_wired_count and unlock that many times: the count also includes wires made by other code, such as host libraries.

    • A plain retry loop does not work either, because macOS munlock returns 0 on pages that are not wired.

    • sceKernelMunlock has the same counting gap, but that code is already on main and #10889.

  2. core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp:1382-1384: the write watch can disagree with the host protection.

    • The host mprotect and GuestWriteWatchProtectionChanged run in sequence under the allocation lock, not under the watch's _lock.
    • A Collect on another thread can run between them. It sees Written with PROT_WRITE, sets Protected and maps the page read-only. ProtectionChanged then writes Written | protection and drops Protected.
    • The page stays read-only while the state says it is writable. The next guest write faults, record() returns true because the state has PROT_WRITE, and the write retries forever.
    • I found this by reading GuestWriteWatch.cpp (Collect, ProtectionChanged, record); I did not reproduce it. This PR adds the first caller.
    • Fix: run the host mprotect and the state update under _lock. For example, add a write-watch call that does both, and call it instead of mprotect plus GuestWriteWatchProtectionChanged.
  3. core/libs/prx/libc/src/GuestAllocations.cpp:498: GuestAllocationsGuestPages matches the exact segment extent.

    • It matches [p_vaddr, p_vaddr + p_memsz), but FindImageContaining and collectImagePages round segments up to host pages.
    • An address in a segment's last host page past p_memsz gets bias 0 and is rounded from absolute 16 KiB boundaries.
    • Example: image loaded at B with B mod 0x4000 = 0x1000, one segment with p_vaddr 0 and p_memsz 0x4100. mprotect(B + 0x4200, 0x100) gives first = B + 0x3000, and the span after the trim is [B + 0x3000, B + 0x5000). That changes 4 KiB of guest page 0, which the guest did not name. With the image's bias the span is [B + 0x4000, B + 0x5000).
    • Fix: take the bias from image.base, which FindImageContaining already returns in DoMprotect. The rounding at DirectMemory.cpp:1304 would have to move after that search. That also saves the second dl_iterate_phdr walk per call. If you keep the walk, round p_memsz up to the host page, as the other two functions do.
  4. docs/dev/debt/libc.md:120 is now stale.

    • It says "Nothing calls either yet: the macOS memory paths are not in the tree".
    • DoMprotect now calls GuestWriteWatchProtectionChanged, and nothing calls GuestWriteWatchProtection yet.
    • Fix: say which of the two is wired.
  5. core/libs/tests/GuestKernelStackProtect.cpp:521-525: the re-exec works, but the checks it reaches skip on macOS. On this Mac every child prints a skip message and exits 0:

    • --read-only-bottom and --reprotect-bottom: "skipped the main stack bottom checks, the stack is mapped within 1 MiB of its bottom" (line 493);
    • the unlimited-stack check: "skipped the unlimited main stack check, the stack limit cannot be raised or overcommit is strict" (line 540).

    The test passes, but no main stack check runs on macOS, so the commit subject overstates what it tests. Either say so in the commit and body, or keep macOS out of those checks the way mugurc's series does at 27e968974 (#elif defined(__linux__), with only ProtectStack(0) on other hosts).

  6. Question: Export.cpp:398. The macOS sceKernelVirtualQuery fallback reads mach_vm_region's protection directly. libc.md:120 asks code that reads a page's protection to ask GuestWriteWatchProtection first. This path only runs for memory the registry does not know. Can a watched page reach it? I did not trace every GuestWriteWatchRegister caller.

  7. Nits.

    • GuestAllocations.cpp:20-21: nothing in the file uses <mach-o/dyld.h> or <mach-o/loader.h>.
    • GuestAllocations.cpp:521-525: the end trim has no effect. No registered range starts in [finish, end), so ForRegistered returns the same spans, and only the start trim changes the outcome.
    • GuestAllocations.cpp:514: guestPage is hard-coded as 0x4000, while GuestAllocationsGuestPages takes page as a parameter.

AI-assisted: yes (Claude Code applied the PR, ran the tests, read the code and wrote this review).

@imellag

imellag commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@maximilianfv Thanks for the careful review. The new commits are abc443d1df (test) and 08fcb44bb0 (library), head 08fcb44bb0. Point by point:

  1. munlockall leaving pages wired. Fixed in the library. On macOS, mlock now wires only the parts of the range that g_lockedRanges does not already hold, under g_lockedRangesLock, and undoes its own wires if a later part fails. libkernel then holds at most one wire per page, so the one munlock per recorded range in sceKernelMunlockall, and the one in sceKernelMunlock, unwire it completely. Wires from other code are left alone. GuestMlock now checks user_wired_count on macOS: 1 after three overlapping locks, 0 after munlock, and 0 after munlockall of a page locked twice. With the wire-once part reverted, it fails at the first of those checks. Linux still calls mlock once per request.

  2. Write watch race. Fixed as you suggested. GuestWriteWatchProtect takes the watch's lock, calls mprotect and records the protection, and DoMprotect calls it instead of mprotect plus GuestWriteWatchProtectionChanged. The protection write watch test now protects a watched page with it, checks the host and recorded protection, and checks that a write after restoring it is collected.

  3. GuestAllocationsGuestPages extent. I kept the walk and round p_memsz up to the host page, as FindImageContaining and collectImagePages do. fix(libkernel): protect ranges with unmapped pages as measured on a PS5 #9882 replaces FindImageContaining with a search over every image a range overlaps, so taking the bias from its result would tie this PR to fix(libkernel): protect ranges with unmapped pages as measured on a PS5 #9882's code even more. I'd rather do that as a follow-up once both are in.

  4. libc.md entry. Updated: changes go through GuestWriteWatchProtect, mappings call GuestWriteWatchProtectionChanged, and sceKernelVirtualQuery asks GuestWriteWatchProtection.

  5. Stack protect test. You're right, the children skip on macOS. The main stack checks now build and run on Linux only, as in mugurc's series, and other hosts run ProtectStack(0). The description no longer claims more.

  6. sceKernelVirtualQuery fallback. I couldn't rule it out for every GuestWriteWatchRegister caller either, so the fallback now takes the write permission from GuestWriteWatchProtection when the page is watched.

  7. Nits.

    • The Mach-O includes are gone.
    • The end trim is gone, since it never changed the spans.
    • TrimToRegistered takes the page size from the caller.

    While doing this I also found that the separate #elif defined(__APPLE__) include block, taken from the series, hid the #else block's includes from macOS. Combined with fix(libc): find guest image pages through dyld on macOS hosts #10366, which adds its mach-o includes in that #else block, the file wouldn't compile. The block is gone, and ElfTypes.hpp is included for Apple after main's block.

Tested on the same M4 Pro tree, now with #10530, #10876, #10903, #10907, #9668 and #10345 as well. ctest -R '^(guest_|write_watch)' -j6 (682 tests) has 13 failures, all ones that fail without this PR too. The focused set (guest_memory, guest_mlock, guest_mprotect_unmapped, guest_kernel_stack_protect, guest_system_configuration, guest_kernel_is_stack, guest_credentials and the ten write_watch_* tests) passes 3 runs out of 3. On Linux, the changed .cpp files except WriteWatch.cpp pass -fsyntax-only with GCC 14. The description is updated.

AI-assisted: yes

@maximilianfv maximilianfv 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.

Requesting changes again, for a new reason. The fixes for my earlier points work: the macOS munlockall now leaves no wire behind, and the write watch race is closed. But wire-once creates a new problem on macOS. After a range is replaced in place, mlock returns 0 and wires nothing (item 1, reproduced). The A/B is clean otherwise.

Re-review of 08fcb44bb0 against my review at 3895f10311:

Earlier point Status at 08fcb44bb0
1. munlockall leaves pages wired Fixed. My probe now reads 1 wire after two locks and 0 after munlockall (it read 2 and 2 without this PR). But see item 1 below.
2. Write watch race Fixed. Watch::Protect holds _lock across mprotect and the state update, and writes the same state ProtectionChanged does.
3. Bias of GuestAllocationsGuestPages Fixed for the example I gave.
4. Stale libc.md entry Rewritten. One detail is still off (item 3).
5. Stack protect children skip Fixed. The main stack checks now run on Linux only.
6. sceKernelVirtualQuery and the write watch Fixed for the write bit. A nit about the extent remains (item 4).
7. Nits Fixed.

Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)

Applied at 08fcb44bb0 to mugurc's macOS series (27e968974) merged with main at 2742004ed2, with other open macOS fixes, then toggled only this PR. The series already carries this PR's first library commit, so I set up the two states like this:

  • Without: the code hunks of 7d41eab9a5 reversed, as in my earlier review, with main's GuestKernelStackProtect.cpp and WriteWatch.cpp. WriteWatch.cpp is main's because the new checks need GuestWriteWatchProtect, which only this PR adds. The new GuestMlock.cpp is used in both states.
  • With: the 11 files at 08fcb44bb0.

Two local build changes were needed on both sides. Neither touches this PR's logic:

  • GuestAllocations.cpp at this head does not compile on this tree. The error is GuestAllocations.cpp:26: fatal error: 'link.h' file not found (see item 2).
  • The series' own dyld code in that file needs <mach-o/dyld.h> and <mach-o/loader.h>. I put <link.h> under #ifndef __APPLE__ and added those two includes to the new #ifdef __APPLE__ block.

Release build. The focused set is guest_memory, guest_mlock, guest_mprotect_unmapped, guest_kernel_stack_protect, guest_system_configuration, guest_kernel_is_stack, guest_credentials and the ten write_watch_* tests. The suite run is ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6 (674 tests). I ran the "with" state first, then "without", then rebuilt "with" and ran the focused set again.

Without this PR With this PR
guest_mlock abort, GuestMlock.cpp:92: WiredCount(mapped) == 1 (the new wire-count check) Passed
guest_credentials abort, GuestCredentials.cpp:70 Passed
guest_kernel_is_stack abort, GuestKernelIsStack.cpp:50 Passed
guest_system_configuration abort, GuestSystemConfiguration.cpp:710 in CheckVmmapSysctl Passed
guest_kernel_stack_protect abort, GuestKernelStackProtect.cpp:509: ChildPasses("--read-only-bottom") Passed
guest_memory, guest_mprotect_unmapped, 10 write_watch_* Passed Passed
Focused set (17 tests) 12 of 17 pass 17 of 17, in 4 runs out of 4
Suite 44 of 674 fail 39 of 674 fail; the 5 above are the only difference, nothing new
Probe: mlock twice, then munlockall (user_wired_count) 2, then 2 1, then 0
Probe: mlock, fixed remap, mlock again (user_wired_count) 1, 0, then 1 1, 0, then 0 (item 1)

guest_avplayer and guest_refused_connect failed in both suite runs. guest_avplayer fails its timing check (eof stop median ... exceeds 10 ms) both ways when run alone: it passed 1 of 10 runs without this PR and 4 of 10 with it. Neither test uses the changed code.

Not built on Linux or Windows.

Findings

  1. core/libs/prx/libkernel/DirectMemory/Export.cpp:703-725: on macOS, mlock can now succeed without locking anything.

    • WireHostPages skips ::mlock on every page that g_lockedRanges records. Only sceKernelMunmap (:318) and munlock (:769) forget a range.

    • A mapping that replaces a locked range in place leaves the record behind. On macOS that new mapping has no wires. Paths that do this:

      • a fixed sceKernelMapFlexibleMemory, sceKernelMapDirectMemory or mmap, through RemapFixedIntoRegistered;
      • sceKernelReleaseDirectMemory, through UnmapDirectMemoryViews (DirectMemory.cpp:1586);
      • the thread stack DoMunmap calls in Thread.cpp.
    • Reproduced on this tree:

      Step user_wired_count without this PR user_wired_count with this PR
      sceKernelMapFlexibleMemory 64 KiB, then mlock_nid_postfix 1 1
      sceKernelMapFlexibleMemory at the same address with 0x10 (fixed), then a fixed anonymous mmap_nid_postfix 0 0
      mlock_nid_postfix again, which returns 0 1 0
    • FreeBSD wires the new pages on that second mlock. Before this PR macOS did too, because the stale record was harmless while mlock always called the host.

    • Fix: forget the locked range wherever a mapping is removed or replaced, not only in the sceKernelMunmap export.

      • _releaseFlexible (Export.cpp:170) already runs after every map, fixed remap and munmap in this file, so a ForgetLockedRange there covers most of them.
      • Add it to DoMunmap and UnmapDirectMemoryViews for the other paths.
    • A GuestMlock check for this sequence would be: mlock, fixed remap, mlock, then WiredCount == 1.

  2. core/libs/prx/libc/src/GuestAllocations.cpp:26: on macOS this file now depends on #10366.

    • Without the old #elif defined(__APPLE__) block, Apple builds take main's #else block, which includes <link.h>. macOS has no <link.h>.
    • On mugurc's series with main, which built this file at 3895f10311, the build now stops there.
    • Removing the block makes sense next to #10366, but the checklist only lists #10889 and #9882. Please add #10366 to Depends on, or put <link.h> under #ifndef __APPLE__ here.
  3. docs/dev/debt/libc.md:120: the rule for mappings does not match the code.

    • The entry says code that maps over watched pages "has to call GuestWriteWatchProtectionChanged", and your reply says mappings call it.
    • At this head nothing outside tests/WriteWatch.cpp calls it. The mapping paths call GuestWriteWatchRegister again (DirectMemory.cpp, around lines 607, 756 and 951), which re-reads the host protection.
    • Please describe what the mapping paths actually do, and say that GuestWriteWatchProtectionChanged has no caller yet.
  4. core/libs/prx/libkernel/DirectMemory/Export.cpp:408-410 (nit):

    • watchedEnd is computed but never used. Its limit, address + 1, makes it one page at most anyway.
    • The write bit comes from the watch for the queried page only, but [start, end) is the whole mach_vm_region entry. A watched, armed page and a read-only page next to it can share one host entry.
    • Fix: pass info->end as the limit, clamp info->end to watchedEnd, and do the same on the start side.
  5. Question: Export.cpp:766-775 and :827, a race I found by reading the code and did not reproduce.

    • UnlockHostPages calls ::munlock before it takes g_lockedRangesLock in ForgetLockedRange. sceKernelMunlockall likewise unwires outside the lock, after swapping the map out.
    • Example: a munlock([A, B)) unwires, then a concurrent mlock([A, C)) sees [A, B) still recorded and wires only [B, C). The forget then drops [A, B). The mlock returned 0, but [A, B) is not wired.
    • On macOS, holding the lock across ::munlock and the forget, as WireHostPages does, closes this.

AI-assisted: yes (Claude Code applied the PR, ran the tests, read the code and wrote this review).

@imellag

imellag commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@maximilianfv Thanks again, all five are real. New commit 7729b6d75d:

  1. mlock after a remap. Fixed in WireHostPages. Inside a range that g_lockedRanges records, it now asks mach_vm_region for each host region and wires again the ones whose user_wired_count is 0. That covers every path that replaces a locked range (fixed map, UnmapDirectMemoryViews, the thread stack DoMunmap) without hooking each one. GuestMlock now does your sequence on macOS: lock, sceKernelMapFlexibleMemory fixed at the same address (the wire count drops to 0), lock again, and require a wire count of 1. Without the fix it fails there; with it the test passes 3 runs out of 3.
  2. <link.h> on macOS. Right. Without fix(libc): find guest image pages through dyld on macOS hosts #10366, main's include block includes <link.h> on Apple. I'd rather not touch those lines, because fix(libc): find guest image pages through dyld on macOS hosts #10366 changes them, so I've added fix(libc): find guest image pages through dyld on macOS hosts #10366 to Depends on.
  3. libc.md. It now says what the mapping paths do: they call GuestWriteWatchRegister again, which reads the new host protection. It also says that GuestWriteWatchProtectionChanged has no caller yet.
  4. sceKernelVirtualQuery extent. The fallback now walks GuestWriteWatchProtection runs from the start of the host region and reports the run that holds the address. So the start, the end and the write bit all come from one run, and the unused watchedEnd is gone.
  5. Unlock race. On macOS UnlockHostPages holds g_lockedRangesLock across ::munlock and the record update, and sceKernelMunlockall unwires inside the lock before releasing it. Linux and Windows are unchanged.

Retested on the same M4 Pro tree as before:

  • ctest -R '^(guest_|write_watch)' -j6 (682 tests): 13 failures in both runs, the same ones that fail without this PR.
  • The focused set (the seven guest_ memory tests and the ten write_watch_*) passes 3 runs out of 3.
  • On Linux the changed files pass g++ -std=gnu++20 -fsyntax-only with GCC 14.

AI-assisted: yes

@krishalivaibhav

Copy link
Copy Markdown
Contributor

Test report: #10898 (sceKernelMunlockall on macOS)

Tested on: Apple M2 MacBook Air, macOS 27.2 (26B118), x86_64 Release build under Rosetta 2, AppleClang 21.0.0, Ninja.
Tree: origin/main a5a780e merged into a scratch branch holding our open macOS stack (integ8 029564a43: #10552, #10853 and their dependencies, plus the process-parameters, mlock-PROT_NONE and math-exact fixes). Base commit 1ddafb62d. PR head 08fcb44 (5 commits, builds on our #10889).

Conflict: one hunk, core/libs/prx/libkernel/DirectMemory/Export.cpp, around LockHostPages/WireHostPages. Resolved on the scratch branch as agreed with the coordinator: took the PR's side (new WireHostPages, RecordLockedRangeLocked, munlockall changes) and kept our #10889 mach_vm_region walk inside LockHostPages (ENOMEM on KERN_INVALID_ADDRESS, throw on other failures, ENOMEM when the region is unreadable and not executable) in place of the PR's info.protection == VM_PROT_NONE check. PR otherwise unchanged.

test (3 runs each) without with
guest_mlock abort 3/3 pass 3/3

Broad run: ctest -R "^(guest_|libc_|libkernel_)" = 678 tests. On the base 133 do not pass, 344 of them "Not Run" because their executables do not build here (AGC driver SpirvOptimizer.cpp needs pthread_getattr_np; guest_tls_get_addr and guest_memory test objects fail to compile). Those are Not Run both ways. With the PR: 129 do not pass. Newly passing versus base: guest_mlock, guest_kernel_is_stack, guest_kernel_stack_protect, guest_system_configuration, guest_credentials. Newly failing: guest_refused_connect, which is flaky on the base too (abort in 2 of 3 base reruns), so I do not attribute it to the PR.

Not checked: other platforms; the individual tests besides guest_mlock in isolation for 3 runs; real-title behaviour.

AI-assisted: yes (Claude Code)

@maximilianfv maximilianfv 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.

Approving. The five points from my review at 08fcb44bb0 are fixed, and the macOS A/B is clean: the remap probe that showed the regression now reads 1 wire again, the focused set goes from 12 of 17 to 17 of 17, and no new test fails in the suite. Three text fixes remain (items 1 to 3). Item 4 is for when #7631 lands. Item 5 is a question. None of them needs a re-test.

Re-review of 7729b6d75d against my review at 08fcb44bb0:

Earlier point Status at 7729b6d75d
1. mlock after a fixed remap wires nothing Fixed. AddUnwiredHostPages (Export.cpp:717-733) wires each host region in a recorded range whose user_wired_count is 0. My probe reads 1, 0, then 1 (it read 1, 0, then 0 at 08fcb44bb0). GuestMlock.cpp:93-97 now checks the same sequence.
2. <link.h> needs #10366 Handled: #10366 is in Depends on. This tree does not have #10366, so I still guard <link.h> locally (see Tested).
3. libc.md:120 mapping rule Fixed. Outside the tests, nothing calls GuestWriteWatchProtectionChanged, and the mapping paths call GuestWriteWatchRegister.
4. sceKernelVirtualQuery extent Fixed (Export.cpp:412-424). The loop terminates: both branches of Watch::Protection return a runEnd past the cursor.
5. Unlock race Fixed on macOS. UnlockHostPages and sceKernelMunlockall hold g_lockedRangesLock across ::munlock.

I also checked the failure path of WireHostPages. It unwires only the pieces this call wired. The pieces are disjoint and in ascending order, so the RLIMIT_MEMLOCK retry starts clean.

Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)

Applied at 7729b6d75d to mugurc's macOS series (27e968974) merged with main at 2742004ed2, with other open macOS fixes, then toggled only this PR. The series already carries this PR's first library commit, so I set up the two states as in my earlier reviews:

  • Without: the series' files with the code hunks of 7d41eab9a5 reversed, plus main's GuestKernelStackProtect.cpp and WriteWatch.cpp. The libraries built in this state export none of the PR's new functions (GuestAllocationsGuestPages, GuestAllocationsTrimToRegistered, GuestWriteWatchProtect). The new GuestMlock.cpp is used in both states.
  • With: the 11 files at 7729b6d75d, merged with the series' own macOS code in those files. As before, <link.h> is under #ifndef __APPLE__ and the series' <mach-o/dyld.h> and <mach-o/loader.h> are included, because this tree does not have #10366.

Release build. The focused set is guest_memory, guest_mlock, guest_mprotect_unmapped, guest_kernel_stack_protect, guest_system_configuration, guest_kernel_is_stack, guest_credentials and the ten write_watch_* tests. The suite run is ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6 (674 tests). Order: "with", then "without", then "with" rebuilt and run again. lldb gave the stop locations.

Without this PR With this PR
guest_mlock abort, GuestMlock.cpp:92: WiredCount(mapped) == 1 Passed
guest_credentials abort, GuestCredentials.cpp:70 Passed
guest_kernel_is_stack abort, GuestKernelIsStack.cpp:50 Passed
guest_system_configuration abort, GuestSystemConfiguration.cpp:710 in CheckVmmapSysctl Passed
guest_kernel_stack_protect abort, GuestKernelStackProtect.cpp:509: ChildPasses("--read-only-bottom") Passed
guest_memory, guest_mprotect_unmapped, 10 write_watch_* Passed Passed
Focused set (17 tests) 12 of 17, in 2 runs out of 2 17 of 17, in 4 runs out of 4
Suite (674 tests) 43 fail 39 fail. The 5 tests above pass. Nothing that passes without this PR fails with it, except two tests that fail alone both ways (below).
Probe: mlock twice, then munlockall (user_wired_count) 2, then 2 1, then 0
Probe: mlock, fixed sceKernelMapFlexibleMemory remap, mlock again (user_wired_count) 1, 0, then 1 1, 0, then 1 (it read 1, 0, then 0 at 08fcb44bb0)

Three tests differ between the two suite runs, and none of them uses the changed code. Run alone 6 times in each state:

  • guest_avplayer passed 0 of 6 without this PR and 0 of 6 with it.
  • guest_refused_connect passed 0 of 6 without and 1 of 6 with.
  • guest_socket_read_write passed 4 of 6 without and 3 of 6 with.

I also probed sceKernelMunlockall over a locked range whose first 16 KiB had been released with sceKernelReleaseDirectMemory. The rest of the range ended with 0 wires, so a released view does not leave a hole that makes the per-range munlock fail.

Not built on Linux or Windows. The ForgetLockedRangeLocked split builds on every host, but I only built it on macOS.

Findings

  1. docs/dev/debt/libkernel.md:246 is stale since 7729b6d75d.

    • It says sceKernelVirtualQuery "reports the region mach_vm_region returns, so its extent is the host's". The extent is now the host region cut to the write watch run that holds the address.
    • Fix: describe the extent as the host region cut to that run. CONTRIBUTING asks debt entries to state the gap first, so lead with the two gaps: the start edge skipped in a segment gap, and the extent being the host's rather than the guest's.
  2. docs/dev/debt/libkernel.md:249 has no macOS case for munlockall.

    • It lists only "VirtualUnlock on Windows ... and munlockall on Linux".
    • On macOS, libkernel now holds at most one wire per page and unwires each recorded range with one munlock.
    • Fix: add that sentence. Also say that wires made by other host code inside a recorded range are counted as libkernel's (item 5).
  3. The body's overlap line about #7201 understates the overlap.

    • The body says "Of #7201's changes, only the GuestKernelStackProtect.cpp page query is the same".
    • gh pr diff 7201 also implements, for macOS:
      • the sceKernelVirtualQuery fallback through GuestWriteWatchProtection;
      • sceKernelMunlockall under #if !defined(_WIN32) && !defined(__APPLE__), with one munlock per recorded range;
      • write watch notification in both DoMprotect branches.
    • Fix: list these next to the "no other open PR implements the same functions" box, so the landing order with #7201 is recorded.
  4. core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp:1337-1340: the thread stack branch of DoMprotect still calls plain mprotect on macOS.

    • libc.md:120 says code that changes the protection of watched pages has to go through GuestWriteWatchProtect, "as mprotect in libkernel does". The span branch does; this branch does not.
    • At this head the branch is harmless. MapAligned registers with the write watch only on Linux (DirectMemory.cpp:949-951), so macOS guest stacks are not watched.
    • #7631, which your test tree includes, changes that guard to defined(__linux__) || defined(__APPLE__). With it, the watch keeps a writable record for a stack page the guest made read-only. A write fault on that page then retries instead of faulting, and a Collect can make a PROT_NONE page readable. I found this by reading the code and did not reproduce it.
    • Fix: on macOS, use GuestWriteWatchProtect_nid_postfix for both mprotect calls in this branch, including the retry after MapUnmappedPages. Doing it here or in #7631 both work, as long as one of them does.
  5. Question: Export.cpp:729. The test user_wired_count == 0 counts wires made by any code.

    • Example: a recorded range is remapped in place, and host code wires the new pages. The next guest mlock skips them and adds no wire, and a later guest munlock removes the host's wire.
    • I can't name a path in the tree where host code wires guest pages, so this is a note for the debt entry (item 2), not a blocker.

AI-assisted: yes (Claude Code applied the PR, ran the tests, read the code and wrote this review).

@imellag
imellag force-pushed the feat/libkernel-memory-protection-macos branch from 7729b6d to 32f4ad2 Compare October 25, 2026 14:37
@imellag

imellag commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@maximilianfv Thanks for the re-review and the approval. New head 32f4ad23d1:

  1. libkernel.md macOS entry. It now leads with the two gaps: the start edge that is skipped in a segment gap, and sceKernelVirtualQuery reporting the host's extent (the mach_vm_region region cut to the write watch run that holds the address) instead of the guest's.
  2. Wires on macOS. These went into a new short entry next to it, "Locked memory on macOS", rather than into the mlockall, munlockall entry, which would have gone over the 1000-character limit. It says that libkernel holds at most one wire per page, that sceKernelMunlockall unlocks each recorded range with one munlock, and that a wire other host code makes on a page of a recorded range counts as libkernel's (your item 5).
  3. Macos port #7201 overlap. The body now lists what Macos port #7201 also implements for macOS: the sceKernelVirtualQuery fallback, sceKernelMunlockall per recorded range, and the write watch update in both DoMprotect branches. The "no other open PR" box points to that list.
  4. Stack branch of DoMprotect. Done here. On macOS both mprotect calls of that branch, including the retry after MapUnmappedPages, now go through GuestWriteWatchProtect. This conflicts with feat(libkernel): direct memory mappings on macOS hosts #7631 on those lines, and the body says how to resolve it: keep feat(libkernel): direct memory mappings on macOS hosts #7631's host cases, with the macOS call going through the watch.

I also rebased the branch onto #10889's current head 04638bf776, since krishalivaibhav changed its LockHostPages check. Thanks @krishalivaibhav for the report: this is the hunk you resolved by hand, and it now applies as is. The branch still merges cleanly with main.

Tested on the same tree as before. That tree carries #7631's version of the stack branch, so there I routed its single macOS mprotect through GuestWriteWatchProtect the same way: ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6 (674 tests) gives 13 failures, all of them tests that also fail without this PR. The focused set of 17 passes 3 runs out of 3.

@maximilianfv maximilianfv 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.

Requesting changes, after my 13:14Z approval, because of the new commit 32f4ad23d1: its stack branch clears errno on success on macOS (item 1, reproduced) and keeps a Linux-only retry that does not compile on macOS (item 2). The macOS A/B is clean, and the rest of the update resolves my 13:14Z review.

Re-review of 32f4ad23d1 against my review at 7729b6d75d. git range-diff shows that commits 1 to 5 are patch-identical after the rebase onto #10889's 04638bf776. Only 32f4ad23d1 is new.

Earlier point Status at 32f4ad23d1
1. libkernel.md extent wording, gap first Fixed (libkernel.md:246)
2. macOS munlockall wire semantics Fixed in the new "Locked memory on macOS" entry (:247), including wires from other host code, which also answers my question 5
3. #7201 overlap list Fixed. All four shared parts named in the body appear in gh pr diff 7201
4. Stack branch through the write watch Done, but see items 1 and 2. My premise there was also wrong (item 2)

Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)

Applied at 32f4ad23d1 to mugurc's macOS series (27e968974) merged with main at 2742004ed2, with other open macOS fixes, then toggled only this PR. The setup matches my earlier reviews:

  • Without: the series with the code hunks of 7d41eab9a5 reversed, plus main's GuestKernelStackProtect.cpp and WriteWatch.cpp.
  • Old head: the 11 files at 7729b6d75d.
  • New head: the old head plus 32f4ad23d1. The tree carries the series' version of the stack branch, which is #7631's split: Linux keeps the retry, and other hosts make one mprotect. So, like you, I routed that one macOS mprotect through GuestWriteWatchProtect. The ENOMEM retry as written at the head is not exercised here.

All three states keep the series' own mlock check in place of #10889's. Release build. The suite run is ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6, 674 tests.

Without this PR Old head 7729b6d75d New head 32f4ad23d1
Focused set (the 7 guest_ memory tests and the 10 write_watch_* tests) 12 of 17: guest_mlock, guest_credentials, guest_kernel_is_stack, guest_system_configuration and guest_kernel_stack_protect abort 17 of 17, 2 runs out of 2 17 of 17, 3 runs out of 3
Suite 46 fail 42 fail 42 fail, the same 42 as the old head
Probe: stack page registered with the watch by the probe, then mprotect R; watch record n/a 3 (stale) 1
Same page, mprotect NONE, then GuestWriteWatchCollect; host protection n/a 1 (readable) 0
Probe: errno set to 35, then a successful mprotect_nid_postfix on a guest thread stack page not run (plain mprotect) not run (same plain mprotect) 0, 3 runs out of 3 (item 1)
Same probe on a flexible memory page not run not run 35

Without the PR, the suite fails the 5 tests above. guest_refused_connect failed in both head runs and passed in the "without" run. Run alone, it passed 1 of 8 without this PR and 3 of 8 with it in my first review, and 0 of 6 and 1 of 6 in my last one. It uses none of the changed code. All other failures are the same in all three states.

Not built on Linux or Windows.

Findings

  1. core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp:1340: a successful mprotect on a guest thread stack sets errno to 0 on macOS.

    • errno = error; runs when error is 0 too.
    • The guest's errno is the host's: __error_nid_postfix returns &errno (core/libs/prx/libc/src/Process.cpp:239-241).
    • With errno set to 35, a successful mprotect_nid_postfix on a stack page leaves 0, while on a flexible memory page it leaves 35.
    • POSIX says no function sets errno to 0, and FreeBSD's mprotect does not touch it on success.
    • Fix: if (error != 0) errno = error;.
  2. DirectMemory.cpp:1336-1351: the macOS branch keeps a retry through MapUnmappedPages, which exists only on Linux (:180-204).

    • Nothing else declares it, so once the file builds on macOS (with #7631's include block), this branch does not compile there. My tree used #7631's split instead (see Tested), so it never compiled this code.
    • The call itself is not new: main's non-Windows branch already makes it, and on main the file does not build on macOS at all.
    • #7631 resolves it by splitting the branch: Linux keeps the retry, and macOS makes one call.
    • Your body proposes the same resolution, but the head's lambda keeps the retry on macOS. That is my fault: my item 4 asked for "both mprotect calls in this branch, including the retry". I was also wrong about the effect. macOS guest threads run on host pthread stacks (core/libs/prx/libkernel/Pthread/src/Thread.cpp:992-994 on main allocates the guest stack under #if defined(__linux__) only), and #7631 does not change that. So nothing registers a macOS stack with the write watch, and this commit changes no watch state today. My probe had to register the page itself.
    • Suggestion, together with item 1: drop the lambda and use #7631's shape now:
      #elif defined(__APPLE__)
              const int error = GuestWriteWatch::GuestWriteWatchProtect_nid_postfix(insidePointer, insideBytes, nativeProtection);
              if (error != 0) errno = error;
              const bool protectedPages = error == 0;
      #else
              (main's Linux code, unchanged)
      #endif
      That leaves the Linux lines as on main and makes the #7631 conflict trivial. Also consider adding #7631 to Depends on, since this file's macOS code builds only with it, as GuestAllocations.cpp needs #10366.
  3. Nits, docs/dev/debt/libkernel.md:

    • :247 says "macOS has no munlockall". It exists and fails with ENOSYS, as your body says: on this Mac it returns -1 with errno 78 (ENOSYS). Suggested wording: "macOS's munlockall fails with ENOSYS".
    • :250, the mlockall, munlockall entry, still lists only Windows and Linux. A pointer to the new entry would help.
  4. Question, core/libs/prx/libc/src/GuestAllocations.cpp, GuestAllocationsGuestPages:

    • The image search matches from the unrounded segment start (GuestAllocations.cpp:496), while FindImageContaining rounds the start down to the host page (DirectMemory.cpp:1282).
    • An address in [hostPageStart(p_vaddr), p_vaddr) of a segment whose start is not host-aligned therefore gets bias 0 here but is found in the image by DoMprotect.
    • The relinker may align PT_LOAD starts so that this cannot happen. I did not check that, and this is not a blocker.

AI-assisted: yes (Claude Code applied the PR, ran the tests, read the code and wrote this review).

@imellag

imellag commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@maximilianfv Thanks, both findings are right. New head f8fac50a31:

  1. errno on success. On macOS the stack branch now sets errno only when GuestWriteWatchProtect fails.
  2. MapUnmappedPages on macOS. I dropped the lambda and used your shape: #elif defined(__APPLE__) makes one GuestWriteWatchProtect call, and the #else branch is main's Linux code, unchanged. That is feat(libkernel): direct memory mappings on macOS hosts #7631's split, so whichever lands second keeps feat(libkernel): direct memory mappings on macOS hosts #7631's Linux branch and this macOS branch. feat(libkernel): direct memory mappings on macOS hosts #7631 is now in Depends on, since DirectMemory.cpp builds on macOS only with its include block. Thanks for pointing out that macOS guest threads run on host pthread stacks, so nothing registers them with the watch today. The body no longer claims more than the call itself.
  3. libkernel.md. The new entry now says that macOS's munlockall fails with ENOSYS. The mlockall, munlockall entry points to it for macOS.
  4. GuestAllocationsGuestPages. I did not check how the relinker aligns PT_LOAD starts either. Rather than rely on it, the image search now rounds the segment start down to the host page, as FindImageContaining does. An address in [hostPageStart(p_vaddr), p_vaddr) now gets the image's bias in both places.

Tested on the same M4 Pro tree as before (#7631's split, now with these two code changes):

  • The focused set of 17 passes 3 runs out of 3.
  • ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6 (674 tests): 14 failures. The same 14 fail with the two code changes reverted, and none of them uses the changed code. The tree has gained other changes since my last run, which is why there are 14 failures instead of 13.
  • I did not rerun your errno probe; the fix is the line you suggested.

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

@maximilianfv maximilianfv 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.

Approving f8fac50a31. It resolves all four items of my review at 32f4ad23d1, the macOS A/B is clean, and nothing fails only with this PR. It needs a rebase onto main first: there is a conflict in docs/dev/debt/libkernel.md (see below).

Earlier item Status at f8fac50a31
1. errno set to 0 on a successful stack mprotect Fixed (DirectMemory.cpp:1336-1339, if (error != 0) errno = error;), and checked with the probe below
2. Linux-only MapUnmappedPages retry on macOS Fixed. macOS makes one GuestWriteWatchProtect call, and the Windows and Linux lines are byte-for-byte main's
3. libkernel.md nits Fixed (:247 says munlockall fails with ENOSYS, and :250 points to the macOS entry)
4. GuestAllocationsGuestPages start rounding Fixed (GuestAllocations.cpp:496-498). It now rounds like FindImageContaining and the registry's collectImagePages. DoMprotect is its only caller

Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)

Applied at f8fac50a31 to mugurc's macOS series (27e968974) merged with main at 2742004ed2, with other open macOS fixes, then toggled only this PR. The states are the same as in my earlier reviews:

  • Without: the series with the code hunks of 7d41eab9a5 reversed, plus main's GuestKernelStackProtect.cpp and WriteWatch.cpp.
  • New head: the 11 files at f8fac50a31. The tree carries #7631's split of the stack branch, so the three lines of your __APPLE__ block went into that split's #else branch. On macOS the compiled code is the same.

Release build. The suite run is ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6, 674 tests.

Without this PR Previous head 32f4ad23d1 (my last review) New head f8fac50a31
Focused set (the 7 guest_ memory tests and the 10 write_watch_* tests) 12 of 17, 2 runs out of 2: guest_mlock, guest_credentials, guest_kernel_is_stack, guest_system_configuration and guest_kernel_stack_protect abort 17 of 17, 3 runs out of 3 17 of 17, 3 runs out of 3
Suite 47 fail 42 fail 42 fail, the same 42 as at 32f4ad23d1
Probe: errno set to 35, then a successful mprotect_nid_postfix on a guest thread stack page (R, then RW) 35, 35 0, 0 35, 35 (3 runs out of 3)
Probe: stack page registered with the write watch by the probe, then mprotect R, then NONE plus GuestWriteWatchCollect; watch record, then host protection n/a 1, then 0 1, then 0 (3 runs out of 3)

The 47 failures without this PR are the 42 plus the five aborts above. guest_refused_connect failed in both states this time. As in my earlier reviews, it uses none of the changed code.

I also checked one of the automated findings with a probe: sceKernelMunlockall over a locked 64 KiB range whose second 16 KiB was released with sceKernelReleaseDirectMemory. The pages before and after the hole ended with 0 wires (3 runs out of 3), so the per-range munlock does not stop at the hole.

Not built on Linux or Windows.

Before merging

  1. Conflict with main in docs/dev/debt/libkernel.md:250.
    • main's 9e6bd365f6 rewrote the mlockall, munlockall entry and dropped "On main, XPSemu (PPSA97358) fails to load at these imports (mlockall #4386, munlockall)".
    • main also rewrote the socket entry on the next line.
    • Fix: take main's line and add only your "macOS is under Locked memory on macOS".
    • Until then, the "Based on current main" box is out of date.
  2. Nit, for the #7631 merge. #7631's split has a __linux__ branch and an #else branch for the other hosts. This PR has __APPLE__ and then #else (main's code). So whichever PR lands second needs four branches: Windows, __APPLE__, __linux__ and #else, not only "#7631's Linux branch and this macOS branch".

A second automated read raised no new defects in the code this commit touches. Its other notes are about design and performance in code that has not changed since my 13:14Z approval, and I am not raising them here.

AI-assisted: yes (Claude Code applied the PR, ran the tests, read the code and wrote this review).

@imellag

imellag commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@maximilianfv Thanks. Rebased onto main 14c99b52ac. The mlockall, munlockall entry is now main's text plus "macOS is under Locked memory on macOS", and the socket line is main's. No code changed: git range-diff against f8fac50a31 shows only those debt lines. On the #7631 nit, agreed: I've reworded Depends on so whichever lands second carries four branches.

@maximilianfv maximilianfv 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.

Approving 119e879525. It is the rebase my approval of f8fac50a31 asked for, and no code changed.

Rebase check. I ran git range-diff e7e3e9c198..f8fac50a31 14c99b52ac..119e879525, comparing each series against its own merge-base.

  • Commits 1, 3, 4 and 6 are unchanged.
  • Commits 2, 5, 7 and 8 differ only in docs/dev/debt/libkernel.md context lines that main rewrote (connect, mlockall, munlockall, socket).
  • The net diffs of the 9 code files are byte-for-byte the same at both heads.
  • The PR's own debt lines are unchanged except the mlockall, munlockall entry. That entry is now main's line plus "; macOS is under Locked memory on macOS", which resolves the conflict from my last review.
  • The head also merges cleanly with today's main (aec3b38800).

Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)

The tree is the same as in my earlier reviews: mugurc's macOS series (27e968974) merged with main at 2742004ed2, with other open macOS fixes. It is a Release build. The suite is ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6, which runs 674 tests.

f8fac50a31 (my last review) 119e879525
Focused set (the 7 guest_ memory tests and the 10 write_watch_* tests) 17 of 17, 3 runs out of 3 17 of 17, 3 runs out of 3
Suite 42 fail 38 fail, all among the earlier 42

The four tests that passed this time are guest_refused_connect and the three guest_job_manager_worker_spin timing tests. None of them uses the changed code.

A correction to my earlier runs: the tree still had an older version of #10889's LockHostPages region check. The difference was != KERN_SUCCESS returning ENOMEM and protection == VM_PROT_NONE. This run uses the check as it is in this PR (KERN_INVALID_ADDRESS returns ENOMEM, another failure throws, and a page must be readable or executable), so guest_mlock now passes with this PR's actual code.

Not built on Linux or Windows.

Optional body nits:

  • "It sits on #10889's head 04638bf776": the first commit is now its rebased copy 7d11b0309d, with the same patch.
  • Tested still calls f8fac50a31 the current head and does not mention the rebase.

AI-assisted: yes (Claude Code checked the range-diff, rebuilt and ran the tests, had the debt text read independently, and wrote this review).

@imellag

imellag commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

@maximilianfv Thanks for the rebase check. Both body nits are fixed: the first line now names the rebased copy 7d11b0309d, and Tested says that 119e879525 is the rebase of f8fac50a31, with your range-diff result and your rerun at this head.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 27, 2026
@adriadam10 adriadam10 self-assigned this Oct 27, 2026
imellag and others added 7 commits October 27, 2026 08:16
- mprotect: the guest rounds a range to its 16 KiB pages. A Mach-O image
  is loaded at a multiple of the host page size only (4 KiB under
  Rosetta), so the pages are counted from where the ELF address 0 of the
  image holding the range was loaded, and a rounded edge smaller than a
  guest page that lies in the gap between two segments, where no host page
  exists, is left out. The write watch is told the new protection.
- sceKernelVirtualQuery reads the region with mach_vm_region.
- sceKernelMunlockall unlocks the ranges libkernel locked, one by one:
  macOS has no munlockall, which fails there with ENOSYS and would
  overwrite errno.
- The stack protect test reads page protections with mach_vm_region.

This is mugurc's f66a4df from his macOS series, without the mlock
check that boykopovar#10889 adds.

Co-authored-by: mugurc <mceker@gmail.com>
…on macOS

The main stack checks run the test again in a child through
/proc/self/exe, which macOS does not have, so the child exited with 127
and the check failed before testing anything. On macOS the child now
runs the path _NSGetExecutablePath reports.
… on Linux

The previous commit ran the main stack checks on macOS through
_NSGetExecutablePath, but on macOS every child skips them: the stack is
mapped within 1 MiB of its bottom and its limit cannot be raised. They
are now built and run on Linux only, as in mugurc's macOS series, and
other hosts run the thread stack check alone.
- mlock on macOS wires only the pages libkernel has not wired yet. XNU
  counts each wire of a page, and the locked ranges merge overlapping
  locks, so a page locked twice stayed wired after munlock and
  munlockall. The mlock test checks the wire count on macOS.
- mprotect changes a watched page through GuestWriteWatchProtect, which
  applies the host protection and records it under the write watch's
  lock, so a collection on another thread cannot run between the two.
  The write watch test covers it.
- sceKernelVirtualQuery takes a watched page's write permission from the
  write watch for memory outside the guest allocation registry.
- GuestAllocationsGuestPages counts an image segment up to the end of
  its last host page, as the image page collection does.
- The macOS includes stay in main's include block, so the host code
  other pull requests add there still sees them; the unused Mach-O
  headers and the end trim, which never changed the protected spans,
  are gone, and the guest page size is passed in.
- The libc and libkernel debt entries describe what is wired now.
- mlock on macOS skipped every page the locked ranges recorded, but a
  mapping that replaces a locked range in place leaves the record behind
  without wires, so a later mlock returned 0 and wired nothing. Within a
  recorded range it now wires again the host regions whose wire count is
  0. The mlock test locks, remaps the range fixed and locks again.
- munlock and munlockall on macOS hold the locked-range lock across the
  host unwire and the record update, so a concurrent mlock cannot see a
  range as wired while it is being unwired.
- sceKernelVirtualQuery limits the host region it reports to the write
  watch run that holds the address, so the permissions it reports hold
  for the whole extent.
- The libc debt entry says what the mapping paths do: they register the
  range again, and GuestWriteWatchProtectionChanged has no caller yet.
@adriadam10
adriadam10 force-pushed the feat/libkernel-memory-protection-macos branch from 119e879 to 896ae02 Compare October 27, 2026 07:24
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 27, 2026
@imellag

imellag commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

@adriadam10 Thanks for the rebase after #10889. I checked it with git range-diff: the #10889 commit is gone and the other seven only differ in context lines of docs/dev/debt/libkernel.md. I updated Depends on and the description to match.

@imellag imellag mentioned this pull request Oct 27, 2026

@maximilianfv maximilianfv 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.

Approving 896ae02ab5. It is 119e879525, which I approved yesterday, rebased onto main 81267b2204 with the first commit dropped now that #10889 is merged. The code is unchanged.

Rebase check. I ran git range-diff 14c99b52ac..119e879525 81267b2204..896ae02ab5.

  • The dropped commit 7d11b0309d makes the same change as #10889's merge 589799aa53, so the LockHostPages region check and the Mach includes now come from main.
  • Commits 1-3, 6 and 7 are unchanged. Commits 4 and 5 differ only in a context line of docs/dev/debt/libc.md: the entry above the write-watch bullet is now main's "Exception runtime on macOS".
  • Leaving out the docs and the dropped commit, the diff of the 9 code files matches the old one line for line. Only the hunk offsets moved.
  • mlockall, munlockall in libkernel.md is still main's current line plus "; macOS is under Locked memory on macOS". The two new libkernel entries do not repeat anything main documents.
  • I read the PR next to the main changes to the same files since 14c99b52ac (#10366, #10528, #11173, #11190). None of them touches the PR's hunks, and I found no interaction.

Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)

The tree is the same as in my earlier reviews: mugurc's macOS series (27e968974) merged with main at 2742004ed2, with other open macOS fixes, in a Release build.

  • The tree has no main after 2742004ed2, so I added #10889's merged check to LockHostPages by hand.
  • In the 9 code files this PR touches, the tree then has every line the PR adds.
  • Against 896ae02ab5, those files lack only main changes this PR does not touch: the TSAN guards in GuestMlock.cpp, sceKernelMapNamedSystemFlexibleMemory, and main's later changes to GuestAllocations.cpp and DirectMemory.cpp.
  • The suite is ctest -R '^(guest_|write_watch)' -E 'audio_in|voice' -j6, which runs 674 tests.

The reference arm is the same tree with an older version of this PR (the 32f4ad23d1 code) and #10889's earlier check. That is not "without this PR". The earlier rounds measured that: 12 of 17 without, 17 of 17 with.

Reference (older version of this PR) 896ae02ab5
Focused set (the 7 guest_ memory tests and the 10 write_watch_* tests) 17 of 17 17 of 17, 3 runs out of 3
Suite 22 fail, 25 skipped 18 fail, all among the earlier 22; the same 25 skipped

The four tests that passed only with this PR are timing checks that do not use the changed code. The three guest_job_manager_worker_spin tests also varied between runs in earlier rounds. guest_avplayer fails its 10 ms EOF stop latency threshold (median 17.7 ms) on the reference tree in all 5 runs and passed once with this PR.

Not built on Linux or Windows.

Optional body nits:

  1. Tested still says "The current head 119e879525". The check_conventions.py and pr_overlap.py results are also from that head.
  2. Two open PRs touch the same lines and are not in the overlap list:
    • #11042 rewrites the same mlockall, munlockall line in libkernel.md and also edits Export.cpp and GuestMlock.cpp.
    • #11577 edits GuestMlock.cpp next to this PR's hunks.

AI-assisted: yes (Claude Code checked the range-diff, rebuilt and ran the tests, had the code and debt text read independently, and wrote this review).

@adriadam10
adriadam10 merged commit 4a60059 into boykopovar:main Oct 27, 2026
21 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Re-checked after the rebase onto #10889: the range-diff shows only context changes in docs/dev/debt/libkernel.md, no logic changes. macOS behaviour was verified by @maximilianfv (17/17 focused tests) and the write-watch macOS jobs are green; merged.

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.

5 participants