[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 <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#852240}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>

1 files changed, 29 insertions, 39 deletions

diff --git a/chromium/third_party/blink/renderer/modules/webaudio/script_processor_node.cc b/chromium/third_party/blink/renderer/modules/webaudio/script_processor_node.cc
index 12f620d5ac8..77f9478ec05 100644
--- a/chromium/third_party/blink/renderer/modules/webaudio/script_processor_node.cc
+++ b/chromium/third_party/blink/renderer/modules/webaudio/script_processor_node.cc
@@ -123,6 +123,16 @@ void ScriptProcessorHandler::Initialize() {
 }
 
 void ScriptProcessorHandler::Process(size_t frames_to_process) {
+  // The main thread might be accessing the shared buffers. If so, silience
+  // the output and return.
+  MutexTryLocker try_locker(process_event_lock_);
+  if (!try_locker.Locked()) {
+    // We're late in handling the previous request. The main thread must be
+    // very busy.  The best we can do is clear out the buffer ourself here.
+    Output(0).Bus()->Zero();
+    return;
+  }
+
   // Discussion about inputs and outputs:
   // As in other AudioNodes, ScriptProcessorNode uses an AudioBus for its input
   // and output (see inputBus and outputBus below).  Additionally, there is a
@@ -203,47 +213,27 @@ void ScriptProcessorHandler::Process(size_t frames_to_process) {
   buffer_read_write_index_ =
       (buffer_read_write_index_ + frames_to_process) % BufferSize();
 
-  // m_bufferReadWriteIndex will wrap back around to 0 when the current input
-  // and output buffers are full.
-  // When this happens, fire an event and swap buffers.
+  // Fire an event and swap buffers when |buffer_read_write_index_| wraps back
+  // around to 0. It means the current input and output buffers are full.
   if (!buffer_read_write_index_) {
-    // Avoid building up requests on the main thread to fire process events when
-    // they're not being handled.  This could be a problem if the main thread is
-    // very busy doing other things and is being held up handling previous
-    // requests.  The audio thread can't block on this lock, so we call
-    // tryLock() instead.
-    MutexTryLocker try_locker(process_event_lock_);
-    if (!try_locker.Locked()) {
-      // We're late in handling the previous request. The main thread must be
-      // very busy.  The best we can do is clear out the buffer ourself here.
-      output_buffer->Zero();
+    if (Context()->HasRealtimeConstraint()) {
+      // For a realtime context, fire an event and do not wait.
+      PostCrossThreadTask(
+          *task_runner_, FROM_HERE,
+          CrossThreadBind(&ScriptProcessorHandler::FireProcessEvent,
+                          WrapRefCounted(this), double_buffer_index_));
     } else {
-      // With the realtime context, execute the script code asynchronously
-      // and do not wait.
-      if (Context()->HasRealtimeConstraint()) {
-        // Fire the event on the main thread with the appropriate buffer
-        // index.
-        PostCrossThreadTask(
-            *task_runner_, FROM_HERE,
-            CrossThreadBind(&ScriptProcessorHandler::FireProcessEvent,
-                            WrapRefCounted(this), double_buffer_index_));
-      } else {
-        // If this node is in the offline audio context, use the
-        // waitable event to synchronize to the offline rendering thread.
-        std::unique_ptr<WaitableEvent> waitable_event =
-            std::make_unique<WaitableEvent>();
-
-        PostCrossThreadTask(
-            *task_runner_, FROM_HERE,
-            CrossThreadBind(
-                &ScriptProcessorHandler::FireProcessEventForOfflineAudioContext,
-                WrapRefCounted(this), double_buffer_index_,
-                CrossThreadUnretained(waitable_event.get())));
-
-        // Okay to block the offline audio rendering thread since it is
-        // not the actual audio device thread.
-        waitable_event->Wait();
-      }
+      // For an offline context, wait until the script execution is finished.
+      std::unique_ptr<WaitableEvent> waitable_event =
+          std::make_unique<WaitableEvent>();
+
+      PostCrossThreadTask(
+          *task_runner_, FROM_HERE,
+          CrossThreadBind(
+              &ScriptProcessorHandler::FireProcessEventForOfflineAudioContext,
+              WrapRefCounted(this), double_buffer_index_,
+              CrossThreadUnretained(waitable_event.get())));
+      waitable_event->Wait();
     }
 
     SwapBuffers();