Skip to content

Commit 1375de9

Browse files
hochmibrunin
authored andcommitted
[Backport] CVE-2021-21165: Object lifecycle issue in audio
Prevent accessing shared buffers from audio rendering thread The shared buffer in ScriptProcessorNode can be accessed by the audio rendering thread when it is held by the main thread. The solution suggested here is simply to expand the scope of the mutex to minimize the code change. This is a deprecated feature in Web Audio, so making significant changes is not sensible. By locking the entire scope of Process() call, this area would be immune to the similar problems in the future. Bug: 1174582 Test: The repro case doesn't crash on ASAN. Change-Id: I2b292f94be65e6ec26c6eb0e0ed32b3fb2d88466 Commit-Queue: Hongchan Choi <[email protected]> Reviewed-by: Raymond Toy <[email protected]> Cr-Commit-Position: refs/heads/master@{#852240} Reviewed-by: Allan Sandfeld Jensen <[email protected]> Reviewed-by: Jüri Valdmann <[email protected]>
1 parent 306867a commit 1375de9

File tree

1 file changed

+29
-39
lines changed

1 file changed

+29
-39
lines changed

chromium/third_party/blink/renderer/modules/webaudio/script_processor_node.cc

Lines changed: 29 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,16 @@ void ScriptProcessorHandler::Initialize() {
123123
}
124124

125125
void ScriptProcessorHandler::Process(size_t frames_to_process) {
126+
// The main thread might be accessing the shared buffers. If so, silience
127+
// the output and return.
128+
MutexTryLocker try_locker(process_event_lock_);
129+
if (!try_locker.Locked()) {
130+
// We're late in handling the previous request. The main thread must be
131+
// very busy. The best we can do is clear out the buffer ourself here.
132+
Output(0).Bus()->Zero();
133+
return;
134+
}
135+
126136
// Discussion about inputs and outputs:
127137
// As in other AudioNodes, ScriptProcessorNode uses an AudioBus for its input
128138
// and output (see inputBus and outputBus below). Additionally, there is a
@@ -203,47 +213,27 @@ void ScriptProcessorHandler::Process(size_t frames_to_process) {
203213
buffer_read_write_index_ =
204214
(buffer_read_write_index_ + frames_to_process) % BufferSize();
205215

206-
// m_bufferReadWriteIndex will wrap back around to 0 when the current input
207-
// and output buffers are full.
208-
// When this happens, fire an event and swap buffers.
216+
// Fire an event and swap buffers when |buffer_read_write_index_| wraps back
217+
// around to 0. It means the current input and output buffers are full.
209218
if (!buffer_read_write_index_) {
210-
// Avoid building up requests on the main thread to fire process events when
211-
// they're not being handled. This could be a problem if the main thread is
212-
// very busy doing other things and is being held up handling previous
213-
// requests. The audio thread can't block on this lock, so we call
214-
// tryLock() instead.
215-
MutexTryLocker try_locker(process_event_lock_);
216-
if (!try_locker.Locked()) {
217-
// We're late in handling the previous request. The main thread must be
218-
// very busy. The best we can do is clear out the buffer ourself here.
219-
output_buffer->Zero();
219+
if (Context()->HasRealtimeConstraint()) {
220+
// For a realtime context, fire an event and do not wait.
221+
PostCrossThreadTask(
222+
*task_runner_, FROM_HERE,
223+
CrossThreadBind(&ScriptProcessorHandler::FireProcessEvent,
224+
WrapRefCounted(this), double_buffer_index_));
220225
} else {
221-
// With the realtime context, execute the script code asynchronously
222-
// and do not wait.
223-
if (Context()->HasRealtimeConstraint()) {
224-
// Fire the event on the main thread with the appropriate buffer
225-
// index.
226-
PostCrossThreadTask(
227-
*task_runner_, FROM_HERE,
228-
CrossThreadBind(&ScriptProcessorHandler::FireProcessEvent,
229-
WrapRefCounted(this), double_buffer_index_));
230-
} else {
231-
// If this node is in the offline audio context, use the
232-
// waitable event to synchronize to the offline rendering thread.
233-
std::unique_ptr<WaitableEvent> waitable_event =
234-
std::make_unique<WaitableEvent>();
235-
236-
PostCrossThreadTask(
237-
*task_runner_, FROM_HERE,
238-
CrossThreadBind(
239-
&ScriptProcessorHandler::FireProcessEventForOfflineAudioContext,
240-
WrapRefCounted(this), double_buffer_index_,
241-
CrossThreadUnretained(waitable_event.get())));
242-
243-
// Okay to block the offline audio rendering thread since it is
244-
// not the actual audio device thread.
245-
waitable_event->Wait();
246-
}
226+
// For an offline context, wait until the script execution is finished.
227+
std::unique_ptr<WaitableEvent> waitable_event =
228+
std::make_unique<WaitableEvent>();
229+
230+
PostCrossThreadTask(
231+
*task_runner_, FROM_HERE,
232+
CrossThreadBind(
233+
&ScriptProcessorHandler::FireProcessEventForOfflineAudioContext,
234+
WrapRefCounted(this), double_buffer_index_,
235+
CrossThreadUnretained(waitable_event.get())));
236+
waitable_event->Wait();
247237
}
248238

249239
SwapBuffers();

0 commit comments

Comments
 (0)