Repository navigation
fix(libkernel): fail open with O_CREAT and O_EXCL on a Windows symbolic link - #9496
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the Windows reparse-point handling and the cross-platform regression cases. The check recognizes the link itself before the exclusive create can follow a dangling link to its target; O_CREAT without O_EXCL remains covered as a separate case. The guest_filesystem tests verify both the errno and guest error mapping, ensure no target is created on failure, and clean up the link and target. The PR checks for conventions, intake, eligibility, labels and report pass; hosted product build jobs are skipped.
|
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
Rechecked the current head and overlap state. The Windows reparse-point check runs before exclusive create can follow a dangling link; ordinary O_CREAT through that link remains covered. Regression checks verify POSIX and SCE errors, target non-creation, later ordinary creation, and cleanup. The author reports a Windows red/green run and a broader suite; hosted eligibility, conventions, intake, labels and report pass, while product/relinker jobs are skipped. The #5021/#6936/#7201 manual shared-file warnings were not found as concrete overlaps in the reviewed diff. I did not run local tests.
MarcZX10
left a comment
There was a problem hiding this comment.
The Windows reparse-point check and regression cases are well targeted: they verify EEXIST through both POSIX and SCE entry points, confirm the dangling target is not created, then cover ordinary O_CREAT through the link and cleanup. The earlier manual overlap review also found the #5021/#6936/#7201 warnings were distinct. However, the tests and branch are based on a6dcea9; upstream main is now ab347d1. Please rebase onto current main, rerun the focused guest_filesystem regression and conventions check, and refresh the overlap scan/manual uncomparable-file review. Hosted product jobs are skipped; I did not run local tests.
c47e915 to
fd2c87f
Compare
|
Rebased onto |
|
Holding this as a draft, following the direction on #5239: the open queue is above 700 and no named title needs this change on current |
…ic link FreeBSD 11.4's vn_open_cred does not follow the last component when O_EXCL is set, so O_CREAT|O_EXCL on a symbolic link fails with EEXIST whatever it points to. On Windows the host open followed a dangling link and created its target. A reparse point in the last component now fails O_CREAT|O_EXCL with EEXIST before the host open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fd2c87f to
3cd6a37
Compare
|
Rebased onto |
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
|
Checked the |
What
On Windows,
openwithO_CREAT|O_EXCLon a symbolic link whose target is missing succeeded and created the target. FreeBSD 11.4'svn_open_credonly addsFOLLOWwhenO_EXCLis not set, so the lookup finds the link itself and the call fails withEEXISTwhatever the link points to, as POSIX requires forO_CREAT|O_EXCLon a symbolic link.OpenNativenow failsO_CREAT|O_EXCLwithEEXISTbefore the host open when the last component is a reparse point. Any reparse point is an existing name, so the answer does not depend on its kind. A link to an existing file already failed withEEXISTand still does;O_CREATwithoutO_EXCLstill creates the target through a dangling link, as FreeBSD does. Linux already fails in the hostopenand is unchanged.Tested
Windows 11 (10.0.26200), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release,
-DBUILD_TESTING=ON, onmaind6eafa5 (rebased), Developer Mode on so links can be created.guest_filesystem: new checks with a dangling link:openwithO_CREAT|O_EXCL|O_WRONLYfails withEEXIST,sceKernelOpenreturns0x80020011, and no target appears;O_CREAT|O_WRONLYthen creates the target through the link;O_CREAT|O_EXCLfails withEEXISTagain. The link and target are removed afterwards; without link permission the checks are skipped with a message. Onmainthe test fails at the first new check (the open returns a descriptor); with this PR it passes.maind6eafa5 and onmainwith feat(libkernel_sys): implement lchflags #9442, fix(libkernel): fail chflags through a dangling Windows symbolic link with ENOENT #9469, fix(libc): follow a Windows symbolic link in access #9474, fix(libkernel): follow a Windows symbolic link in chmod #9487 and fix(libkernel): fail open with O_CREAT and O_EXCL on a Windows symbolic link #9496 merged together, same machine: 1890 tests each, identical results (1869 passed, 18 skipped; on both, the 3agc_driver_sample_order*tests fail andagc_driver_image_array_window_past_layer_limitis not built, as its 64 KiBalignasis rejected by this MinGW toolchain).python tools/check_conventions.py --base upstream/main: passes.std::filesystem::create_symlink), a console, a title run.Overlaps
tools/pr_overlap.py --branchon the rebased head lists no direct overlap. Of the 9 pull requests it could not compare, Macos port #7201 adds a macOS-onlyO_CREATcheck after theO_NOFOLLOWblock, below this one, and fix(libkernel): remove a Windows directory junction without emptying its target #7530 and fix(libkernel): treat Windows junctions in unlinkat like unlink and rmdir #9095 match only in a debt entry; none changes theO_CREAT|O_EXCLcheck.GuestFilesystem.cpptotests/files/; if it lands first, the new cases move with the file in the rebase.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); no silent stubs added.mdfiles or images added to the repositoryAI-assisted (Claude Code).
🤖 Generated with Claude Code