Skip to content

fix(libkernel): recognize Windows anonymous shared memory descriptors - #11403

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:marc/fix-libkernel-shm-anon-windows
Oct 28, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:marc/fix-libkernel-shm-anon-windows

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What

On Windows, anonymous shm_open(SHM_ANON, ...) creates a temporary file but did not register the descriptor as shared memory. fchflags therefore treated it like an ordinary file instead of returning EINVAL, and descriptor duplication/close tracking did not preserve its shared-memory classification.

Create anonymous shared-memory files under the private Windows shared-memory root, track them as shared-memory descriptors, and add regression coverage for fchflags, duplication, close, and descriptor reuse.

Tested

  • x86_64-w64-mingw32-g++ -std=c++20 -D_WIN32 -D_fflush_nolock=fflush -fsyntax-only ... SharedMemory.cpp — passed.
  • The same MinGW syntax check for GuestNamedShmWindows.cpp — passed.
  • git diff --check upstream/main...HEAD — passed.
  • tools/check_conventions.py --base upstream/main --head HEAD — passed.
  • guest_named_shm_windows was not run locally. The automatic PR Windows product job is skipped by path policy, and the documented manual workflow dispatch requires repository admin rights; there is no hosted Windows runtime result for this head.

Checklist

@MarcZX10
MarcZX10 force-pushed the marc/fix-libkernel-shm-anon-windows branch from b7dc292 to b87b825 Compare October 26, 2026 11:04
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (4b1a9a3b2f) after it advanced. The rebased head is b87b825f839a9ff9a1220fcdece93c916d730473; git diff --check, tools/check_conventions.py, and MinGW syntax checks for both changed C++ files pass again. Hosted checks from the initial head passed policy/intake/report; the relinker/product jobs were skipped by path policy. The updated head should trigger fresh checks.

@MarcZX10

MarcZX10 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor Author

One test-review follow-up: VerifyAnonymousDescriptorClassification() closes descriptor before duplicate, so a subsequent _open("NUL", ...) will likely reuse the lower-numbered descriptor first. Merely requiring reused == duplicate would therefore be wrong. To exercise cleanup for the descriptor number assigned to duplicate, open a filler NUL handle after closing descriptor (and verify it reuses descriptor); then close duplicate, open another NUL handle, and verify it reuses duplicate and is no longer classified as shared memory. Alternatively, explicitly arrange descriptor numbers another way.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Follow-up on 78c7984395: the test now forces and asserts reuse of both freed CRT descriptor numbers. git diff --check, tools/check_conventions.py --base upstream/main, and MinGW -fsyntax-only passed (using a compatibility macro for the existing _fflush_nolock declaration issue). I could not run the Windows runtime suite locally, and the documented manual workflow dispatch returned HTTP 403 (admin rights required); the new head's policy checks are running.

@github-actions

github-actions Bot commented Oct 26, 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
PR 9019 core/libs/tests/GuestNamedShmWindows.cpp L215-216

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 4630 docs/dev/debt/libkernel.md
PR 5021 core/libs/prx/libkernel/File/src/SharedMemory.cpp
docs/dev/debt/libkernel.md
PR 6149 docs/dev/debt/libkernel.md
PR 6936 docs/dev/debt/libkernel.md
PR 7088 docs/dev/debt/libkernel.md
PR 7105 docs/dev/debt/libkernel.md
PR 7121 docs/dev/debt/libkernel.md
PR 7135 docs/dev/debt/libkernel.md
PR 7149 docs/dev/debt/libkernel.md
PR 7192 docs/dev/debt/libkernel.md
PR 7447 docs/dev/debt/libkernel.md
PR 7464 docs/dev/debt/libkernel.md
PR 7509 docs/dev/debt/libkernel.md
PR 7804 docs/dev/debt/libkernel.md
PR 8605 docs/dev/debt/libkernel.md
PR 9156 docs/dev/debt/libkernel.md
PR 9242 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 9389 docs/dev/debt/libkernel.md
PR 9557 docs/dev/debt/libkernel.md
PR 9566 docs/dev/debt/libkernel.md
PR 10319 docs/dev/debt/libkernel.md
PR 10442 docs/dev/debt/libkernel.md
PR 10645 docs/dev/debt/libkernel.md
PR 11002 docs/dev/debt/libkernel.md
PR 11215 docs/dev/debt/libkernel.md
PR 11625 docs/dev/debt/libkernel.md
PR 11849 docs/dev/debt/libkernel.md
PR 11872 docs/dev/debt/libkernel.md
PR 11969 core/libs/prx/libkernel/File/src/SharedMemory.cpp
docs/dev/debt/libkernel.md
PR 12170 docs/dev/debt/libkernel.md
PR 12296 docs/dev/debt/libkernel.md

