Repository navigation
feat(libkernel_sys): implement chown, lchown, fchownat and linkat - #9103
Conversation
|
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.
|
|
Independent Windows run of this PR (data only). Environment: Windows 10 Pro 10.0.19045, Ryzen 7 5800X3D, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release, BUILD_TESTING=ON. Merge-tree of the PR head against PR head tested: Results (3 runs each):
GPU: the run output has no Not run: other platforms, the other tests in the same family, the full repository CTest, and any title or console run. AI-assisted (Claude Code). |
|
Windows run of 3c7427d merged onto
Both causes are the same MinGW limitation: libstdc++ in MinGW-w64 GCC 15.2 does not see Windows symbolic links. A probe built with the same compiler shows a
#ifdef _WIN32
const auto attributes = ResolvePathError_nid_no_patch(path1, source) == 0 ? GetFileAttributesW(source.wstring().c_str()) : INVALID_FILE_ATTRIBUTES;
if (attributes != INVALID_FILE_ATTRIBUTES && (attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0)
#else
if (ResolvePathError_nid_no_patch(path1, source) == 0 && std::filesystem::is_symlink(std::filesystem::symlink_status(source, error)))
#endif
Not run: a fix for |
|
The owner check runs before the path lookup, so a lookup error turns into a throw when the owner or group is not 0 or -1. FreeBSD 11.4 does it the other way round: Probe on 3c7427d, Linux (Ubuntu 24.04, GCC 13.3, Release): temporary lines in
Calling int APS5_VABI chown_nid_postfix(const char* path, std::uint32_t uid, std::uint32_t gid) {
FileStat status{};
if (stat_nid_postfix(path, &status) != 0) return -1;
RequireGuestOwner("chown", uid, gid);
return 0;
}The same goes for Not run: Windows, a title. AI-assisted (Claude Code). |
|
Thanks for the Windows run and the two probes. Both are fixed in 2c5d550:
Tested on Windows 11 Pro 10.0.26300 with MinGW-w64 GCC 15.2.0 (WinLibs ucrt-posix-seh r7), Release, |
|
Thanks for both fixes. The separate PR for |
|
Thanks for clarifying. I'll wait to see which PR lands first and adjust mine accordingly to avoid redundant changes. |
|
#9442 adds |
2c5d550 to
6650443
Compare
|
@HQasper this PR now conflicts with Could you merge current If that is easier for you, I have the finished merge ready (parents: your head |
All four exports threw. The ownership calls follow fchown: owner and group 0 and the unchanged value are accepted and any other value throws. The path is looked up first with stat, lstat or fstatat, as FreeBSD 11's kern_fchownat does, so a lookup error is returned before the owner is checked. fchownat rejects flags other than AT_SYMLINK_NOFOLLOW with EINVAL. linkat passes AT_FDCWD and absolute paths to link and rejects flags other than AT_SYMLINK_FOLLOW with EINVAL; a relative path with a directory descriptor and a symbolic link source without AT_SYMLINK_FOLLOW throw. On Windows a symbolic link source is detected with the file attributes, since libstdc++ there reports it as a regular file. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
fef3938 to
705f6e1
Compare
|
@MarcZX10 @Gruzzberry @oneandonlydean Since #9336 makes I've also turned off the fork's branch rules, so maintainers can push to this branch from now on. |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the implementation, test changes, debt notes, and current checks. The ownership calls perform path or descriptor lookup before validating unsupported owner values, preserving the intended lookup-error precedence; fchownat also validates its supported flag mask before forwarding. The linkat implementation clearly bounds its supported cases and throws for the two unsupported cases instead of silently claiming success. The added cases cover null and empty paths, lookup errors, flag errors, unchanged and guest ownership, symlinks, descriptor-relative paths, and hard-link results on both reported platforms. The PR reports focused Linux and Windows test runs; current hosted product test jobs are skipped, while conventions, intake, and labels pass. I found no blocking issue in the reviewed changes.
What
chown,lchown,fchownatandlinkatin libkernel_sys were throwing stubs. This implements them on top of the calls libkernel_sys already has.chown,lchownandfchownatfollowfchown: ownership is not modelled, so owner and group 0 and the unchanged value (-1) are accepted, and any other owner or group throws throughNotImplemented_nid_no_patch, naming the call.fchownis unchanged.stat(chown),lstat(lchown) orfstatat(fchownat), and the owner is checked afterwards, as in FreeBSD 11'skern_fchownat. So a null, empty, missing or not-a-directory path or a bad descriptor fails as those calls do, also with a foreign owner, and nothing is changed on the host.fchownatflags other thanAT_SYMLINK_NOFOLLOW(0x200) returnEINVAL, as in FreeBSD 11'skern_fchownat.linkatpassesAT_FDCWD(-100) and absolute paths tolink, and flags other thanAT_SYMLINK_FOLLOW(0x400) returnEINVAL, as in FreeBSD 11'ssys_linkat. A relative path with a directory descriptor throws. A symbolic link as the source withoutAT_SYMLINK_FOLLOWalso throws: FreeBSD then links the symbolic link itself, whilelinkfollows it. On Windows the source is checked forFILE_ATTRIBUTE_REPARSE_POINT, asfchmodatdoes, since libstdc++ there reports a symbolic link as a regular file. WithAT_SYMLINK_FOLLOWa symbolic link source goes tolink, which follows it on every host, on Windows since fix(libkernel_sys): follow a symbolic link in link on Windows #9336.libkernel_sys.prxexports the four NIDs and importsstatandlstatfromlibkernel.prx, which defines them.Tested
Linux (Ubuntu 24.04.5 container, GCC 13.3, Release,
BUILD_TESTING=ON):guest_kernel_syswith a new ownership and link case: passed. It checks owner and group 0 and -1 for all three ownership calls,fchownatthroughAT_FDCWDand a directory descriptor, with and withoutAT_SYMLINK_NOFOLLOW,ENOENT,ENOTDIR,EFAULTandEINVAL, and the throws with their messages. A foreign owner with a missing path, a null path, a not-a-directory path or a bad descriptor returns the lookup error, andfchownatwith a bad flag and a foreign owner returnsEINVAL. A dangling symbolic link failschownwithENOENTand passeslchown. Forlinkatit checks that a hard link is made (link count 2),EEXIST,ENOENT,EINVALforAT_SYMLINK_NOFOLLOW,EFAULT, and the two throws. WithAT_SYMLINK_FOLLOWon a symbolic link it checks that the new name is a hard link to the target (link count 3), and that it is not a symbolic link (on Windows: no reparse point attribute). The symbolic link part was checked to run by breaking one of its expectations once.patched_libraries: passed, and the patched exports and imports were checked withnm -D.python3 tools/check_conventions.py --base upstream/main: passed.guest_kernel_sys,guest_kernel_sys_thread_defaults,guest_kernel_sys_sdk_version,guest_kernel_sys_barrier,guest_kernel_sys_module_internalandpatched_librariespassed.-fsyntax-onlyonExport.cppandGuestKernelSys.cppat 705f6e1: passed.Windows 11 Pro 10.0.26300 (MinGW-w64 GCC 15.2.0 WinLibs
x86_64-ucrt-posix-sehr7, Release,BUILD_TESTING=ON, symbolic links allowed through Developer Mode, so the symbolic-link block runs):guest_kernel_sys,guest_kernel_sys_thread_defaults,guest_kernel_sys_sdk_version,guest_kernel_sys_barrierandguest_kernel_sys_module_internalpassed, with the symbolic-link block running. This covers the changed Windows case ofAT_SYMLINK_FOLLOWon a symbolic link, which now goes throughlinkas fix(libkernel_sys): follow a symbolic link in link on Windows #9336 does.guest_kernel_syspassed on 2c5d550. On 3c7427d it fails at line 136 (guest errno 2), as in a reviewer's Windows run; that run found the two issues fixed in 2c5d550.Note: #9008 and #9200 have merged and their neighbouring-stub conflicts are resolved in 705f6e1. Since #9336 merged, linkat on Windows passes a symbolic link source with AT_SYMLINK_FOLLOW to link, which now follows it, instead of throwing.
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)Duplicate and overlap check
Checked on 2026-10-23 against upstream main 0a0ed72 and the heads of all 1240 open PRs.
chown,lchown,fchownatorlinkat.Export.cpp,Export.Discovery.cpp,GuestKernelSys.cppor the libkernel_sys debt file, merging it after this PR gives no conflict that it does not already have with main.lchflags) now conflicts with main itself inExport.Discovery.cpp, after feat(libkernel_sys): implement lchmod #9200 removed the stub next to it. Itslchflagsstub is next to thelchownstub this PR removes, so the same file stays the only conflict there.fchownso a foreign owner returnsEPERMinstead of throwing. This PR keeps the throw thatfchownhas on main forchown,lchownandfchownat. If fix(libkernel_sys): return EPERM for unsupported fchown ownership #8457 merges first, the same change can follow here.