Repository navigation
fix(libkernel): fail fcntl record locks with EFAULT on an unreadable flock pointer - #15081
Gruzzberry wants to merge 1 commit into
Conversation
…flock pointer Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
BunnyFl
left a comment
There was a problem hiding this comment.
This matches FreeBSD 11.4's sys_fcntl, which does copyin of the struct flock (and the old struct __oflock) before kern_fcntl looks at the descriptor. So EFAULT now comes before the virtual-directory EINVAL and before a bad descriptor, as the new -1 case pins.
Build and test on Windows (WinLibs GCC, BUILD_TESTING=ON), PR merged into current main:
guest_kernel_fcntlpasses.- With
main'sDescriptorControl.cpprestored, it segfaults on the new0x10cases.
|
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.
|
ahmadmuj
left a comment
There was a problem hiding this comment.
Checked head 97da73eed49dac518ee6e06a8e2df7cd6771cb39, PR base 92d2fdc3438f83e39d624eb65f3195bf2701568c on Linux/WSL. Built the actual libkernel/libc modules; guest_kernel_fcntl passes.
A separate protected-page probe passes all 24 cases: 23/24/31/32 readable bytes, all six lock commands, descriptor -1. It checks exact -1 returns and guest EFAULT/EBADF, including the 24-byte old and 32-byte current cutoffs. This matches the copy-before-descriptor order in FreeBSD.
Forcing the old size to 32 breaks six cases; restoring raw memcpy crashes twelve. After restoring the source, all 24 pass again. The same probe crashes in twelve cases on main 0fbe14c15361c420c65bc22cf2c3dc14cbda81d1.
Looks good to me for input copying. This does not test result copyout, host locking, or console behavior.
What
fcntlwithF_GETLK,F_SETLK,F_SETLKWand the oldF_OGETLK,F_OSETLK,F_OSETLKWread the gueststruct flockwith a rawmemcpy. A guest passing an invalid non-null pointer (for example 0x10) crashed the host emulator with an access violation. FreeBSD 11.4'ssys_fcntlcopies the structure in beforekern_fcntllooks at the descriptor, so these commands fail withEFAULTfor an unreadable pointer, ahead ofEBADFand the other checks. The structure is now read withGuestCopyIn_nid_no_patch(24 bytes for the old commands, 32 for the new ones), and the virtual-directoryEINVALcheck moved after it. TheF_GETLKresult copy-out already failed withEFAULTand is unchanged.Tested
Windows 11 (10.0.26200), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release,
-DBUILD_TESTING=ON, on currentmain.guest_kernel_fcntl: new checks pass 0x10 to all six lock commands on a regular file and toSetLockon descriptor -1, expectingEFAULT. Onmainthe test crashes with a segmentation fault; with this change it passes, three runs out of three.pr_overlap_encoding,agc_host_import_cut_lifetimeandagc_driver_depth_plane_partial_storedid not fail here. The failures arevulkan_validation_gate_reportsandcheck_compile_test_windows_gcc_response_text_mode_is_not_reinterpreted(the WindowsApps python environment) and the twomain_sync_renames_test_*_rename_with_detection_disabledtests (a non-ASCII file name assertion in a git script, 3 of 3 failing alone, not present in the earlier baseline).python tools/check_conventions.py --base upstream/main: passes.mainand here: on non-Windows hostsF_GETLKinto readable but read-only guest memory is not caught by the result copy-out.Overlaps
RecordLockinDescriptorControl.cpp(feat(libkernel): answer tcgetpgrp and tcsetpgrp on a pipe with its SIGIO owner #9566 touches the virtual-directory check line this PR moves). Both are older, so they go first and this PR rebases.tools/pr_overlap.pycould not compareDescriptorControl.cppagainst feat(libkernel): report open, read and close notes on watched files #7121, feat(libkernel_sys): implement pwrite through the shared POSIX provider #9019, feat(libkernel): cancel blocked POSIX reads with owned operations #12682, fix(libkernel): preserve blocking sockets over nonblocking native transport #12688, feat(libkernel): cancel blocked POSIX writes with partial progress #12700 and feat(libkernel): support deferred cancellation in POSIX accept #12965, which conflict withmainthemselves. Their diffs do not touchRecordLock.Checklist
main; no other open PR implements the same functionNotImplemented_nid_no_patch); no silent stubs added.mdfiles or images added to the repositoryAI-assisted (Claude Code).
🤖 Generated with Claude Code