Skip to content

fix(libkernel): fail sendto and recvfrom with EFAULT on an unusable pointer - #13756

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
Gruzzberry:fix/recvfrom-sendto-efault
Oct 31, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
Gruzzberry:fix/recvfrom-sendto-efault

Conversation

@Gruzzberry

Copy link
Copy Markdown
Contributor

What

sendto and recvfrom on a guest socket crashed or returned a wrong errno when a guest pointer could not be used: a send buffer or destination address that is null, 0x10 or on an unreadable page, a receive buffer on an unwritable page, and an unreadable or unwritable address or address-length pointer. FreeBSD 11.4 fails these with EFAULT (sys_sendto/kern_sendit/getsockaddr, sys_recvfrom/kern_recvit/recvit). Native Windows recv with a bad pointer answered ECONNABORTED (53) here.

  • sendto copies the destination address into a local buffer (after the existing ENAMETOOLONG/EINVAL length checks, before the descriptor lookup, as getsockaddr does) and fails with EFAULT if it cannot be read. The send buffer is checked page by page right before the native send, after the existing EINVAL, address and port-zero checks.
  • recvfrom copies *addressLength in before the descriptor lookup (sys_recvfrom does the copyin before kern_recvit looks up the descriptor). An unwritable receive buffer is probed after the wait; if the socket would answer with its own error first (EAGAIN, ENOTCONN, a reset) that errno is returned, an empty stream or datagram returns 0, otherwise a datagram is dropped and the call fails with EFAULT, as soreceive does after dequeuing. The address and its length are written with the checked copy; as in recvit, the length is written back (the caller's value when no address buffer is given) and a failed write is EFAULT after the data is consumed.

Tested

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

  • guest_udp: new CheckPointerFaults for UDP and TCP on loopback. sendto with a null, 0x10, unreadable and page-crossing buffer, and (UDP) a 0x10, unreadable and page-crossing destination; a read-only page is a valid send buffer. recvfrom with a 0x10, unreadable, read-only and page-crossing address-length pointer, a 0x10, unreadable and read-only receive buffer, and a 0x10, unreadable and page-crossing address buffer (data consumed); an empty nonblocking socket with a bad buffer answers EAGAIN; with no address buffer the length comes back unchanged. On main the test crashes (segfault); with this change it passes, three runs out of three, together with the 95 socket, datagram, TCP and net tests.
  • Full suite: the only failures are pr_overlap_encoding, agc_host_import_cut_lifetime and agc_driver_depth_plane_partial_store (fail on main here too), plus vulkan_validation_gate_reports and check_compile_test_windows_gcc_response_text_mode_is_not_reinterpreted, which fail because the WindowsApps python.exe cannot be opened by those scripts.
  • python tools/check_conventions.py --base upstream/main: passes.
  • Not tested: Linux and macOS (the new code uses the existing copy helpers; the peek path is not Windows-only), a console, a title run.
  • Not fixed, same on main: the early EFAULT for a null buffer with a non-zero length and for an address buffer without an address-length pointer (fix(libkernel): accept an address buffer without a length pointer in recvfrom #7507 changes the second) still come before the descriptor state checks. The writability probe covers the whole requested length, not only the bytes that arrive, so a short message into a buffer whose tail is unmapped fails here where FreeBSD would succeed. A send with an unreadable buffer on an unconnected, shut-down or oversized-datagram socket answers EFAULT where FreeBSD answers EDESTADDRREQ, EPIPE, ENOTCONN or EMSGSIZE first, because a native send cannot be tried without side effects. For datagram sockets the buffer is checked before the destination address is validated, as sosend_dgram copies the data in before udp_output checks the address.

Overlaps

Checklist

AI-assisted (Claude Code).

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

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.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 11896 core/libs/prx/libkernel/Socket/src/Udp.cpp L1622-1628
PR 12682 core/libs/prx/libkernel/Socket/src/Udp.cpp L1606-1607, L1623-1624
PR 12970 docs/dev/debt/libkernel.md L353
PR 13582 docs/dev/debt/libkernel.md L353
PR 13587 docs/dev/debt/libkernel.md L353
PR 14128 docs/dev/debt/libkernel.md L353

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 4630 docs/dev/debt/libkernel.md
PR 5021 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 6936 docs/dev/debt/libkernel.md
PR 7088 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 7105 docs/dev/debt/libkernel.md
PR 7121 docs/dev/debt/libkernel.md
PR 7135 docs/dev/debt/libkernel.md
PR 7149 docs/dev/debt/libkernel.md
PR 7192 docs/dev/debt/libkernel.md
PR 7447 docs/dev/debt/libkernel.md
PR 7464 docs/dev/debt/libkernel.md
PR 7509 docs/dev/debt/libkernel.md
PR 7804 docs/dev/debt/libkernel.md
PR 8605 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 9156 docs/dev/debt/libkernel.md
PR 9199 docs/dev/debt/libkernel.md
PR 9242 docs/dev/debt/libkernel.md
PR 9293 docs/dev/debt/libkernel.md
PR 9297 docs/dev/debt/libkernel.md
PR 9302 docs/dev/debt/libkernel.md
PR 9326 docs/dev/debt/libkernel.md
PR 9389 docs/dev/debt/libkernel.md
PR 9557 docs/dev/debt/libkernel.md
PR 9566 docs/dev/debt/libkernel.md
PR 9713 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 9843 docs/dev/debt/libkernel.md
PR 9938 docs/dev/debt/libkernel.md
PR 10025 docs/dev/debt/libkernel.md
PR 10442 docs/dev/debt/libkernel.md
PR 10558 docs/dev/debt/libkernel.md
PR 10645 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 11002 docs/dev/debt/libkernel.md
PR 11215 docs/dev/debt/libkernel.md
PR 11939 docs/dev/debt/libkernel.md
PR 12688 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 12700 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 12925 docs/dev/debt/libkernel.md
PR 12965 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@Gruzzberry Reviewed at f6681e1bc0. The fault results match FreeBSD 11.4: the address and length are copied in before the descriptor lookup, a datagram is dropped while stream data stays queued, the address copied out is MIN(namelen, sa_len), and nothing is sent or dequeued before EFAULT. #13752, #13742, #13738 and #13695 each merge cleanly in both orders. Changes requested in Udp.cpp, because the new probes reject some valid receives and add cost to every call:

  • GuestWritable (lines 426–435) requires both read and write access. On Linux, a PROT_WRITE receive buffer is valid for native recv, but the new GuestCopyIn preflight rejects it. Please validate output without requiring read permission and add a regression test.
  • The full requested-length probe at line 1657 rejects valid short receives. With a one-byte datagram, a destination at the final writable byte of a page, a protected next page, and a requested length of 2, native Linux recv returns 1. The new probe fails, and RefuseUnwritable discards the datagram and returns EFAULT. Please preserve successful receives when all actually transferred bytes are writable, and test the return value, bytes, and queue state.
  • Cost on the success path: GuestReadable and GuestWritable run on every successful sendto and recvfrom and scale with the requested length, not the bytes transferred, so a receive into a 64 KiB buffer makes about 34 checked copies per packet (each a host syscall plus the arena hooks; on Windows also a VirtualQuery and the mapping lock). Could the probe stay off the success path, for example by checking only after the native call fails with a fault, or, where the native call can't be tried safely (the Windows receive), by one VirtualQuery walk instead of reading and writing back a byte per page? Not writing back would also stop the call touching bytes past the received data, which FreeBSD never writes and another thread may be using.

The two receive cases were reproduced with the exact Linux checked-copy and probe bodies, real protected mappings and native datagram sockets (a focused source harness, not the full guest library). Separately, send() and recv() (lines 1468 and 1478) still use raw access; fine to leave for a follow-up, but worth a debt line.

@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 f6681e1bc0, 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_udp: 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_udp 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.

…ointer

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Gruzzberry

Copy link
Copy Markdown
Contributor Author

@oneandonlydean thank you. Rebased on current main (049619c202) and changed Udp.cpp as follows.

  1. GuestWritable is gone. On Linux and macOS the receive now goes straight to the host recvfrom, so a PROT_WRITE-only buffer works and a fault comes back as EFAULT from the host call. A regression test receives into a PROT_WRITE-only page (POSIX only; Windows has no write-only protection).
  2. No probe of the requested length. A 1-byte datagram into the last writable byte of a page, next page protected, requested length 2, now returns 1 with the byte in place and the queue empty (test, both UDP and TCP). On Windows the receive uses the writable prefix found by one VirtualQuery walk: if the pending datagram (peeked into the prefix) or the queued stream bytes (FIONREAD) fit in it, the receive runs into the prefix; otherwise the datagram is dropped and the call fails with EFAULT, stream data staying queued. A test checks that a datagram too big for the prefix is dropped (the next datagram arrives intact).
  3. Cost: no checked copies on the success path of either call. On Linux and macOS the send and receive are the host calls alone. On Windows a send makes one page walk, and a receive makes one walk, plus a peek only when the buffer is shorter than the request. GuestReadable (the checked copy) now runs only on error paths, to keep the FreeBSD order where a UDP buffer fault beats an address error. Nothing is written back to the buffer any more.
  4. send() and recv() are still raw; recorded as a debt entry in docs/dev/debt/libkernel.md, together with the stream edge cases below.

Known differences, in that entry: on Linux and macOS a stream receive or send that faults after copying some bytes returns the short count (FreeBSD returns EFAULT); on Windows data arriving after the check, or MSG_WAITALL, can return the prefix as a short success.

Tested on Windows 11, WinLibs GCC 15.2.0, Release: guest_udp 3/3 and the socket tests 3/3; guest_udp crashes with the previous head's Udp.cpp and the new tests. Full suite: only environmental failures (vulkan_validation_gate_reports, check_compile_test_windows_gcc_response_text_mode_is_not_reinterpreted) plus annotate_module timing out under load on this machine (it also times out on an unrelated branch), and guest_timed_wait_cpu which passes alone 3/3. check_conventions.py passes. Not run on Linux or macOS here; @jotitadev if you can rerun on Linux that would help.

AI-assisted: yes (Claude Code).

@Gruzzberry
Gruzzberry force-pushed the fix/recvfrom-sendto-efault branch from f6681e1 to 736f3c9 Compare October 31, 2026 11:39
@github-actions github-actions Bot added the docs label Oct 31, 2026
@adriadam10 adriadam10 self-assigned this Oct 31, 2026
@adriadam10
adriadam10 merged commit 577fe63 into boykopovar:main Oct 31, 2026
11 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the three points from the earlier review against the new head: GuestWritable is gone (POSIX receives go straight to the host call, PROT_WRITE-only test added), no requested-length probe (1-byte datagram at the last writable byte returns 1 and empties the queue; oversized datagram on Windows is dropped with EFAULT), and no checked copies on the success path. Address copy sizes are bounded (<= 255 in, min(length, size) out), the debt entry covers raw send/recv and the stream short-count differences, and the new guest_udp test crashes without the change. Conventions, windows and library-portability are green; 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