Repository navigation
fix(libSceNet): receive sceNetRecvmsg through the iovecs it had at entry - #11619
Conversation
sceNetRecvmsg guarded the destinations of the iovec table it saw at entry, then read the live table again to scatter the received bytes. When an earlier destination overlapped a later iovec, the received bytes replaced that iovec and the rest of the data went to an address that was never guarded, which crashed the host when that address was not writable. Copy the header and the iovec table once, as FreeBSD 11.4's sys_recvmsg does with copyin and copyiniov, use that copy for the guards, the scatter and the address, and write the header back from it as sys_recvmsg's copyout does. Fixes boykopovar#11610 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed head 3d2b723f714d2e7c643b612e490614898d5fbbfe.
[P2] Handle a zero-length iovec list without null pointer arithmetic. message_length() accepts iov_length == 0 with iov == nullptr, and sceNetRecvmsg() supports this zero-capacity receive path. The new std::vector<NetIovec> vectors(header.iov, header.iov + header.iov_length) evaluates nullptr + 0; pointer arithmetic on a null pointer is undefined, and the range constructor may subtract the null iterators. Please default-construct vectors and copy/assign the range only when iov_length > 0 (or otherwise explicitly handle the empty case), then add a zero-iovec receive test.
The overlapping-destination test is a good regression for the issue: it ensures the scatter uses the entry-time iovec descriptors after an earlier destination overwrites the guest table. The report says it fails on baseline and passes on the change on Windows 11; I did not rerun the Windows tests. At review time eligibility and report pass, while conventions/labels are still queued and intake is running; platform build jobs are skipped by path policy.
Copy the iovec table only when there is one, and receive a datagram with no iovecs in guest_sce_net: it returns 0 with MSG_TRUNC and the sender's address. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review. With this head the full suite passes 2546 of 2548 on Windows, with the same two failures as AI-assisted: yes (Claude Code) |
MarcZX10
left a comment
There was a problem hiding this comment.
Re-reviewed updated head ff5e67de1581fc5d3ec8e0cc2c976e25134c07db. The empty iovec case now leaves the copied vector list empty without forming a range from a null pointer, and the new regression covers a one-byte datagram with {iov=nullptr, iov_length=0} and checks the returned truncation/header fields. The earlier overlapping-iovec test still exercises the entry-time snapshot behavior. I verified that the diff from #11472 merges cleanly with this head; it changes sceNetSendmsg/sendto and adds separate fault tests, while this change is confined to sceNetRecvmsg and its test.
The author reports guest_sce_net passing 3/3 and the full Windows suite passing 2546/2548 with the same two baseline failures. I did not independently rerun those Windows tests. Hosted conventions, labels, eligibility and report pass; platform build jobs are skipped by path policy. The local host lacks required PS5 CPU instructions, so it cannot execute the guest tests here.
|
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.
|
jotitadev
left a comment
There was a problem hiding this comment.
First run on Linux, since the PR lists Linux as not tested.
Exact head ff5e67de1581fc5d3ec8e0cc2c976e25134c07db, merged on main cb45ae2dd. Linux x86_64 (bare metal, not WSL), i7-8700, Ubuntu 26.04, kernel 7.0, GCC 15.2, glibc 2.43, CMake 4.2.3, Ninja, Release, BUILD_TESTING=ON, full build.
guest_sce_netpasses, 3 of 3 runs. The 13guest_net*|guest_sce_net*tests pass.- Full non-GPU suite on the same
mainwithout the PR: 1361/1361. - With only
core/libs/prx/libSceNet/Export.cppreverted tomain(new test kept),guest_sce_netcrashes with SIGSEGV. So the overlapping-iovec case from #11610 also crashes on Linux, and the test catches it there.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Thanks @jotitadev for the bare-metal Linux run and the baseline-library control. I added both to the Tested section. AI-assisted: yes (Claude Code) |
What
sceNetRecvmsgnow receives through the header and iovec table it had at entry, instead of reading the live table again after the receive.It opened write guards for the destinations of the iovecs it saw at entry, received into a host buffer, then read
msg->iov[i]again to scatter the data. When an earlier destination overlaps a later iovec, as #11610 reports, the received bytes replace that iovec first. The rest of the data then goes to an address that was never guarded, and the process crashes when that address is not writable.FreeBSD 11.4's
sys_recvmsg:copyin, thencopyiniov);recvit);msg_iovand the newmsg_namelen,msg_controllenandmsg_flags(copyout).sceNetRecvmsgnow does the same:name_length;The checks and their order are unchanged. #11472 (sendmsg) touches the function above it, and the two merge cleanly.
Tested
Windows 11 Pro 26200, WinLibs GCC 15.2.0 r7, full Release build with
BUILD_TESTING=ON,main74acfad.guest_sce_netgainsCheckOverlappingReceiveVectors. A 20-byte datagram is received into two iovecs: the first is the 16 bytes of the second iovec, the second a 4-byte buffer. The first 16 payload bytes are an iovec pointing to a guest page made inaccessible withsceKernelMprotect. The receive must return 20 and put the last 4 bytes into the entry-time second buffer. The second iovec must then hold the received bytes, and the header must keepiov,iov_length2,flags0 andname_length16.Export.cppchange,guest_sce_netcrashes 3 of 3 with an access violation writing to that page (write of 0x0000000200420000). With the change it passes 3 of 3.mainon this machine:guest_wide_formatting(fix(libc): convert the Windows time zone name from the ANSI code page to UTF-8 for tzname, %Z and tm_zone #10981, German code page, fixed by fix(libc): convert Windows time zone names to UTF-8 #10987) andagc_driver_depth_plane_partial_store(AGC: imported depth-plane partial-store test fails refresh-proof assertion on Windows (NVIDIA and AMD) #10664, fixed by fix(agc): collect upload generation after host import #10696).guest_socket_host_write_sceNetRecvmsg_udpand_tcp, which check the write tracking and guards on the destinations, pass.iov_length > 0.CheckEmptyReceiveVectorsreceives a 1-byte datagram withiovnull andiov_length0: it returns 0 withMSG_TRUNC(0x10),name_length16 and the sender's address.net_empty_stream_recvmsg_nullbuf(Windows only) passes the same arguments on a stream socket. With this head the full suite again passes 2546 of 2548 with the same 2 failures, andguest_sce_netpasses 3 of 3.python tools/check_conventions.py --base origin/mainpasses.python tools/pr_overlap.py --branch fix/net-recvmsg-iovec-snapshotcould not compare 6 conflicting PRs (fix(libSceNet): stop Sackboy loading retries on peer sockets #5836, Macos port #7201, fix(libSceNet): report the receive queue length in sceNetGetSockInfo #9932, fix(libSceNet): describe sockets made with socket() in sceNetGetSockInfo #9948, feat(libSceNet): keep TCP option state #11272, build(libs): support Emscripten memory64 compilation #11341); I checked their diffs by hand and none changessceNetRecvmsgorCheckMessages.maincb45ae2 on bare-metal Linux (Ubuntu 26.04, GCC 15.2, Release) (review):guest_sce_netpasses 3 of 3, the 13guest_net*|guest_sce_net*tests pass, and with onlyExport.cppfrommainguest_sce_netcrashes with SIGSEGV, so the overlapping-iovec case also crashes on Linux before the change.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)🤖 Generated with Claude Code