Repository navigation
fix(libkernel): fail chflags through a dangling Windows symbolic link with ENOENT - #9469
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the Windows-only lookup path and focused symlink tests. For reparse-point inputs the change follows the target through the existing guest stat helper, preserving ENOENT for a dangling target and checking directory type for trailing separators; the non-reparse path is unchanged. The debt note and limitations are explicit. Conventions, intake, eligibility, labels, and report pass; hosted Windows/Linux build and test jobs were skipped. The Windows 11 focused runs described in the PR are author-reported; I did not run them locally.
|
Full suite with this PR merged together with #9442, #9469, #9474 and #9487, against |
|
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.
The Windows-only path and focused symlink cases were reviewed previously, and the reported combined Windows run is useful. The tested commit and the current branch base are still 47 upstream commits behind current main (now 110a18b); the reported baseline is a6dcea9. Please rebase/update onto current main and rerun the focused guest_filesystem checks plus the relevant conventions and overlap checks on that base before treating the earlier results as current. I checked the overlap bot warnings: the listed Stdio.cpp PRs touch other areas, and #6044 does not list GuestFilesystem.cpp, so I found no direct conflict in those warnings. No local test was run for this follow-up.
313a0b5 to
9b277bd
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 |
9b277bd to
5dc1dc1
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.
5dc1dc1 to
0de1e4d
Compare
… with ENOENT std::filesystem::status does not follow a Windows symbolic link under MinGW, so the lookup chflags makes saw a link whose target is missing as an existing file and returned 0. A reparse point in the last component is now looked up through the target with the guest stat, as FreeBSD 11.4's namei follows it: a missing target gives ENOENT, and a trailing slash after a link to a file gives ENOTDIR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
0de1e4d to
a5c72e0
Compare
|
Checked |
What
On Windows,
chflagssucceeded on a symbolic link whose target is missing and faileddangling/withENOTDIR. FreeBSD 11.4'skern_chflagsatlooks the path up withFOLLOW, so both fail withENOENT.The cause is the lookup in
LookupError: under MinGW,std::filesystem::statusdoes not follow a Windows symbolic link and reports a dangling link as an existing regular file. When the last component (or the one before a trailing slash) is a reparse point, the lookup now goes through the target withFile::TryFillFileStat, the gueststat, which already follows links: a missing target givesENOENT, a link to a file followed by a slash givesENOTDIR, a link to a directory is accepted. Linux is unchanged;chflagsis the only caller ofLookupError.Measured on
mainwith a standalone probe, the same dangling link:stat,truncate,utimesandopenalready fail withENOENT;chflagsreturned 0.Tested
Windows 11 (10.0.26200), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release,
-DBUILD_TESTING=ON, onmain938812d (rebased; since #9109,chflags("dangling/")fails withENOTDIRfrom the trailing-slash check before this lookup runs, so that check was dropped from the test; FreeBSD givesENOENTthere), Developer Mode on so links can be created.guest_filesystem: new checks on a dangling link (ENOENT) and a link to a file (0,ENOTDIRwith a trailing slash,EOPNOTSUPPfor an unknown flag), then both links are removed withunlink. Onmainthe test fails at the first new check (chflagson the dangling link returns 0); with this PR it passes. Without link permission the link checks are skipped with a message.guest_kernel_sys: passes.main938812d 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, fix(libkernel): fail open with O_CREAT and O_EXCL on a Windows symbolic link #9496, fix(libc): rename onto or from a Windows directory symbolic link like a link #9661 and feat(libkernel_sys): implement settimeofday #9682 merged together, same machine: 2029 tests each; on both, the 3agc_driver_sample_order*tests fail andagc_driver_image_array_window_past_layer_limitis not built (its 64 KiBalignasis rejected by this MinGW toolchain). In the parallel run (ctest -j 12) one other test failed on the merged branch (guest_memoryin one run,guest_kqueuein another,agc_driver_shutdown_threads_queued_prebuildsin both); each passes alone, three runs out of three.python tools/check_conventions.py --base upstream/main: passes.std::filesystem::create_symlinkthere), a console, a title run.#9442 (
lchflags) is independent of this PR; its dangling-link check passes with or without it.Overlaps
tools/pr_overlap.py --branchon the rebased head lists no direct overlap; of the 12 pull requests it could not compare, none changesLookupErrororchflags. fix(kernel): safely copy guest path arguments #9423 edits the start ofchflags_nid_postfix, notLookupError.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