Repository navigation
fix(libSceNet): validate bind and connect guest addresses - #11559
kattkaries wants to merge 2 commits into
Conversation
|
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head c96633ea5f585e9ead63dd5f630de730f9e9e6b2. The bind/connect paths snapshot the guest address before conversion or socket operations, and the UDP AF_UNSPEC disconnect decision uses that same checked snapshot. The focused tests exercise unreadable page-boundary inputs and recovery/association behavior. The PR reports 22 focused Linux checks passing after rebase; I did not rerun them here, and Windows/macOS/console behavior remains untested as stated. No actionable correctness issue found in the reviewed change.
|
A Windows run of
|
What
sceNetBindandsceNetConnectread guest socket addresses directly, so unreadable addresses could crash the host. They also accepted unreadable trailing bytes without validating them. Both now snapshot up tosizeof(sockaddr_storage)bytes throughGuestSocketReadGuest_nid_no_patchbefore conversion. Connect reads the datagram disconnect family from the same snapshot, so a failed address read cannot disconnect a UDP association. Failed reads return0x8041010eand set errno 14. Existing descriptor, null, short-address and already-bound checks retain their order and errors.The eight new tests cover wholly unreadable addresses, a split family header, a split address body, and unreadable trailing bytes, for IPv4 and IPv6 UDP sockets. They check error results, valid-call recovery, and association preservation, including unreadable AF_UNSPEC addresses.
Reference: FreeBSD 11.4
getsockaddrcopies the supplied address before socket operations: https://github.com/freebsd/freebsd-src/blob/releng/11.4/sys/kern/uipc_syscalls.c . This motivates checked reads; no console validation is claimed. Snapshots are capped at host sockaddr_storage size; this change does not establish console behavior for oversized address lengths.Open-PR review: #11472 fixes sendmsg/sendto guest reads and explicitly excludes bind/connect; it shares Export.cpp and CMakeLists.txt. #7490 adds a listening-socket check later in sceNetConnect; its exact diff does not change our validation lines. #9834 implements HTTP transport. No open PR found implements this pointer-validation fix. The full pr_overlap tool was run. Its CMake insertion conflict with #10949 was resolved by moving our test registration; the rerun reports no direct overlaps. It could not compare six PRs that conflict with main (#5836, #7201, #9932, #9948, #11272, #11341); their actual branch diffs were inspected and do not change these address-read functions or introduce this helper.
Tested
Ubuntu 24.04 container on Linux, GCC 13.3.0, CMake 3.28.3, Release full-build configuration with BUILD_TESTING=ON. Initial reproduction base: afc780e. Rebased main: 263eee0. Final tested head: c96633e. All 22 focused checks passed again after the rebase and CMake registration move.
Before changing Export.cpp, all eight new cases failed on main: six segfaulted, and two aborted because unreadable trailing bytes did not return EFAULT. The final tests additionally cover zero lengths and failed AF_UNSPEC disconnect attempts.
After the fix, 22/22 tests passed, zero skipped:
cmake --build build --target guest_net_address_fault_tests guest_sce_net_tests guest_net_buffer_validation_tests net_dontwait_tests net_blocking_send_tests --parallel 6 ctest --test-dir build -R '^(guest_net_address_fault_.*|guest_sce_net|guest_net_buffer_validation|net_dontwait_.*|public_headers)$' --output-on-failure python3 tools/check_conventions.py --base origin/mainConventions and git diff --check pass. Only the relevant runtime targets were built; the entire project/test suite was not built/run. Windows, macOS and console behavior were not tested. No title tested.
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)