Repository navigation
fix(libkernel): map shared memory and files executable on Windows - #12339
Conversation
A guest mmap with PROT_EXEC and MAP_SHARED of a shared-memory object or a regular file threw on Windows. The section is now created from a handle reopened with GENERIC_EXECUTE, as PAGE_EXECUTE_READ or PAGE_EXECUTE_READWRITE, the owner ACE of named objects grants execute with read, and a writable shared mapping of a read-only descriptor fails with EACCES, as FreeBSD 11.4 shm_mmap and vn_mmap do. Fixes boykopovar#12337
|
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.
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.
|
|
Re-run on the current
The only full-suite difference is the new The selection |
|
One overlap follow-up: the refreshed overlap report also shows #12296 editing the same Windows named-shared-memory and regular-file-mapping debt entries. The production changes appear complementary, so this is a documentation/rebase coordination issue rather than duplicate functionality. Please coordinate merge order with #12296’s author and declare it under |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the Windows section-creation and view-protection path, including the ReOpenFile access mask, named-object owner ACE, shared read-only write rejection, and the new protection/access matrix. I do not see a defect in those paths in this diff. The Windows and Linux test results in the description are from 46fe6f24374a, while the submitted head is dbd826bc88a03afc62cd9df46881fa06ea1d6be2; the description explicitly says the current-main application was not rerun. Please run at least guest_shm_open and guest_mmap_executable on Windows at this exact head (and the focused executable-mapping test on Linux if available), then update Tested. I have not approved pending that refreshed evidence; hosted checks currently cover conventions/intake/labels only.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Correction to my review comment: I overlooked the author’s October 28 10:47 follow-up in this thread. It reports a Windows run on the exact current head dbd826bc8 (Windows 11, WinLibs GCC 15.2): guest_shm_open passes, guest_mmap_executable passes, and ctest -E agc passes 1686/1686; the control with only the PR tests also reproduces the failures on main. My statement that these results were only from the earlier 46fe6f commit was incorrect. The exact-head Windows test-evidence request is satisfied. I have not yet cleared the separate coordination point about the overlapping Windows-memory debt edits in #12296.
|
Thanks @MarcZX10. On #12296 (@JJenkx): the overlap is more than the debt text, so here is what I found and what I propose. Production overlap. Both PRs refuse a writable
The rest does not overlap:
Proposed order: #12296 first, since it was opened earlier and is narrower. Then I rebase this PR on it, drop the now-redundant Current Linux at this exact head
The production change is Windows-only, so this Linux run shows that the new and changed tests build and pass on Linux. It does not exercise the fix itself. |
…c-shared-mapping fix(libkernel): map shared memory and files executable on Windows
What
On Windows, a guest
mmapwithPROT_EXECandMAP_SHAREDof a shared-memory object or a regular file threw "an executable shared file mapping is not implemented on Windows" (#12337). FreeBSD 11.4 maps these calls, and so do the Linux and macOS branches ofDoMapFileBacked_nid_no_patch:uipc_shm.c878-879 gives execute to any descriptor opened for reading;vfs_vnops.cvn_mmapallows execute unless the file system is mountednoexec.Changes:
DirectMemory.cpp, Windows branch:ReOpenFileopens withGENERIC_READ | GENERIC_EXECUTE, plusGENERIC_WRITEfor a read-write descriptor. Its protection isPAGE_EXECUTE_READorPAGE_EXECUTE_READWRITE, as CreateFileMapping requires. The extra handle is closed once the section exists. Mappings without execute take the existing path.EACCES, asshm_mmapdoes (885-887). Before, shared-memory descriptors skipped the access check inPosixMemory.cpp.SharedMemory.cpp: the owner ACE of named objects grantsGXwithGR, so a readable named object can be reopened with execute.guest_mmap_executable(guest_memory_tests executable-mappings, Windows and Linux). It runs shared protections 0-7 against a regular file opened read-write, read-only and write-only, an anonymous shm object, a named shm object opened read-write and read-only, and a JIT descriptor. For each it runs code from executable mappings, writes through writable ones, and checks the errno of refusals.GuestShmOpen.cppnow runs on Windows. Its last line (apwritepast the end of an empty object returning 0) stays off Windows: it fails there independently of this change.docs/dev/debt/libkernel.md: the Windows "noPROT_EXEC" gap is removed. What still differs is stated:fchmod;EACCESwhere FreeBSD maps it.Tested
At this head (
dbd826bc8, onmain3a40945e6), details:guest_shm_open,guest_mmap_executable0xc0000409)ctest -E agcmain: 1685/1685)guest_mmap_executableguest_shm_open,guest_mmap_executablectest -E agc-j16; the 14 failures (12 of them timeouts) all pass rerun one at a timeEarlier, on commit
46fe6f24374a:guest_shm_open,guest_mmap_executable0xc0000409)ctest -E "^agc"ctest -E "^agc"ReOpenFilepath makes both tests fail;EACCEScheck makesguest_mmap_executablefail on the errno check;tools/check_conventions.py --base origin/main: 0 errors. Twodebt-lengthwarnings, on entries that were already over 1000 characters onmain(1752 and 2861; now 1843 and 3214).Not tested:
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)EACCEScheck inDoMapFileBacked_nid_no_patch)