Repository navigation
fix(net): reject unwritable resolver error outputs - #14250
oneandonlydean merged 1 commit 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
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — looks good
The checked guest-memory write preserves the existing null-argument error and returns a bounded error for an unwritable non-null output; the console behavior for that latter case remains explicitly unmeasured.
What I ran
Reviewed head 98991c19d81502e3af6d24e8e13d95d834acc4a4 against base 8728104b26178cd05433390b79d3e71fde7a2b1c; the PR is open and its merge preview is clean against current main (ad4854ce62584b3438576786299c88e79d2e8601). Environment: Linux x86_64, Python 3.11.2, GCC 12.2.0, Git 2.39.5.
git diff --check 8728104b26178cd05433390b79d3e71fde7a2b1c...98991c19d81502e3af6d24e8e13d95d834acc4a4
python3 tools/check_conventions.py --base 8728104b26178cd05433390b79d3e71fde7a2b1c --head 98991c19d81502e3af6d24e8e13d95d834acc4a4 --title 'fix(net): reject unwritable resolver error outputs' --body /tmp/anyps5-pr-precheck-14250/pr-body.md
python3 tools/pr_abi_audit.py --base 8728104b26178cd05433390b79d3e71fde7a2b1c --head 98991c19d81502e3af6d24e8e13d95d834acc4a4 --json
python3 tools/pr_test_quality.py --root /tmp/anyps5-review-14250 --json core/libs/prx/libSceNet/Export.cpp core/libs/tests/GuestSceNet.cpp docs/dev/debt/libSceNet.md
python3 tools/pr_stack_check.py 14250 --repo boykopovar/AnyPS5 --output-dir /tmp/anyps5-pr-precheck-14250/stackResults
| Check | Result |
|---|---|
| Diff whitespace | Passed; no output (empty diff-check output). |
| Conventions and PR template | Passed. |
| Export/ABI audit | Passed with one unrelated lexical warning: RunNetThread has no header declaration. The changed resolver export had no finding. |
| Test-quality heuristic | Warning only: it did not find the C++ test path literally in CMake; the test is included via the repository's generated test source registration. This is a heuristic false positive, not a test failure. |
| Current-base stack/merge preview | Passed; clean against current main. |
| Hosted CI on this head | Windows, conventions, labels, intake, report, library-portability, and eligibility passed. Linux and specialized jobs were skipped. |
| Build / local C++ test | Not run. |
Raw output excerpts
All conventions checks passed.
{"status":"passed_with_warnings","findings":[{"symbol":"RunNetThread","kind":"public-declaration-not-found","message":"No APS5_VABI declaration was found in a header; verify whether this is intentionally private or missing from the public interface."}]}{"findings":[{"path":"core/libs/tests/GuestSceNet.cpp","line":1,"kind":"possible-unregistered-cpp-test","message":"Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."}],"negative_control_candidates":[]}
Findings
Blocking
None found in the reviewed change.
Non-blocking
The PR documents that the console's result for a non-null but unwritable status pointer is unmeasured. AnyPS5 now returns EFAULT after a checked guest write fails, while the existing null check still returns EINVAL and unknown resolver IDs still return EBADF. That is a clearly scoped host-safety behavior; no console equivalence is claimed.
Sources
- Resolver implementation at the reviewed head
- Regression assertions at the reviewed head
- Debt note at the reviewed head
Not verified
No local build or C++ test execution; the exact-head C++ test binary was not available. Linux and specialized hosted jobs were skipped. No console run was performed, so the behavior of an unwritable output on PS5 remains unknown.
|
Windows result for head Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release,
AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine. |
What
sceNetResolverGetErrorpreviously wrote the resolver status through a raw guest pointer after validating the resolver ID. A non-null, unmapped output pointer crashed the host. Use the existing checked guest write and returnEFAULTwhen the copy fails. PreserveEINVALfor null output andEBADFfor an unknown resolver.Add an unmapped-output regression beside the existing resolver error tests. The console's exact error for this pointer remains unmeasured and is recorded in libSceNet technical debt.
Tested
guest_sce_netterminated with SIGSEGV on main. With the fix, the same CTest suite passed (1/1, 1.44 s).BUILD_TESTING=ONpassed ate8c7c8438c(based on mainca82d0c022), followed by 13 passing network CTest suites. After refreshing onto main8728104b26, the affected target rebuilt andguest_sce_netpassed again at98991c19d8.python3 tools/check_conventions.py --base upstream/main --head HEAD: passed on the final head.tools/pr_overlap.pyscanned 1,634 open PRs at the prior rebased headd62428e9a3. The final main refresh adds only an unrelated macOS write-watch test. The shared debt insertion with our fix(net): reject unwritable MAC address outputs #14244 was removed; the scan no longer reports it. Seven shared-file comparisons failed closed because those PRs conflict with main: fix(libSceNet): stop Sackboy loading retries on peer sockets #5836, fix(libSceNet): report the receive queue length in sceNetGetSockInfo #9932, feat(libkernel): cancel blocked POSIX reads with owned operations #12682, fix(libkernel): preserve blocking sockets over nonblocking native transport #12688, feat(libkernel): cancel blocked POSIX writes with partial progress #12700, feat(libkernel): support deferred cancellation in POSIX accept #12965, and docs(debt): separate entries with blank lines and drop derived export totals #14212. Their exact heads and changed hunks were manually checked; none editssceNetResolverGetErroror this regression. docs(debt): separate entries with blank lines and drop derived export totals #14212 changes debt-file spacing only.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)