[Backport] Security bug 1029865

Merge M80: "Neuter DefaultDecoderFactory after MediaFactory destruction."

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/1990200

Since we started allowing asynchronous stop of media::PipelineImpl, we
have released the DefaultDecoderFactory on the media thread assuming it
was safe to continue usage after ~MediaFactory. It turns out this is not
the case for the MojoDecoderFactory used by DefaultDecoderFactory.

To fix, we now neuter the DefaultDecoderFactory in ~MediaFactory to
prevent decoders from being created. This is done under a lock since the
decoder creation methods are called from the media thread and
~MediaFactory happens on the render thread at time of ~RenderFrame.

TBR=<U+200B>xhwang

(cherry picked from commit 7b100c28d219f682763522ed0a3e30e231c1176b)

Fixed: 1029865
Change-Id: I3cde99102863565c05f9da042f27eaac982bcc28
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>

4 files changed, 30 insertions, 2 deletions

diff --git a/chromium/content/renderer/media/media_factory.cc b/chromium/content/renderer/media/media_factory.cc
index 8228992bfde..bc800288557 100644
--- a/chromium/content/renderer/media/media_factory.cc
+++ b/chromium/content/renderer/media/media_factory.cc
@@ -169,6 +169,11 @@ MediaFactory::~MediaFactory() {
   // new tasks using the DecoderFactory will execute, so we don't need to worry
   // about additional posted tasks from Stop().
   if (decoder_factory_) {
+    // Prevent any new decoders from being created to avoid future access to the
+    // external decoder factory (MojoDecoderFactory) since it requires access to
+    // the (about to be destructed) RenderFrame.
+    decoder_factory_->Shutdown();
+
     // DeleteSoon() shouldn't ever fail, we should always have a RenderThread at
     // this time and subsequently a media thread. To fail, the media thread must
     // be dead/dying (which only happens at ~RenderThreadImpl), in which case
diff --git a/chromium/content/renderer/media/media_factory.h b/chromium/content/renderer/media/media_factory.h
index 5c3032076be..4ae50195e76 100644
--- a/chromium/content/renderer/media/media_factory.h
+++ b/chromium/content/renderer/media/media_factory.h
@@ -43,6 +43,7 @@ class LayerTreeSettings;
 namespace media {
 class CdmFactory;
 class DecoderFactory;
+class DefaultDecoderFactory;
 class MediaLog;
 class MediaObserver;
 class RemotePlaybackClientWrapper;
@@ -173,7 +174,7 @@ class MediaFactory {
   media::RendererWebMediaPlayerDelegate* media_player_delegate_ = nullptr;
 
   // The CDM and decoder factory attached to this frame, lazily initialized.
-  std::unique_ptr<media::DecoderFactory> decoder_factory_;
+  std::unique_ptr<media::DefaultDecoderFactory> decoder_factory_;
   std::unique_ptr<media::CdmFactory> cdm_factory_;
 
   // Media resource cache, lazily initialized.
diff --git a/chromium/media/renderers/default_decoder_factory.cc b/chromium/media/renderers/default_decoder_factory.cc
index 755b06c9c0c..394a64b6f58 100644
--- a/chromium/media/renderers/default_decoder_factory.cc
+++ b/chromium/media/renderers/default_decoder_factory.cc
@@ -57,6 +57,10 @@ void DefaultDecoderFactory::CreateAudioDecoders(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     MediaLog* media_log,
     std::vector<std::unique_ptr<AudioDecoder>>* audio_decoders) {
+  base::AutoLock auto_lock(shutdown_lock_);
+  if (is_shutdown_)
+    return;
+
 #if !defined(OS_ANDROID)
   // DecryptingAudioDecoder is only needed in External Clear Key testing to
   // cover the audio decrypt-and-decode path.
@@ -84,6 +88,10 @@ void DefaultDecoderFactory::CreateVideoDecoders(
     const RequestOverlayInfoCB& request_overlay_info_cb,
     const gfx::ColorSpace& target_color_space,
     std::vector<std::unique_ptr<VideoDecoder>>* video_decoders) {
+  base::AutoLock auto_lock(shutdown_lock_);
+  if (is_shutdown_)
+    return;
+
 #if !defined(OS_ANDROID)
   video_decoders->push_back(
       std::make_unique<DecryptingVideoDecoder>(task_runner, media_log));
@@ -123,4 +131,10 @@ void DefaultDecoderFactory::CreateVideoDecoders(
 #endif
 }
 
+void DefaultDecoderFactory::Shutdown() {
+  base::AutoLock auto_lock(shutdown_lock_);
+  external_decoder_factory_.reset();
+  is_shutdown_ = true;
+}
+
 }  // namespace media
diff --git a/chromium/media/renderers/default_decoder_factory.h b/chromium/media/renderers/default_decoder_factory.h
index 1b1305d3617..cb471dc3fb3 100644
--- a/chromium/media/renderers/default_decoder_factory.h
+++ b/chromium/media/renderers/default_decoder_factory.h
@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "base/synchronization/lock.h"
 #include "media/base/decoder_factory.h"
 #include "media/base/media_export.h"
 
@@ -33,9 +34,16 @@ class MEDIA_EXPORT DefaultDecoderFactory : public DecoderFactory {
       const gfx::ColorSpace& target_color_space,
       std::vector<std::unique_ptr<VideoDecoder>>* video_decoders) final;
 
+  // Called from the renderer thread to prevent any more decoders from being
+  // vended on other threads.
+  void Shutdown();
+
  private:
-  std::unique_ptr<DecoderFactory> external_decoder_factory_;
+  base::Lock shutdown_lock_;
+  bool is_shutdown_ GUARDED_BY(shutdown_lock_) = false;
 
+  std::unique_ptr<DecoderFactory> external_decoder_factory_
+      GUARDED_BY(shutdown_lock_);
   DISALLOW_COPY_AND_ASSIGN(DefaultDecoderFactory);
 };