Skip to content

fix(libkernel): check socket address arguments before the descriptor - #8392

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
xXPBSXx:fix/socket-argument-order-fb
Oct 21, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
xXPBSXx:fix/socket-argument-order-fb

Conversation

@xXPBSXx

@xXPBSXx xXPBSXx commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What

AddressLengthError (the length and null checks of Address(), i.e. FreeBSD getsockaddr) now runs before Lookup() in bind, connect and sendto (only when an address is given). Valid arguments on a closed descriptor still fail with EBADF; the family and short-AF_INET checks stay after the descriptor lookup.
Fixes #8391
Follow-up to #8154 (socket address length). setsockopt is not changed here; the pointer and length checks of setsockopt/getsockopt are in #7511. git merge-tree against the open socket PRs #7490, #7850, #7475 and #8312 is clean; pr_overlap was not run.

Tested

Windows 10 (19045), MinGW-w64 GCC 15.2.0, Release, on upstream/main d50ec76 + this commit: guest_socket_address_length fails without the change to Udp.cpp (bind(-1) with a 1-byte address answers 9) and passes with it; 28 other socket and sceNet tests pass (29 of 29); python tools/check_conventions.py --base upstream/main passes. Not tested: Linux/macOS, a console, a title run.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw; 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
  • Unblocks: none
  • AI-assisted: yes

AI-assisted (Claude Code).

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 20, 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 9169 core/libs/tests/GuestSocketAddressLength.cpp L58, L70

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
PR 7201 core/libs/prx/libkernel/Socket/src/Udp.cpp

@xXPBSXx

xXPBSXx commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Overlap note: #7511 (validate the pointers of getsockopt and setsockopt like FreeBSD) adds the same null-value and negative-length checks for setsockopt. git merge-tree of the two heads is clean, but the setsockopt part of this PR then repeats those checks; this PR only moves them in front of the descriptor lookup, which is what kern_setsockopt does (uipc_syscalls.c:1414-1417). If #7511 lands first, I can rebase this to the descriptor-order change only. I should have listed #7511 in the description; pr_overlap was not run before opening.

bind, connect and sendto looked the descriptor up before they looked at the
address length, so a descriptor that is not open hid an argument error.
FreeBSD 11.4 sys_bind, sys_connect and sendit run getsockaddr first
(ENAMETOOLONG, EINVAL, EFAULT) and only then look the descriptor up, so
these arguments are now answered first and EBADF only follows for a valid
address.

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

Copy link
Copy Markdown
Collaborator

Thanks, the ordering matches FreeBSD's getsockaddr before getsock. One thing in the description: it says setsockopt now rejects a null value with a non-zero length and a negative length before the descriptor, but the diff doesn't change setsockopt, and it still looks the descriptor up first. Could you either drop that sentence or add the change with a test?

@xXPBSXx xXPBSXx changed the title fix(libkernel): check socket address and option arguments before the descriptor fix(libkernel): check socket address arguments before the descriptor Oct 20, 2026
@xXPBSXx

xXPBSXx commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, you are right: the diff changes only the address checks of bind, connect and sendto; setsockopt still looks the descriptor up first. The setsockopt sentence came from an earlier version of this change that I dropped because #7511 covers the pointer and length checks of setsockopt/getsockopt. I removed it from the description, the title and the linked issue. My earlier comment here about the setsockopt part overlapping #7511 was therefore also wrong: there is no overlap in setsockopt.

@xXPBSXx

xXPBSXx commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Linux data point for ce8f7803ea (WSL2, Ubuntu 24.04, GCC 13.3, Release, -j4; a virtualised kernel, not bare-metal Linux): the socket and UDP tests pass on Linux (14 of 14). This replaces the "Linux not tested" line in the description for this head. Not tested: macOS.

AI-assisted (Claude Code).

@adriadam10 adriadam10 self-assigned this Oct 21, 2026
@adriadam10
adriadam10 merged commit 0b117a8 into boykopovar:main Oct 21, 2026
12 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the order of bind, connect and sendto against FreeBSD 11.4 (getsockaddr before getsock), the new test pins the length/null errors before EBADF and fails on main, the family checks stay after the lookup, and Windows plus WSL2 runs are in the thread; merged.

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

Labels

fix error Closes an issue (Fixes #N) libkernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

libkernel: a descriptor that is not open hides the address argument errors of bind, connect and sendto

3 participants