Skip to content

fix(recorder): copy audio buffer before async handoff to fix use-after-free#1054

Open
christian-apollo wants to merge 3 commits intosoftware-mansion:mainfrom
christian-apollo:fix/recorder-callback-use-after-free
Open

fix(recorder): copy audio buffer before async handoff to fix use-after-free#1054
christian-apollo wants to merge 3 commits intosoftware-mansion:mainfrom
christian-apollo:fix/recorder-callback-use-after-free

Conversation

@christian-apollo
Copy link
Copy Markdown

Closes #1053

Introduced changes

IOSRecorderCallback::receiveAudioData and AndroidRecorderCallback::receiveAudioData were pushing the raw audio buffer pointer received from the platform (CoreAudio render callback on iOS, Oboe data callback on Android) onto an SPSC queue and then dereferencing it asynchronously from the offloader worker thread. Both the iOS AudioBufferList * and the Android void * are owned by the audio system only for the duration of the synchronous callback — by the time the worker thread runs memcpy (iOS) or ma_data_converter_process_pcm_frames (Android), the source buffer can already be reused or freed. That's a use-after-free; in production it surfaces as a _platform_memmove segfault inside IOSRecorderCallback::taskOffloaderFunction.

This PR copies the buffer into an owned allocation in receiveAudioData and frees it after the worker is done with it on every code path:

  • iOS — allocate a new AudioBufferList header plus per-channel mData, deep-copy the contents, send the owned pointer through the queue, and free it via a small freeOwnedAudioBufferList helper after both the fast (matching format) and the converter (resampling) paths have read from it.
  • Androidma_malloc an owned buffer, std::memcpy the frames, and ma_free after both the fast and resampling paths in taskOffloaderFunction. Also added a guard that ignores the data == nullptr sentinel that the TaskOffloader destructor sends to unblock the worker, since that sentinel carries no allocation to free.

The fix is symmetric on both platforms; the underlying mistake (capturing a borrowed buffer and reading it across threads) was the same on both sides.

Reproduction

The race is hard to reproduce deterministically — it depends on the SPSC worker being preempted long enough for the audio system to reuse the source slot before the worker reads it. Inspection alone shows the bug, and the production crash trace in #1053 lands exactly inside the iOS memcpy loop at IOSRecorderCallback.mm:164.

Checklist

  • Linked relevant issue
  • Updated relevant documentation — n/a, internal correctness fix
  • Added/Conducted relevant tests — the bug is a memory race in native audio thread paths; I'm not aware of an existing harness for these; happy to add one if you can point me at the right place
  • Performed self-review of the code
  • Updated Web Audio API coverage — n/a
  • Added support for web — n/a
  • Updated old arch android spec file — n/a

The receiver block on iOS (CoreAudio) and Android (Oboe) is invoked
synchronously and owns the audio buffer only for the duration of that
call. Both AndroidRecorderCallback::receiveAudioData and
IOSRecorderCallback::receiveAudioData pushed the raw pointer onto an
SPSC queue and dereferenced it later from the offloader thread, which
is a use-after-free — the buffer can be reused or freed by the audio
system before the worker thread reads it.

Copy the buffer into an owned allocation in receiveAudioData and free
it after taskOffloaderFunction is done with it (along both code paths,
including the converter path on iOS and the resampling path on
Android). Skip processing and frees on the cleanup sentinel
(data == nullptr) which the TaskOffloader destructor sends.

Crash signature on iOS:
  Crashed: Thread
  0  libsystem_platform.dylib       _platform_memmove + 144
  1  ApolloScooters                 audioapi::IOSRecorderCallback::taskOffloaderFunction
  2  ApolloScooters                 std::__thread_proxy<...IOSRecorderCallback::prepare...>
Copy link
Copy Markdown
Collaborator

@mdydek mdydek left a comment

Choose a reason for hiding this comment

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

nice one, while reviewing found another fatal race condition (destructor of the callback object could be fired in the same time when receiver function, thus making f.e. converterOutputBuffer_ invalid), I fixed it in this pr

@mdydek mdydek added the fix label May 7, 2026
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.

Use-after-free in iOS/Android recorder: buffer pointer dereferenced after callback returns

2 participants