Skip to content

Conversation

@jserv
Copy link
Contributor

@jserv jserv commented Jan 18, 2026

This replaces stack-local buffers with heap allocation for audio threads to eliminate use-after-free when threads outlive the calling function.

  • play_sfx/play_music: heap-allocate sound_t + data buffer together
  • sfx_handler/music_handler: take ownership and free argument
  • Use detached threads for native (non-blocking), joinable for EMSCRIPTEN
  • Handle pthread_create failure by freeing allocation
  • music_handler: free previous mid/music_midi_data to prevent leak
  • shutdown_audio: stop playback first, proper resource cleanup
  • Fix EMSCRIPTEN double-join by not setting init flags after immediate join

Summary by cubic

Fixes a use-after-free in SDL audio by moving sfx/music buffers to the heap and giving handler threads ownership of their data. Playback is now stable across native and EMSCRIPTEN, with proper cleanup and fewer leaks.

  • Bug Fixes

    • Heap-allocate sound_t and data; handlers free their argument.
    • Validate sfx/music size before allocation to avoid excessive guest-driven allocation.
    • Free previous mid and music_midi_data before loading new music.
    • Stop playback first, then free Mix_Music/Mix_Chunk in shutdown_audio.
    • Handle pthread_create failures by freeing allocations.
    • Fix EMSCRIPTEN double-join by joining immediately and not setting init flags.
  • Refactors

    • Native: use detached threads for non-blocking sfx/music.
    • EMSCRIPTEN: use joinable threads and join in play_*; keep thread handles only under EMSCRIPTEN.
    • Consolidated resource cleanup and removed invalid thread joins.

Written for commit dddf65d. Summary will update on new commits.

@jserv jserv added this to the release-2026.1 milestone Jan 18, 2026
@jserv jserv requested a review from ChinYikMing January 18, 2026 07:45
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 1 file

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/syscall_sdl.c">

<violation number="1" location="src/syscall_sdl.c:857">
P1: Validate `sfx_data_size` against a reasonable maximum before allocating. The new code removes the previous fixed-size bound, so a large or malformed size from guest memory can cause huge allocations or overflow.</violation>

<violation number="2" location="src/syscall_sdl.c:938">
P1: Validate `music_data_size` against a maximum before allocating. The new heap allocation removes the previous upper bound, so malformed sizes can lead to excessive allocation or overflow.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

This replaces stack-local buffers with heap allocation for audio threads
to eliminate use-after-free when threads outlive the calling function.
- play_sfx/play_music: heap-allocate sound_t + data buffer together
- sfx_handler/music_handler: take ownership and free argument
- Use detached thread for native (non-blocking), joinable for EMSCRIPTEN
- Handle pthread_create failure by freeing allocation
- music_handler: free previous mid/music_midi_data to prevent leak
- shutdown_audio: stop playback first, proper resource cleanup
- Fix EMSCRIPTEN double-join by not setting init flags after immediate
  join
- Validate size bounds before allocation to prevent DoS from guest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants