Repository navigation
fix(libkernel_sys): fail fpathconf on a kqueue or shm descriptor with EOPNOTSUPP - #11296
adriadam10 merged 4 commits into
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
The Linux regression evidence is useful and the added branch is reached after the existing FIFO/socket handling. One validation gap remains: the reported issue includes Windows behavior, but the only focused run listed is Linux, and the hosted Windows product job is skipped for these paths. Could you run guest_kernel_sys on Windows at the exact head and record the kqueue and anonymous shared-memory cases? That would exercise the Windows GuestFstat and descriptor implementations that the Linux run does not cover. I have not independently built this head.
|
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.
|
|
Windows run of head On Windows the new check fails 3/3 at line 555 (errno 22), with or without the change. I printed
The anonymous object on Windows is a AI-assisted: yes (Claude Code); results above were run on this machine. |
MarcZX10
left a comment
There was a problem hiding this comment.
The Windows follow-up exposed a failing case in the new behavior: SHM_ANON descriptors still return the UFS pathconf values, because Windows backs anonymous shared memory with a temporary file that is not present in sharedDescriptorAccess; IsSharedMemoryDescriptor_nid_no_patch therefore returns false and this new branch is skipped. That leaves the stated shared-memory behavior incomplete on Windows, and the added Windows test fails for the anonymous descriptor. Please cover the Windows anonymous shared-memory descriptor too (or narrow the claimed scope and test contract if it is intentionally unsupported), add a regression assertion, and rerun guest_kernel_sys on Windows and Linux. The existing kqueue and named-shm results do not cover this path.
|
Thanks for the Windows run and the breakdown — that's exactly right. The anonymous I've switched the test (52c8d67) from an anonymous descriptor to a named one, which you confirmed is recognised and returns The remaining item — anonymous descriptors not being recognised on Windows — is a gap in the shared-descriptor tracking itself ( AI-assisted: yes (Claude Code); the Linux build/test/coverage-check above was run on this machine. |
MarcZX10
left a comment
There was a problem hiding this comment.
Thanks for changing the regression to use a named shared-memory object. That is useful cross-platform coverage for the newly added shared-descriptor check, and the prior Windows report confirms that this named object returns EOPNOTSUPP.
The issue reproduction is specifically shm_open(SHM_ANON, ...), however. Your follow-up says Windows backs that anonymous object with a temporary file that is not registered in sharedDescriptorAccess, so IsSharedMemoryDescriptor_nid_no_patch still returns false and _fpathconf_nid_postfix still falls through to the UFS constants for the reported Windows case. Please either include and test recognition of the Windows anonymous descriptor on both platforms, or narrow this PR’s stated scope to kqueue plus named shared-memory objects and leave the original SHM_ANON report explicitly unresolved (for example, with a separate issue). I am not asking anyone to close this PR.
Please also refresh onto current main (35ac3402c80b152983e8aa11196835d3a16370bf); this head is 2 commits ahead and 258 behind, and both Export.cpp and GuestKernelSys.cpp changed upstream. The merge preview is clean, but the existing Linux/Windows test reports predate the latest commit and current base. Rerun guest_kernel_sys on Linux and Windows at the rebased exact head before merge.
MarcZX10
left a comment
There was a problem hiding this comment.
One concrete checklist mismatch remains: this PR still says Fixes #11291, but #11291 specifically reports Windows shm_open(SHM_ANON, ...), and the author confirms the current Windows code still returns the UFS values for that descriptor. Merging this checklist would auto-close an issue whose reported Windows case remains reproducible. Please either implement and test recognition of the Windows anonymous descriptor, or remove Fixes #11291 and clearly leave that case unresolved as the earlier review requested. I also refreshed sync: current upstream/main is 204c6747d0a1cbaf932def3dea0778810cc3935e; this head is 2 ahead / 261 behind. Export.cpp and GuestKernelSys.cpp changed since the base; merge preview is clean. Rebase and rerun guest_kernel_sys on Linux and Windows at the resulting exact head.
52c8d67 to
cfd6631
Compare
|
You're right on both points — thanks for the careful review. Scope / I filed #11372 for the Windows case. The mechanism there (from reading the source): the anonymous Rebase. Rebased onto current AI-assisted: yes (Claude Code); the Linux build/test/coverage-check was run on this machine. |
cfd6631 to
35220d2
Compare
|
Rebased onto current AI-assisted: yes |
|
@MarcZX10 this should be ready for another look when you have a moment — all three points from your reviews are resolved:
Also flagging that #11403 lists this one under AI-assisted: yes |
|
A Windows run of
|
MarcZX10
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 35220d20f9a4715c0e610ce30a2676e6b2397b63 after the scope/checklist and sync follow-ups. The PR now explicitly limits itself to kqueue and the shared-memory descriptors this helper recognizes, states that Windows SHM_ANON remains unresolved in #11372, and no longer claims to close #11291. The PcAsyncIo special case remains before the new descriptor rejection; the tests cover both fpathconf aliases for a kqueue and named shared memory and check EOPNOTSUPP (errno 45). The Windows runtime report at this head passes guest_kernel_sys 3/3 with a baseline-source control; the Linux report covers the focused path and its negative control. Current-main merge preview is clean; upstream changes since the branch do not edit the modified fpathconf logic or new test block. git diff --check and conventions pass. This approval is scoped to the stated partial behavior and does not claim the Windows anonymous-shm case is fixed.
|
Thanks @MarcZX10 for working through the scope and sync iterations, and @BunnyFl for the Windows run at the exact head AI-assisted: yes |
35220d2 to
c5f5df6
Compare
… EOPNOTSUPP _fpathconf_nid_postfix only treated FIFOs and sockets as descriptors without a vnode, so a kqueue or shm_open descriptor fell through to the UFS constants (LINK_MAX 32767, NAME_MAX 255, PIPE_BUF 512). FreeBSD 11.4 kern_fpathconf (kern_descrip.c:1411-1429) answers _PC_ASYNC_IO for every descriptor and returns EOPNOTSUPP for a no-vnode descriptor that is not a pipe or socket. Return EOPNOTSUPP for kqueue and shared-memory descriptors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An anonymous shm descriptor is recognised as shared memory only on Linux, where the memfd link name is matched; on Windows the anonymous %TEMP% object is not registered in the shared-descriptor table, so the descriptor is not recognised and fpathconf never reaches the EOPNOTSUPP branch. A named descriptor is recognised on both platforms, so the check now opens a named object and unlinks it, keeping the same branch coverage without a Linux-only assumption. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… can import them Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
c5f5df6 to
149ec09
Compare
|
Rebased onto current |
…and use the renamed kqueue check sceKernelJitCreateSharedMemory's size object and maximum protection live in libkernel's g_jitDescriptors, which the receive side of SCM_RIGHTS doesn't fill, so a received JIT descriptor failed sceKernelJitMapSharedMemory with EBADF (XairRrR's review). Passing one now throws, with the random device and directory cases; IsJitSharedMemory answers it. The merge of main brought boykopovar#11296's rename of IsKqueueDescriptor to IsKqueueDescriptor_nid_no_patch, which the kqueue check now calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
fpathconfon a kqueue orshm_opendescriptor returned the UFS constants (_PC_LINK_MAX32767,_PC_NAME_MAX255,_PC_PIPE_BUF512, …) because_fpathconf_nid_postfixonly treats FIFOs and sockets as descriptors without a vnode. FreeBSD 11.4kern_fpathconf(sys/kern/kern_descrip.c:1411-1429) answers_PC_ASYNC_IO(200112) for every descriptor and then, for a descriptor with no vnode that is not a pipe or socket, returns -1 withEOPNOTSUPP. This adds that case: a kqueue (IsKqueueDescriptor) or shared-memory (File::IsSharedMemoryDescriptor_nid_no_patch) descriptor now fails withEOPNOTSUPPfor every name except_PC_ASYNC_IO.Scope. This covers the descriptors that
IsSharedMemoryDescriptor_nid_no_patchrecognises: kqueue, and namedshm_openobjects on both Linux and Windows, plus anonymousshm_open(SHM_ANON, …)on Linux. It does not fix the Windows anonymousSHM_ANONcase reported in #11291: on Windows that descriptor is not registered as shared memory, soIsSharedMemoryDescriptor_nid_no_patchreturns false andfpathconfstill falls through to the UFS values. That is a separate shared-descriptor-recognition gap, tracked in #11372.Tested
Linux (Ubuntu 24.04, x86_64).
ctest --test-dir build -R '^guest_kernel_sys$'passes with the change and aborts without it (guest errno 22 instead of 45). Dropping only the shared-memory half of the new branch (keeping the kqueue half) makes the named-shm_opencase fail at the check, so both halves of the condition are covered. The GuestKernelSys fpathconf cases use a kqueue and a namedshm_opendescriptor (named recognition works on both platforms, per the Windows run reported in review). Also re-verified building and passing after a local rebase onto currentmain(4e43321).tools/check_conventions.py --base origin/mainandtools/pr_overlap.pyboth clean. A Windows rerun ofguest_kernel_syswould still be welcome from someone with a Windows build.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)shm_open; the WindowsSHM_ANONcase is tracked separately in libkernel: Windows anonymous shm_open (SHM_ANON) descriptors are not recognised as shared memory #11372)🤖 Generated with Claude Code