Repository navigation
fix(libc): use the Annex K size limit and codes of the PS5 libc, as measured on a PS5 - #8965
ionut27001 wants to merge 4 commits into
Conversation
|
Probe lines the body refers to (p83, payload process on a PS5, firmware 12.70; log lines 734-791, quoted unchanged): |
|
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.
|
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main 522e32e and the 12.70 log in this PR, plus #10270's console gist (dcadb97109bb78711ff3f7ba1dc479ed, firmware 13.60):
- The 2^61-1 limit and EINVAL for an oversized destsz match your log, #10270's psafestr2 boundary search, and the
rsizeMaxalready in main's BoundsChecking.cpp:29. It still merges cleanly with main. - #10270 now changes the same twelve functions to the same limit and EINVAL, also calls the constraint handler and covers wcscpy_s, so the two will conflict throughout Strings.cpp, GuestStrings.cpp, GuestWideStrings.cpp and libc.md. Could you coordinate with @orezraey on which one lands? The fopen_s test and debt rewrite here look like the part #10270 doesn't cover.
- Oversized counts: #10270's gist shows
memcpy_s(d, 4, "abc", 1<<61)returning 22 with the destination zeroed (psafestr3mcpy_count_big), while this branch returns 34 for that call (count > destsz).strncat_s(d="ab", 8, "c", 1<<61)returns 22 with d[0]=0 on the console (ncat_count_big), but 0 with "abc" here. psafestr3/4 show the same 22 for memmove_s, wcsncpy_s, wcsncat_s and wmemcpy_s, so the debt line saying those functions "accept one" would describe a known divergence. That run used a different firmware from yours, though. - wcscpy_s is on main now (SonyCrt.hpp:140, used by WideConversion.cpp:37 and libSceAmpr) with the old
(SIZE_MAX >> 1) / 2limit and ERANGE. With this PR,wcscpy_s(d, 0x2000000000000000, u"a")returns 0 whilewcsncpy_s(d, 0x2000000000000000, u"a", 1)returns 22. - Small one: the body still describes #7841 as open, but it's merged as BoundsChecking.cpp.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
@ionut27001 @harisibrar1 I checked the psafestr3/4 sources and raw logs, including the firmware and resolved-module lines. They show oversized memcpy_s counts returning 22 with the destination cleared, and oversized strncat_s counts returning 22 with dest[0] cleared. This branch instead returns 34 and success respectively. The older destination-size measurements do not establish a different result for these count cases. I’m changing this head to CHANGES rather than landing a known measured divergence and fixing it afterward. I recommend using #10270 for the shared safe-copy/append implementation after its #9935 prerequisite, then rebasing this PR onto that result and retaining any distinct fopen_s tests/debt evidence. That also brings wcscpy_s and the constraint-handler paths into the same implementation. Please coordinate that split so the two overlapping implementations do not land independently. Neither a stale debt statement that oversized counts are accepted nor the current green tests settle these measured count results. |
|
@ionut27001 The local declarations differ from main’s public headers: |
|
Checked on Linux: Ubuntu 26.04 (WSL2), GCC 15.2.0, Release, this head (
Not checked: Windows, macOS, a title. AI-assisted: yes (Claude Code). |
|
Agreed. I'll leave the shared safe-copy and append changes to #10270. Once #9935 and #10270 land, I'll rebase this PR onto them and keep only what is distinct:
If nothing distinct is left, I'll close it. |
What
The Annex K size limit and violation codes of the PS5 libc, as measured in a payload process on a PS5 (firmware 12.70), differ from the ones the copy functions in
core/libs/prx/libc/src/Strings.cppused.memcpy_saccepts a destination size up to0x1fffffffffffffffand returnsEINVAL(22) from0x2000000000000000up (L752-L766);wmemcpy_shas the same limit in elements (L772-L784). That isRSIZE_MAX= 2^61-1 for bytes and for wide elements. This libc usedSIZE_MAX >> 1for bytes and half of that for wide elements, and returnedERANGE(34) above it.strcpy_swith a destination size ofSIZE_MAXreturns 22 and leaves the destination alone (L787); this libc returned 34.strncpy_swith a count ofSIZE_MAX, a destination size of 4 and a 6-character source returns 22 withdest[0]set to zero (L791); this libc returned 34. The code checks the count againstRSIZE_MAXbefore whether the string fits; the measured call has a source that does not fit, so it does not show the case of a source that fits.What changed: one
RsizeMaxof 2^61-1 for the byte and wide Annex K functions ofStrings.cpp,EINVALinstead ofERANGEfor a destination size above it, and the count check instrncpy_s(and so instrcpy_s, which now passesRSIZE_MAXas its count).memmove_s,wmemmove_s,strncat_s,wcsncpy_sandwcsncat_stake the same limit and the sameEINVAL;strcat_sandwcscat_sfollow through them. Those siblings were not measured, so that is an inference from the measured functions, and the debt entry says so.memset_skeeps its own limit.Unchanged and matching the measurement: a count above the destination size returns 34 and zero-fills the destination (
memcpy_s, L767); an overlapping source, a null destination and a null source return 22, with the destination zero-filled for the overlap and for a null source with a count of 4 (L768-L771);strcpy_sreturns 34 withdest[0]set to zero when the source does not fit, 22 for a destination size of 0 and 22 withdest[0]set to zero for a null source (L785, L786, L788);strncpy_sreturns 0 for a count of 3 into a destination size of 4 and 34 withdest[0]set to zero for a count of 4 (L789, L790).fopen_sneeded no change: the errno of the failed open, the null file pointer on failure and 22 for a null path, mode or file pointer are what L734-L746 show. The test pins them.What is not done: the registered constraint handler is still not called for a violation. The measurement shows one call per violation for
memcpy_s,wmemcpy_s,strcpy_s,strncpy_sandfopen_swith a counting handler installed, but not what the default handler does, so adding calls would change what a title sees. The debt entries for the Annex K functions andfopen_sare cut to that and to the unmeasured parts.Relation to open pull requests:
bsearch_s,wcstok_s,wctomb_s,wcrtomb_s,mbstowcs_s,mbsrtowcs_s,gets_s,abort_handler_s) in a newBoundsChecking.cpp. This pull request changes none of them and only functions already inStrings.cpp, so it does not re-implement anything feat(libc): implement the remaining C11 Annex K bounds-checking functions #7841 adds, and it does not depend on it. feat(libc): implement the remaining C11 Annex K bounds-checking functions #7841 uses its ownrsizeMaxof 2^61-1, which agrees with the measured limit. The two sharecore/libs/prx/libc/CMakeLists.txtanddocs/dev/debt/libc.mdin different hunks;git apply --checkof feat(libc): implement the remaining C11 Annex K bounds-checking functions #7841's diff on this branch is clean.strncpy_sandstrncat_s) edits other lines of the same two functions; its diff applies cleanly on this branch.wcscpy_sto the same Annex K debt entry, so whichever of the two lands second rebases that line. Itswcscpy_sstill uses the old limit andERANGE.Tested
prx/libc/include/Api.hpp(FileStream*,char16_t*,size_t) instead of local declarations:ctest --test-dir build -R "^(guest_libc_annexk_rsizemax|guest_strings|guest_wide_strings)$"ran twice: 3 of 3 passed both times.ctest -R "guest_libc|guest_strings|guest_wide_strings|guest_string"ran twice: 8 of 8 passed both times.libc/src/Strings.cpprestored from the previous commit and rebuilt,guest_libc_annexk_rsizemax,guest_stringsandguest_wide_stringsfail (3 of 8); with the file restored they pass again.agc_driver_image_array_window_past_layer_limit_testsdoes not compile with MinGW (alignas(65536)).python tools/check_conventions.py --base origin/mainpasses at the head of this pull request.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)