[pull] master from libretro:master#971
Merged
pull[bot] merged 8 commits intoAlexandre1er:masterfrom Apr 27, 2026
Merged
Conversation
…cheevos paths
Fixes eight memory-safety bugs sharing a small set of root causes:
uint32_t multiplications feeding size_t allocations, ftell() and
filestream_get_size() returning -1 silently coerced to size_t,
attacker-controlled filenames used as filesystem paths, and
per-channel-count assumptions in audio decode.
* rjpeg, rpng (formats/jpeg/rjpeg.c, formats/png/rpng.c)
Width*height*sizeof(uint32_t) wraps in uint32 on 32-bit hosts
(3DS, Vita, PSP, Wii, Wii U, older Android, 32-bit Windows),
the malloc returns an undersized buffer, and the per-row decode
writes multi-GiB off the end. Add (size_t) casts at every
pixel-buffer malloc site so the multiplication is overflow-safe
on 64-bit. Add a 0x4000 dimension cap matching rbmp.c, rtga.c
and rwebp.c, gated on 32-bit only -- on 64-bit the size_t
arithmetic plus the existing 4 GiB output guard in
rpng_iterate_image are sufficient, and a hard cap would reject
legitimate large images (IrfanView and friends routinely open
tens-of-thousands-pixel JPEGs and PNGs).
* image_transfer Gekko path (formats/image_transfer.c)
Same uint32 multiplication primitive in the post-decode tile
conversion on Wii. Add (size_t) casts. The 32-bit rpng/rjpeg
cap closes the primitive at the source on Wii, but the casts
are kept for defence in depth.
* audio_mixer FLAC (audio/audio_mixer.c)
audio_mixer_mix_flac asks drflac for AUDIO_MIXER_TEMP_BUFFER/2
frames into a 32 KiB stack buffer. drflac writes
frame_count*channel_count floats, so 8-channel FLAC writes
128 KiB into the 32 KiB buffer -- a 96 KiB stack overflow
reaching saved-RIP territory. Reject non-stereo FLAC at
audio_mixer_play_flac time. Mono was already producing wrong
audio per the existing comment in mix_flac; a proper per-
channel-aware fix is left for a separate change.
* cheevos badge_name (cheevos/cheevos_client.c)
rcheevos_client_download_badge interpolates the server-supplied
badge_name into a filesystem path and writes the HTTP body
there. A malicious or MITM'd retroachievements.org could send
badge_name = "../../../../etc/cron.d/evil" and write attacker-
controlled bytes (with a forced .png suffix) anywhere on disk.
Real badge names are numeric IDs optionally suffixed with
"_lock"; reject anything outside [A-Za-z0-9_-].
* cheevos JSON-override loader (cheevos/cheevos_client.c)
ftell()'s long return was assigned to size_t _len, so an ftell
error (-1) became SIZE_MAX; malloc(_len + 1) wrapped to
malloc(0) and contents[_len] = 0 corrupted the heap. Capture
into long, check for the negative sentinel. Debug-only path
(CHEEVOS_JSON_OVERRIDE).
* rxml (formats/xml/rxml.c)
Same pattern, file-scale: filestream_get_size returns -1 on
error and silently flowed into (size_t)(len + 1) as 0 on 64-bit
(malloc(0) -> tiny block) or as a wrapped value on 32-bit
(undersized buffer). Either way memory_buffer[len] = '\0'
wrote far out of bounds. Reject negative sizes and any size
that wouldn't fit in size_t. Reachable via .xml shader presets
and Logiqx .dat database files.
* filestream_read_file (streams/file_stream.c)
The existing size check was
if ((int64_t)(uint64_t)(size + 1) != (size + 1)) goto error;
-- tautological for any positive int64_t, never trips. On
32-bit hosts any file > ~4 GiB silently truncates through the
size_t cast on the malloc and the subsequent filestream_read
overruns the heap. Replace with an explicit size_t-fit check.
* test/formats/test_rpng.c (new)
Six libcheck cases covering rpng_process_ihdr. The two that
exercise the 0x4000 cap (0x4001 and 30000^2) are gated on 32-
bit hosts only since on 64-bit those dimensions are legitimate
after this change. The remaining four (accept-at-limit,
uint32-max-reject, accept-small, zero-rejected) run on every
platform. Wired into Makefile.test alongside the existing
string/utils/hash/lists/queues suites; builds with -Werror
under the same TEST_UNIT_CFLAGS the other tests use; pulls in
trans_stream*.c + -lz for the zlib backend reference rpng.c
links to.
Adds libretro-db/samples/rmsgpack/rmsgpack_overflow_test, a self-contained regression test for the three preceding commits (b701dbb, ee37153, a22e6fc). Seven cases: - STR32 with len 0xFFFFFFFE on a 5-byte stream (rejected) - MAP32 with len 0x10000000 on a 5-byte stream (rejected) - ARRAY32 with len 0xFFFFFFFF on a 5-byte stream (rejected) - Truncated ARRAY16 claiming 10 entries with 5 available (rejected) - Valid fixmap of 2 entries with valid contents (accepted) - Valid ARRAY16 of 4 fixints (accepted) - Valid maximal STR8 (len 0xFF) with that many bytes (accepted) Verified to discriminate pre/post-patch: against b701dbb~1's rmsgpack.c the STR32 case segfaults under tight memory limits (ulimit -v 524288: malloc(0xFFFFFFFF) returns NULL, the unchecked *pbuff = ... is dereferenced); against the patched code all seven cases pass. The test follows the existing samples/ pattern (plain C, asserts manually, exit nonzero on failure) rather than libcheck because that's what the existing CI harness in .github/workflows/Linux-libretro-common-samples.yml understands. Uses intfstream_open_memory so the test needs no filesystem fixtures and the size-bound path fires (memory streams report a known size). Adds a sibling workflow Linux-libretro-db-samples.yml that walks libretro-db/samples/, builds every Makefile-rooted directory, and runs targets named in its RUN_TARGETS allowlist. The workflow is a near-verbatim copy of the libretro-common-samples one with the working-directory changed and an empty initial RUN_TARGETS allow- list except for rmsgpack_overflow_test; new tests added under libretro-db/samples/ in the future just need their Makefile target appended to that allowlist. Tested locally: the workflow's bash logic builds the test, runs it, and the test reports all 7 SUCCESS lines and exits 0. Build takes ~6 seconds on Ubuntu 24 with system gcc and zlib1g-dev.
The font path's whole-line scissor cull added in 5f23e85 only checked the bottom side: skip when the baseline ly is at or below the scissor's bottom edge. The top-side case was left out on the assumption that entries scrolled above the scissor "don't occur in practice." They do — scrolling Ozone's entry list far enough down pushes the topmost entry's text up past the scissor's top edge, where it bleeds into the header bar (the icons get correctly clipped by gfx_display_d3d8_draw's geometry path, but the text leaked through unchecked). Add the symmetric ly < sy cull. Visible glyphs sit at or above the baseline, so a baseline above sy means the whole line is above sy and entirely outside the scissor. Edge-aligned lines (baseline ~ sy or ~ sy2) still render in full; partial overlap isn't clipped. Pixel-perfect glyph clipping would need per-glyph bounding-box checks against the rect with UV remap similar to what gfx_display_d3d8_draw does for quads, which isn't worth the complexity for a fallback-quality backend. The whole-line cull is enough to stop the visible overflow into both of Ozone's header and footer regions.
The d3d8 backend's widgets_enabled hook was added in 5f23e85 to advertise widget support to the rest of RA, on the assumption (per the comment that landed alongside it) that widgets would "work transparently" once gfx_display was implemented. They didn't — d3d8_frame was missing the actual gfx_widgets_frame() call that every other consumer-facing video backend (d3d9/d3d10/d3d11/d3d12, gl1/gl2/gl3, vulkan, etc.) makes once per frame. Returning true from widgets_enabled tells RA to route runtime indicators — fast-forward, rewind, pause, FPS counter, achievement popups, load-progress bars, status text — into the widget queue (gfx_widgets_status_text + the per-widget animation state) instead of falling back to the runloop_msg → font_driver_render_msg path (see gfx/video_driver.c:4742). With no gfx_widgets_frame call, that state was assembled every frame but never rendered, so e.g. holding the fast-forward key produced no on-screen indicator. The fallback OSD msg path still rendered runloop_msg toasts when an overlay wasn't masking them, which is what made the regression look overlay-conditional in practice. Add the missing pieces: - Include gfx/gfx_widgets.h behind HAVE_GFX_WIDGETS, matching the d3d10/d3d9hlsl pattern. - Capture widgets_active from video_info at frame entry. - Between the overlay block and the OSD msg block, prepare the same device state the menu render path uses (reset menu_display.offset, bind the menu vertex buffer, full-screen viewport, ALPHABLENDENABLE re-enabled — d3d8_overlay_render disables it on the way out, defensive FVF) and call gfx_widgets_frame(video_info). No outer BeginScene/EndScene wrap is needed: gfx_display_d3d8_draw and d3d8_font_render_line both wrap their own scene around each DrawPrimitiveUP, matching the per-draw scene convention used throughout the rest of the d3d8 menu path. Also fix the comment on d3d8_gfx_widgets_enabled — the previous text claimed widgets worked transparently, which was wrong; it now documents what advertising support actually triggers in RA.
The previous commit added a libretrodb_leak_test that, under
ASan, catches reintroduced leaks in libretrodb_open / close /
cursor_close. The default Linux-libretro-db-samples.yml
workflow build is plain `make clean all` though, so a future
regression would not actually be caught in CI -- the test would
build and run, exit 0, and the leak would slip through.
Make ASan + UBSan the default for this workflow. Three changes:
1. .github/workflows/Linux-libretro-db-samples.yml: add a
MAKE_ARGS env var = "SANITIZER=address,undefined" and pass
it to the per-sample 'make clean all' invocation.
2. libretro-db/samples/rmsgpack/Makefile and
libretro-db/samples/libretrodb/Makefile (libretrodb's was
added in the previous commit): add the conventional
SANITIZER opt-in block:
ifneq ($(SANITIZER),)
CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS)
LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS)
endif
Matches the convention used in samples/tasks/http/Makefile.
Default build is unchanged; the workflow opts in via
MAKE_ARGS.
3. libretro-db/samples/rmsgpack/rmsgpack_overflow_test.c:
close a pre-existing 7-allocation / 336-byte leak in the
test harness itself. The test called intfstream_close on
each open_memory'd stream but never freed the
intfstream_t struct, mirroring the same convention-
mismatch as the libretrodb_open path the previous commit
fixed in production. Without this, ASan would report
this test as failing under the new SANITIZER=address
build.
Local simulation of the workflow under ASan:
- rmsgpack_overflow_test: 7 SUCCESS lines, exit 0, no ASan
output.
- libretrodb_leak_test: 4 SUCCESS lines, exit 0, no ASan
output.
Reverting the production fix in libretrodb.c (the previous
commit) makes libretrodb_leak_test report 460 bytes leaked
across 10 allocations under this workflow, which is what we
want.
…ts_frame
When an onscreen overlay was active, gfx_widgets (the fast-forward
indicator, FPS counter, achievement popups, load-progress bars,
etc.) silently rendered nothing on d3d10/d3d11/d3d12. Same symptom
across all three backends, same root cause: d3dN_render_overlay
clobbers render state that gfx_widgets_frame relies on, and the
state isn't restored before the widget draw.
Specifically, the overlay render path:
- Binds overlays.vbo (d3d10/d3d11) / overlays.vbo_view (d3d12)
as the IA-stage vertex buffer.
- Sets frame.viewport / frame.scissorRect when the overlay is
not fullscreen (i.e. clamped to the game viewport, not the
full screen).
- On d3d12, also leaves sprites.pipe_blend bound — fine for SDR,
but the OSD-msg block immediately below selects pipe_blend_hdr
in HDR modes, and widgets need to follow the same selection.
gfx_display_d3dN_draw (the entry point widgets use) writes its
sprite vertices into sprites.vbo via Map(), then issues a Draw()
that reads from whatever vbo is currently bound. After an
overlay, that's overlays.vbo — so the GPU pulls vertices from the
wrong buffer at the wrong stride and the result is invisible
geometry, even though the widget state machine has run normally
and the sprite buffer contents are correct. Widgets without an
overlay work because nothing has clobbered the binding the menu
path or the post-frame setup left in place.
Restore the sprite-pipeline state at the top of each widget block,
mirroring the OSD-msg block that follows the widget block in each
of these drivers (which already does this and works for the same
reason):
- d3d10: viewport, blend state, sprites.vbo.
- d3d11: viewport, blend state, sprites.vbo (the existing
viewport-only restore was only a partial fix).
- d3d12: PSO (HDR-aware), viewport, scissor rect, sprites.vbo_view.
Vulkan and gl1 don't hit this because their overlay render paths
either don't share input bindings with widgets (separate command
buffers / fixed-function state) or they happen to set up widget
state from scratch on every frame.
Reported by users of the Steam Deck / Windows community on builds
where overlay + widgets are both enabled. Companion to 7d6477b
which fixed the analogous (but different-mechanism — widgets
weren't being rendered at all) bug on d3d8.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )