[Backport] CVE-2020-15972: Use after free in audio.

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2410743:
Add mutex for allowing the graph to be pulled

Basically add a mutex to protect access to
allow_pulling_audio_graph_.  The main thread waits until it has the
lock before setting this.  This prevents the main thread from deleting
things while the audio thread is pulling on the graph.

A try lock is used so as not to block the audio thread if it can't get
lock.

This is applied to both real time and offline contexts which required
moving the original real-time-only implementation to
audio_destination_node so we can use the same methods for the offline
context.

Tested the repro case from 1125635, and the issue does not reproduce.
We're assuming this will fix 1115901, but I've not been able to
reproduce that locally.

Bug: 1125635, 1115901
Change-Id: I1037d8c44225c6dcc8fe906c29a5a86740a15e1d
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

5 files changed, 105 insertions, 56 deletions

diff --git a/chromium/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc b/chromium/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc
index c85f6d2e26f..9b1a5ec495f 100644
--- a/chromium/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc
+++ b/chromium/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc
@@ -30,7 +30,8 @@
 namespace blink {
 
 AudioDestinationHandler::AudioDestinationHandler(AudioNode& node)
-    : AudioHandler(kNodeTypeDestination, node, 0) {
+    : AudioHandler(kNodeTypeDestination, node, 0),
+      allow_pulling_audio_graph_(false) {
   AddInput();
 }
 
diff --git a/chromium/third_party/blink/renderer/modules/webaudio/audio_destination_node.h b/chromium/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
index c35722c9296..2d1a60c5bb5 100644
--- a/chromium/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
+++ b/chromium/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
@@ -71,6 +71,53 @@ class AudioDestinationHandler : public AudioHandler {
     return is_execution_context_destroyed_;
   }
 
+  // Should only be called from
+  // RealtimeAudioDestinationHandler::StartPlatformDestination for a realtime
+  // context or OfflineAudioDestinationHandler::StartRendering for an offline
+  // context---basically wherever the context has started rendering.
+  // TODO(crbug.com/1128121): Consider removing this if possible
+  void EnablePullingAudioGraph() {
+    MutexLocker lock(allow_pulling_audio_graph_mutex_);
+    allow_pulling_audio_graph_.store(true, std::memory_order_release);
+  }
+
+  // Should only be called from
+  // RealtimeAudioDestinationHandler::StopPlatformDestination for a realtime
+  // context or from OfflineAudioDestinationHandler::Uninitialize for an offline
+  // context---basically whenever the context is being stopped.
+  // TODO(crbug.com/1128121): Consider removing this if possible
+  void DisablePullingAudioGraph() {
+    MutexLocker lock(allow_pulling_audio_graph_mutex_);
+    allow_pulling_audio_graph_.store(false, std::memory_order_release);
+  }
+
+  // TODO(crbug.com/1128121): Consider removing this if possible
+  bool IsPullingAudioGraphAllowed() const {
+    return allow_pulling_audio_graph_.load(std::memory_order_acquire);
+  }
+
+  // If true, the audio graph will be pulled to get new data.  Otherwise, the
+  // graph is not pulled, even if the audio thread is still running and
+  // requesting data.
+  //
+  // For an AudioContext, this MUST be modified only in
+  // RealtimeAudioDestinationHandler::StartPlatformDestination (via
+  // AudioDestinationHandler::EnablePullingAudioGraph) or
+  // RealtimeAudioDestinationHandler::StopPlatformDestination (via
+  // AudioDestinationHandler::DisablePullingAudioGraph), including destroying
+  // the the realtime context.
+  //
+  // For an OfflineAudioContext, this MUST be modified only when the the context
+  // is started or it has stopped rendering, including destroying the offline
+  // context.
+
+  // TODO(crbug.com/1128121): Consider removing this if possible
+  std::atomic_bool allow_pulling_audio_graph_;
+
+  // Protects allow_pulling_audio_graph_ from race conditions.  Must use try
+  // lock on the audio thread.
+  mutable Mutex allow_pulling_audio_graph_mutex_;
+
  protected:
   void AdvanceCurrentSampleFrame(size_t number_of_frames) {
     current_sample_frame_.fetch_add(number_of_frames,
diff --git a/chromium/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc b/chromium/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc
index 851fefa39a5..84643933d14 100644
--- a/chromium/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc
+++ b/chromium/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc
@@ -93,6 +93,7 @@ void OfflineAudioDestinationHandler::Uninitialize() {
 
   render_thread_.reset();
 
+  DisablePullingAudioGraph();
   AudioHandler::Uninitialize();
 }
 
@@ -112,6 +113,7 @@ void OfflineAudioDestinationHandler::StartRendering() {
   // Rendering was not started. Starting now.
   if (!is_rendering_started_) {
     is_rendering_started_ = true;
+    EnablePullingAudioGraph(); 
     PostCrossThreadTask(
         *render_thread_task_runner_, FROM_HERE,
         CrossThreadBindOnce(
@@ -293,20 +295,34 @@ bool OfflineAudioDestinationHandler::RenderIfNotSuspended(
 
   DCHECK_GE(NumberOfInputs(), 1u);
 
-  // This will cause the node(s) connected to us to process, which in turn will
-  // pull on their input(s), all the way backwards through the rendering graph.
-  scoped_refptr<AudioBus> rendered_bus =
-      Input(0).Pull(destination_bus, number_of_frames);
+  {
+    // The entire block that relies on |IsPullingAudioGraphAllowed| needs
+    // locking to prevent pulling audio graph being disallowed (i.e. a
+    // destruction started) in the middle of processing
+    MutexTryLocker try_locker(allow_pulling_audio_graph_mutex_);
+
+    if (IsPullingAudioGraphAllowed() && try_locker.Locked()) {
+      // This will cause the node(s) connected to us to process, which in turn
+      // will pull on their input(s), all the way backwards through the
+      // rendering graph.
+      scoped_refptr<AudioBus> rendered_bus =
+          Input(0).Pull(destination_bus, number_of_frames);
+
+      if (!rendered_bus) {
+        destination_bus->Zero();
+      } else if (rendered_bus != destination_bus) {
+        // in-place processing was not possible - so copy
+        destination_bus->CopyFrom(*rendered_bus);
+      }
+    } else {
+      // Not allowed to pull on the graph or couldn't get the lock.
+      destination_bus->Zero();
+    }
 
-  if (!rendered_bus) {
-    destination_bus->Zero();
-  } else if (rendered_bus != destination_bus) {
-    // in-place processing was not possible - so copy
-    destination_bus->CopyFrom(*rendered_bus);
   }
 
-  // Process nodes which need a little extra help because they are not connected
-  // to anything, but still need to process.
+  // Process nodes which need a little extra help because they are not
+  // connected to anything, but still need to process.
   Context()->GetDeferredTaskHandler().ProcessAutomaticPullNodes(
       number_of_frames);
 
diff --git a/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.cc b/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.cc
index 743cddb2d4b..dd183350f60 100644
--- a/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.cc
+++ b/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.cc
@@ -52,8 +52,7 @@ RealtimeAudioDestinationHandler::RealtimeAudioDestinationHandler(
     base::Optional<float> sample_rate)
     : AudioDestinationHandler(node),
       latency_hint_(latency_hint),
-      sample_rate_(sample_rate),
-      allow_pulling_audio_graph_(false) {
+      sample_rate_(sample_rate) {
   // Node-specific default channel count and mixing rules.
   channel_count_ = 2;
   SetInternalChannelCountMode(kExplicit);
@@ -198,28 +197,38 @@ void RealtimeAudioDestinationHandler::Render(
   // Only pull on the audio graph if we have not stopped the destination.  It
   // takes time for the destination to stop, but we want to stop pulling before
   // the destination has actually stopped.
-  if (IsPullingAudioGraphAllowed()) {
-    // Renders the graph by pulling all the inputs to this node. This will in
-    // turn pull on their inputs, all the way backwards through the graph.
-    scoped_refptr<AudioBus> rendered_bus =
-        Input(0).Pull(destination_bus, number_of_frames);
-
-    DCHECK(rendered_bus);
-    if (!rendered_bus) {
-      // AudioNodeInput might be in the middle of destruction. Then the internal
-      // summing bus will return as nullptr. Then zero out the output.
+  {
+    // The entire block that relies on |IsPullingAudioGraphAllowed| needs
+    // locking to prevent pulling audio graph being disallowed (i.e. a
+    // destruction started) in the middle of processing.
+    MutexTryLocker try_locker(allow_pulling_audio_graph_mutex_);
+
+    if (IsPullingAudioGraphAllowed() && try_locker.Locked()) {
+      // Renders the graph by pulling all the inputs to this node. This will in
+      // turn pull on their inputs, all the way backwards through the graph.
+      scoped_refptr<AudioBus> rendered_bus =
+          Input(0).Pull(destination_bus, number_of_frames);
+
+      DCHECK(rendered_bus);
+      if (!rendered_bus) {
+        // AudioNodeInput might be in the middle of destruction. Then the
+        // internal summing bus will return as nullptr. Then zero out the
+        // output.
+        destination_bus->Zero();
+      } else if (rendered_bus != destination_bus) {
+        // In-place processing was not possible. Copy the rendered result to the
+        // given |destination_bus| buffer.
+        destination_bus->CopyFrom(*rendered_bus);
+      }
+    } else {
+      // Not allowed to pull on the graph or couldn't get the lock.
       destination_bus->Zero();
-    } else if (rendered_bus != destination_bus) {
-      // In-place processing was not possible. Copy the rendered result to the
-      // given |destination_bus| buffer.
-      destination_bus->CopyFrom(*rendered_bus);
     }
-  } else {
-    destination_bus->Zero();
   }
 
-  // Processes "automatic" nodes that are not connected to anything. This can
-  // be done after copying because it does not affect the rendered result.
+  // Processes "automatic" nodes that are not connected to anything. This
+  // can be done after copying because it does not affect the rendered
+  // result.
   context->GetDeferredTaskHandler().ProcessAutomaticPullNodes(number_of_frames);
 
   context->HandlePostRenderTasks();
diff --git a/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.h b/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.h
index 9c8653880c3..89e33ec6a9e 100644
--- a/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.h
+++ b/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_node.h
@@ -81,10 +81,6 @@ class RealtimeAudioDestinationHandler final : public AudioDestinationHandler,
   // Returns a given frames-per-buffer size from audio infra.
   int GetFramesPerBuffer() const;
 
-  bool IsPullingAudioGraphAllowed() const {
-    return allow_pulling_audio_graph_.load(std::memory_order_acquire);
-  }
-
  private:
   explicit RealtimeAudioDestinationHandler(AudioNode&,
                                            const WebAudioLatencyHint&,
@@ -94,32 +90,12 @@ class RealtimeAudioDestinationHandler final : public AudioDestinationHandler,
   void StartPlatformDestination();
   void StopPlatformDestination();
 
-  // Should only be called from StartPlatformDestination.
-  void EnablePullingAudioGraph() {
-    allow_pulling_audio_graph_.store(true, std::memory_order_release);
-  }
-
-  // Should only be called from StopPlatformDestination.
-  void DisablePullingAudioGraph() {
-    allow_pulling_audio_graph_.store(false, std::memory_order_release);
-  }
-
   const WebAudioLatencyHint latency_hint_;
 
   // Holds the audio device thread that runs the real time audio context.
   scoped_refptr<AudioDestination> platform_destination_;
 
   base::Optional<float> sample_rate_;
-
-  // If true, the audio graph will be pulled to get new data.  Otherwise, the
-  // graph is not pulled, even if the audio thread is still running and
-  // requesting data.
-  //
-  // Must be modified only in StartPlatformDestination (via
-  // EnablePullingAudioGraph) or StopPlatformDestination (via
-  // DisablePullingAudioGraph) .  This is modified only by the main threda and
-  // the audio thread only reads this.
-  std::atomic_bool allow_pulling_audio_graph_;
 };
 
 // -----------------------------------------------------------------------------