@github-actions github-actions Bot added libkernel docs fix error Closes an issue (Fixes #N) labels Oct 26, 2026

@rahul05ranjan rahul05ranjan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Traced the Windows anonymous path end to end; the fix is consistent on every side I checked.

  • Classification: IsHostSharedMemoryDescriptor accepts exactly <resolved temp>\AnyPS5\shm\<40 hex>.dat — no separator in the relative part, first 40 chars hex, .dat suffix (SharedMemory.cpp:110ff). CreateAnonymousFile now names files SHA1(random || pid || serial) in hex — 40 chars plus .dat, precisely the shape the validator accepts, so the tracked descriptor and the path check agree instead of the old anyps5-shm-... names that never validated.
  • The test's reinterpret_cast<const char*>(1) is the anonymous sentinel (anonymousObject, SharedMemory.cpp:161), so shm_open(1, ReadWrite, 0600) really exercises the anonymous route. And since shm_open_nid_postfix rejects O_RDONLY for the anonymous object before calling in, TrackSharedMemoryDescriptor(..., true)'s writable flag can't be wrong here.
  • Hardening vs. the old code: files no longer sit in the raw temp_directory_path() with a random_device-seeded, counter-suffixed name; they sit under AnyPS5\shm with an explicit private DACL (SYSTEM/administrators/owner full, no other ACEs), _O_CREAT | _O_EXCL against squatting, and the reparse-point check on the directories. That closes the predictable-name and inherited-ACL exposure the old placement had — with TMP redirected to a shared directory, other local accounts could previously read or pre-create the emulator's guest shared memory.

One gap worth a look: EnsurePrivateWindowsDirectory applies the private DACL only at creation and tolerates ERROR_ALREADY_EXISTS. A <temp>\AnyPS5 that already exists (older build, or anything under the same user) keeps whatever ACL it was created with — the code validates it's a directory and not a reparse point, but never its ACL. If the intent is "objects under this root are readable only by the owner", the existing-directory path would want a GetNamedSecurityInfo check or a repair step.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for checking the ACL behavior. I checked the existing-directory path in EnsurePrivateWindowsDirectory: after CreateDirectoryW returns ERROR_ALREADY_EXISTS, the function still validates the path and unconditionally calls SetNamedSecurityInfoW with the protected DACL built from the owner/SYSTEM/Administrators policy. That call is not limited to newly created directories, so an existing ordinary directory is reset to the private DACL; if applying it fails, the function throws and anonymous shared-memory creation stops. I don't think an additional ACL query is needed for the case you identified. If you see a path where that unconditional reset is bypassed, please point it out.

@ycbuzi

ycbuzi commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

@MarcZX10 The Windows runtime run the hosted job skipped: 78c7984395 merged onto main c4669c9ca2, then #11296 cfd663103a on top (both clean). Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release, plain ctest --repeat until-fail:3.

test result
guest_named_shm_windows passes, 3 of 3
guest_kernel_sys passes, 3 of 3
guest_named_shm_windows_short_temp, _long_short_temp skip: there are no 8.3 short names on this machine's drives

AI-assisted: yes (Claude Code)

@oneandonlydean oneandonlydean self-assigned this Oct 26, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 26, 2026
@MarcZX10
MarcZX10 force-pushed the marc/fix-libkernel-shm-anon-windows branch from 78c7984 to 7589525 Compare October 26, 2026 23:24
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 27, 2026
@thuanquang

Copy link
Copy Markdown
Contributor

@MarcZX10 four points on 7589525. I found them by reading the code; nothing was built or run.

1. mmap(MAP_PRIVATE) of an anonymous object now fails on Windows. Run shm_open(SHM_ANON, O_RDWR, 0600), ftruncate(fd, 0x4000), then mmap(..., PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0). On main the fd is untracked, so PosixMemory.cpp:209 takes the regular-file branch and returns a private copy (:219). Now that SharedMemory.cpp:670 tracks the fd, that branch is skipped, :230 passes GuestPrivate and DirectMemory.cpp:1166 throws ENOTSUP, so mmap fails with errno 45. FreeBSD 11.4 shm_mmap accepts this. Fix: send GuestPrivate shm descriptors to MapPrivateFileCopy too (named objects already fail this way on main), add a private case to the test, and update docs/dev/debt/libkernel.md:172 and :280.

2. fchmod on an anonymous object now fails with EACCES. fchmod(fd, 0644) now calls SetSecurityInfo(DACL) (SharedMemory.cpp:299). That needs WRITE_DAC on the handle, but _wsopen_s (:666-668) only grants read, write and delete. The call is denied, :547 sets EACCES, and Stdio.cpp:1669 returns it without a fallback. On main _wchmod succeeds, and FreeBSD shm_chmod succeeds for the owner. Named opens work only because :400 adds WRITE_DAC. Fix: open the anonymous file with CreateFileW(GENERIC_READ|GENERIC_WRITE|WRITE_DAC|DELETE, ..., FILE_FLAG_DELETE_ON_CLOSE) plus _open_osfhandle, and test fchmod on an anonymous fd.

3. Growing an anonymous object now writes zeros. ftruncate(fd, 1 << 30) now goes through _chsize_s (SharedMemory.cpp:1002). That writes 4 KiB zero blocks (262144 WriteFile calls for 1 GiB) while the object mutex is held. Main only sets the end of file (Stdio.cpp:163). The guest sees the same bytes, but the cost grows with the size. Consider FileEndOfFileInfo here too.

4. Overlap with #9019. The bot reported the test conflict, but there is more. #9019 bounds Windows anonymous writes with RememberDescriptor(..., true) and clamps in Stdio.cpp/Open.cpp. Here, tracking already bounds the same writes in SharedMemory.cpp:852-853 and 876-878. If both merge, every write is clamped twice. Please name the overlap and a merge order in the body. The new bounding is also untested on Windows: GuestShmOpen.cpp:50 is still inside #ifndef _WIN32.

AI-assisted: yes (Claude Code).

@ycbuzi

ycbuzi commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Windows rerun of the current head 758952517c (the patch changed since my run on 78c7984395), merged onto main 14332cd8d0 (3cbb651256, clean). Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release, BUILD_TESTING=ON.

  • Built guest_named_shm_windows_tests with no errors. The only warning is main's existing _sigintr_nid_postfix one in Signals.cpp.
  • guest_named_shm_windows passes both runs of ctest --repeat until-fail:2.
  • Negative control: with main's File/src/SharedMemory.cpp and the rest of this head, it fails with anonymous shared memory was not recognized (guest errno=2).
  • main's guest_named_shm_windows_short_temp and guest_named_shm_windows_long_short_temp exit with their skip code (77) on this machine, on both sides, so they didn't run.

AI-assisted: yes (Claude Code)

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 28, 2026
@MarcZX10
MarcZX10 force-pushed the marc/fix-libkernel-shm-anon-windows branch from 7589525 to 768b29f Compare October 28, 2026 13:05
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 28, 2026
@medazizktata

Copy link
Copy Markdown
Contributor

Windows run of this head (768b29f4e) merged onto main 319621ba2 (Windows 11 Pro, i5-12600KF, WinLibs GCC 15.2, Release): guest_named_shm_windows and its short_temp / long_short_temp variants pass 3/3.

One ordering issue: with anonymous objects now recognised as shared memory, MAP_PRIVATE of an anonymous object takes the shm path in mmap_nid_postfix, which on main skips MapPrivateFileCopy and reaches DoMapFileBacked_nid_no_patch, which refuses private mappings. Adding a private read-write mapping of an SHM_ANON object to guest_shm_open:

Each stage's build recompiled only that PR's file (SharedMemory.cpp, then PosixMemory.cpp). No test on main maps a shared-memory object privately, so nothing catches this today. Suggest Depends on: #12296 for this PR. I can open the private-mapping checks as a small test PR once #12296 is in.

@oneandonlydean
oneandonlydean merged commit d775fc5 into boykopovar:main Oct 28, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs fix error Closes an issue (Fixes #N) libkernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

libkernel: Windows anonymous shm_open (SHM_ANON) descriptors are not recognised as shared memory

6 participants