Skip to content

fix(libSceFont): reject an external font count whose context size overflows 32 bits - #12201

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:fix/font-external-fonts-context-size-overflow
Nov 2, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:fix/font-external-fonts-context-size-overflow

Conversation

@MarcZX10

Copy link
Copy Markdown
Contributor

What

sceFontSupportExternalFonts sized the external font context as (fontMax << 6) | 0x20 in 32 bits. From fontMax = 0x04000000 the shift wraps, so CreateFontContext allocated and cleared a few bytes while recording the full fontMax as max_entries. The bounds check against max_entries (State.cpp) then never fires, and ClaimEntry / AcquireFontCtxEntry scan entries far past the allocation when a font is opened. For example sceFontSupportExternalFonts(lib, 0x04000001, 0x52) allocates 0x60 bytes.

Counts above 0x03ffffff are now rejected with SCE_FONT_ERROR_INVALID_PARAMETER before the context is created.

Two commits: the fix, then the test (in GuestFont.cpp: 0x04000000 and 0xffffffff are rejected, and the supported count of 4 still succeeds afterwards).

Tested

Linux x86-64, a throwaway archive build with the host CPU gate patched. guest_font passes. tools/check_conventions.py --base upstream/main and git diff --check pass. Not run against the unfixed code; the wrap follows from the 32-bit shift.

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 (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

…rflows 32 bits

sceFontSupportExternalFonts sized the external font context as
(fontMax << 6) | 0x20 in 32 bits. From fontMax = 0x04000000 the shift wrapped, so
CreateFontContext allocated and cleared a few bytes while recording the full
fontMax as max_entries. The bounds check against max_entries then never fired,
and opening a font scanned the context's entries far past the allocation.

Counts above 0x03ffffff are now rejected with SCE_FONT_ERROR_INVALID_PARAMETER
before the context is created.
…context size

0x04000000 and 0xffffffff have to be rejected as invalid parameters, and the
supported count of 4 still succeeds afterwards.
@github-actions github-actions Bot added the prx label Oct 28, 2026
@JJenkx

JJenkx commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

A Windows run, since the description lists none. Head 858eb3080e was merged onto main 82d5f01f28 with no conflicts. Setup: Windows 11 Enterprise 10.0.22621, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release, BUILD_TESTING=ON.

libs builds, and guest_font, including this PR's GuestFont.cpp checks, passes in each of 3 runs.

AI-assisted: yes (Claude Code).

@B-Howell

Copy link
Copy Markdown
Contributor

Results in the project's real build on Windows and Linux, with the control the description lists as not run ("Not run against the unfixed code").

Head 858eb3080e014e1c10610f93f5eddbf6e4960a38, merged the way CI does, alone in the tree.

Windows: Windows 11 Pro 26300, AMD Ryzen 7 7800X3D, WinLibs GCC 15.2.0 r7, Ninja Release, BUILD_TESTING=ON, libs rebuilt, on main 895ead67f7c11a46a7a29378c802d11864bed37a (clean).

  • ctest -R '^guest_font$' --repeat until-fail:10: passes all 10 runs.
  • Control: libSceFont/src/Library.cpp restored from main, test kept: aborts with Font check failed at line 442 (0xc0000409).

Linux: Ubuntu 22.04 under WSL2, GCC 11.4.0, Ninja Release, on main ccfb96757f9e31a8f4330cf25f193aae607d7409 (clean).

  • The same command passes all 10 runs.
  • The same control aborts with Font check failed at line 442.

In the merged file, line 442 is the first new check, sceFontSupportExternalFonts(library, 0x04000000u, 0x52) == SCE_FONT_ERROR_INVALID_PARAMETER. So on both platforms the old code accepts the count whose context size wraps, and the fix rejects it. The no file for system font set lines in the output are the existing expected warnings, not the failure.

Not tested: macOS, any title.

AI-assisted (Claude Code).

@maskym

maskym commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Windows result for head 858eb3080e, merged into main 87b810a1db (the PR's Tested section names no Windows run).

Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, BUILD_TESTING=ON, Ninja. Build clean.

The tree also carried the open PRs #12290, #12288, #12285, #12269, #12265, #12232, #12206, #12199, #12197, #12165, #12146, all merged cleanly, so the run is a joint one; the tests of this PR are guest_font (0.07 s), guest_font_graphics (0.03 s), guest_font_metrics (0.03 s), guest_font_outline_profile (0.04 s), guest_font_style_information (0.03 s), guest_font_words (0.03 s), guest_font_writing_line (0.04 s), all passed (every test registered from the tree's changed test files: 38/38). Not run on a PS5.

AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — Windows and Linux control results match the code, no change requested

The new check rejects a font count whose << 6 context size wraps to zero in 32 bits, and the test reports only confirm that behaviour.

What I ran

  • Live head: 858eb3080e014e1c10610f93f5eddbf6e4960a38 (OPEN, same as batch file).
  • gh api GET of issue comments, PR reviews and review comments for #12201; gh pr view --json statusCheckRollup; git -C /marc/AnyPS5 diff f1f17373… 858eb308… (PR diff against its merge base).
  • git show 858eb308…:core/libs/prx/libSceFont/src/Library.cpp and …:core/libs/tests/GuestFont.cpp (lines cited).
  • Nothing was built, run or merged.

Results

Check Result Note
conventions, eligibility, intake, labels SUCCESS hosted
report SUCCESS hosted
windows SUCCESS hosted
linux, relinker, relinker-asan, register-context, write-watch SKIPPED not run by the hosted matrix here

Findings

  • B-Howell (2026-10-28 05:45Z) says the old code accepts the wrapping count and the fix rejects it. Checked: Library.cpp:416 now returns SCE_FONT_ERROR_INVALID_PARAMETER when fontMax > (UINT32_MAX >> 6u). For fontMax = 0x04000000, fontMax << 6 is 0x100000000, which wraps to 0 in 32 bits, so the check is needed. The PR diff is one line in Library.cpp and two Require lines in GuestFont.cpp.
  • B-Howell's line number: the first new check is GuestFont.cpp:420, not 442. The report cites line 442 "in the merged file", which is a tree with main's lines included, so the offset is expected. Not verified: that merged line 442 is the same check.
  • JJenkx, maskym (2026-10-28): Windows and guest_font test reports, all passing, no question or change request. No reply needed.
  • Windows and Linux runs and the control aborts were not reproduced here.

Sources

  • int APS5_VABI sceFontSupportExternalFonts(FontLibrary library, std::uint32_t fontMax, std::uint32_t formats) {
    if (fontMax > (UINT32_MAX >> 6u)) return SCE_FONT_ERROR_INVALID_PARAMETER;
    return SupportFonts(library, &FontLibNative::external_fonts_ctx, fontMax, (fontMax << 6) | 0x20u, formats);
  • Require(sceFontSupportExternalFonts(library, 0x04000000u, 0x52) == SCE_FONT_ERROR_INVALID_PARAMETER);
    Require(sceFontSupportExternalFonts(library, 0xffffffffu, 0x52) == SCE_FONT_ERROR_INVALID_PARAMETER);

Not verified

  • The merged-tree line 442 claim and the 10-iteration Windows and Linux ctest results.
  • The macOS and PS5 behaviour, which the PR and the reports both leave untested.

@oneandonlydean
oneandonlydean merged commit c723bc7 into boykopovar:main Nov 2, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants