Skip to content

fix(libkernel): fail socketpair with EFAULT on an unusable result pointer - #13742

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
Gruzzberry:fix/socketpair-efault
Oct 31, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
Gruzzberry:fix/socketpair-efault

Conversation

@Gruzzberry

Copy link
Copy Markdown
Contributor

What

socketpair with a result pointer the guest cannot write crashed the host: a pointer such as 0x10 was accepted by the guest-arena write guard and the final store to sv faulted. FreeBSD 11.4's sys_socketpair runs kern_socketpair first, so every argument error keeps its source order, then copyouts the two descriptors; if the copyout fails it closes both with kern_close and returns EFAULT.

The pair is now created and installed as before, and the two numbers are written with GuestCopyOut_nid_no_patch. If that fails, both descriptors are closed and the call fails with EFAULT. A null pointer takes the same path (it used to be rejected before anything was created), so the lowest free descriptor numbers are the same afterwards, as on FreeBSD. Close-on-exec registration happens only after the copyout succeeds. Argument errors (family, type, protocol) still win over a bad pointer.

Tested

Windows 11 (10.0.26200), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release, -DBUILD_TESTING=ON, on current main.

  • guest_socket_pair: new checks for 0x10 with a stream and a datagram pair, a mapped page with no access, an sv that starts 4 bytes before such a page (second int unreadable), all giving -1 with EFAULT, and a check that a failed call with SOCK_CLOEXEC releases both numbers (the next pair gets the same numbers). On main the test crashes with a write fault at 0x10; with this change it passes, three runs out of three.
  • Full suite: the failures are pr_overlap_encoding, agc_host_import_cut_lifetime and agc_driver_depth_plane_partial_store, which fail on main in this environment too, plus vulkan_validation_gate_reports and check_compile_test_windows_gcc_response_text_mode_is_not_reinterpreted, which fail here because the Windows Store Python launcher path cannot be read (OSError on python.exe) and touch nothing in this change. guest_libc_cnd_timed_wait and guest_timed_wait_cpu failed in the parallel run under load and passed alone.
  • python tools/check_conventions.py --base upstream/main: passes.
  • Not tested: Linux and macOS, a console, a title run.

Overlaps

tools/pr_overlap.py found no confirmed overlap. It could not compare Udp.cpp and GuestSocketPair.cpp against #5021, #7088, #8605, #9713, #10645, #12682, #12688, #12700 and #12965 because those conflict with main themselves; I read their diffs and none changes socketpair_nid_postfix (#8605, #12682, #12688, #12700 and #12965 only call it in tests).

Checklist

  • Based on current main; no other open PR implements the same function
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); no silent stubs added
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source: none added
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

AI-assisted (Claude Code).

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 5021 core/libs/prx/libkernel/Socket/src/Udp.cpp
core/libs/tests/GuestSocketPair.cpp
PR 7088 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 8605 core/libs/prx/libkernel/Socket/src/Udp.cpp
core/libs/tests/GuestSocketPair.cpp
PR 9713 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 10645 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 12688 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 12700 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 12965 core/libs/prx/libkernel/Socket/src/Udp.cpp

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@Gruzzberry Reviewed at 53a9b2d3b9. The false-return copyout path closes both descriptors correctly. Please also protect the installed pair when GuestCopyOut_nid_no_patch throws. On Linux the helper explicitly throws std::runtime_error for process_vm_writev errors other than EFAULT; the new call is after both Install operations, and neither that exception nor the existing bad_alloc catch rolls them back. A bounded injected copyout exception leaves both entries installed. Use a cleanup guard for the installed descriptors until copyout and close-on-exec registration complete, then release it on success. Add a failure-injection check that confirms both numbers and socket ownership are released on the exceptional path. This can preserve the helper’s existing exception behavior.

@jotitadev jotitadev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linux run of head 53a9b2d3b9, merged (--no-commit) onto main 6a691f69ee. Ubuntu 26.04 (kernel 7.0), GCC 15.2.0, CMake 4.2.3, Ninja, Release, BUILD_TESTING=ON, i7-8700.

  • guest_socket_pair: passes 3/3.
  • Full build ok. Related subset guest_socket|guest_udp|guest_tcp|guest_poll|guest_net|guest_select|guest_kernel_fcntl: 35/35.
  • public_headers: 65/65.
  • Red: with Udp.cpp reverted to main and the new test kept, guest_socket_pair crashes (SEGFAULT) 3/3.
  • Udp.cpp stack: main + #13695 + #13738 + #13742 + #13752 + #13756 merged in that order (all touch Udp.cpp) merges cleanly; full build ok, the five focal tests (guest_socket_pair, guest_socket_options, guest_udp, guest_poll, guest_kernel_fcntl) pass 3/3, the socket subset 35/35 and public_headers 65/65.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

Gruzzberry and others added 2 commits October 31, 2026 11:31
…nter

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…or registration throws

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Gruzzberry
Gruzzberry force-pushed the fix/socketpair-efault branch from 53a9b2d to 0d2ae1a Compare October 31, 2026 11:43
@Gruzzberry

Copy link
Copy Markdown
Contributor Author

@oneandonlydean thanks, fixed in 0d2ae1a300 (rebased on current main).

  • A local InstalledPair guard now owns both installed descriptors from the second Install until the copyout and the close-on-exec registration have finished. Its destructor closes each number independently (an exception from one close cannot skip the other, and errno is restored), so a false return from GuestCopyOut_nid_no_patch (still EFAULT), a std::runtime_error from the helper (propagates unchanged) and a std::bad_alloc from the registry insert (still ENOMEM) all release both numbers and the sockets. It is released only on success.
  • The new check in GuestSocketPair.cpp replaces operator new to fail the first or second allocation of the std::set<int> node size, which are the two close-on-exec inserts. The call returns -1 with ENOMEM, the same two numbers are handed out again by the next socketpair, they are still open and valid, and a pair created afterwards without SOCK_CLOEXEC does not inherit the flag. This exercises the same rollback an exception from the copyout takes. I could not inject the Linux runtime_error itself, because that needs process_vm_writev to fail with something other than EFAULT.
  • The injection is under #ifndef _WIN32 because on Windows libkernel takes operator new from libstdc++-6.dll, so the executable cannot replace it. I could only run the Windows build here, so please read that block on the Linux job.

Tests: guest_socket_pair and guest_recvmsg_memory 3/3, full suite matches main apart from the known environment failures (vulkan_validation_gate_reports, response_text_mode).

@adriadam10 adriadam10 self-assigned this Oct 31, 2026
@adriadam10
adriadam10 merged commit c21eb9a into boykopovar:main Oct 31, 2026
11 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the copyout order against FreeBSD 11.4 sys_socketpair, the InstalledPair rollback (both numbers closed on a false copyout, a helper exception or a close-on-exec bad_alloc, errno kept, registry entries erased by Close) and that the new checks fail on main; Dean's request is met and jotitadev's Linux run covers the earlier head. merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants