From a833fdedacd97fdb48c7c849e43c2d487cdf7ed7 Mon Sep 17 00:00:00 2001
From: Sangbaek Park <sangbaekpark@google.com>
Date: Wed, 01 Apr 2026 17:36:52 -0700
Subject: [PATCH] media: Fix cross-thread data race and deadlock in stream wrapper

This CL fixes a potential Use-After-Free (UAF) vulnerability caused by
a cross-thread data race in MediaFoundationStreamWrapper, as well as a
subsequent self-deadlock issue introduced by the initial synchronization
attempt.

Previously, `demuxer_stream_` was accessed by Media Foundation
threadpool threads via `IsEncrypted()` without synchronization.
Concurrently, the Chromium media thread could null it out during
teardown in `DetachDemuxerStream()`. This race condition could bypass
BackupRefPtr protections and lead to a UAF when dereferencing the
pointer to check the decoder config.

To fix this, `demuxer_stream_` accesses are now safely protected by
`lock_`. To avoid non-recursive mutex self-deadlocks when internal
paths (like `OnDemuxerStreamRead` -> `ReportEncryptionType`) check the
stored encryption state instead of lock since the encrypted state won't
change.

Bug: 497985088
Change-Id: Ibf5168a8f65a9eb7d80da55165866d0c8721953c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7719604
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Sangbaek Park <sangbaekpark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1608895}
---

diff --git a/media/renderers/win/media_foundation_audio_stream.cc b/media/renderers/win/media_foundation_audio_stream.cc
index 2e8f1b3..59ff57e07 100644
--- a/media/renderers/win/media_foundation_audio_stream.cc
+++ b/media/renderers/win/media_foundation_audio_stream.cc
@@ -57,11 +57,6 @@
   return S_OK;
 }
 
-bool MediaFoundationAudioStream::IsEncrypted() const {
-  AudioDecoderConfig audio_config = demuxer_stream_->audio_decoder_config();
-  return audio_config.is_encrypted();
-}
-
 HRESULT MediaFoundationAudioStream::GetMediaType(
     IMFMediaType** media_type_out) {
   AudioDecoderConfig decoder_config = demuxer_stream_->audio_decoder_config();
diff --git a/media/renderers/win/media_foundation_audio_stream.h b/media/renderers/win/media_foundation_audio_stream.h
index 2608074..25affcb2 100644
--- a/media/renderers/win/media_foundation_audio_stream.h
+++ b/media/renderers/win/media_foundation_audio_stream.h
@@ -25,7 +25,6 @@
                         DemuxerStream* demuxer_stream,
                         std::unique_ptr<MediaLog> media_log,
                         MediaFoundationStreamWrapper** stream_out);
-  bool IsEncrypted() const override;
   HRESULT GetMediaType(IMFMediaType** media_type_out) override;
 };
 
diff --git a/media/renderers/win/media_foundation_stream_wrapper.cc b/media/renderers/win/media_foundation_stream_wrapper.cc
index fd7189e6..18bb9c19 100644
--- a/media/renderers/win/media_foundation_stream_wrapper.cc
+++ b/media/renderers/win/media_foundation_stream_wrapper.cc
@@ -79,13 +79,17 @@
   {
     base::AutoLock auto_lock(lock_);
     parent_source_ = parent_source;
+    demuxer_stream_ = demuxer_stream;
   }
-  demuxer_stream_ = demuxer_stream;
   stream_id_ = stream_id;
-  stream_type_ = demuxer_stream_->type();
+  stream_type_ = demuxer_stream->type();
+  is_encrypted_ = (stream_type_ == DemuxerStream::Type::VIDEO)
+                      ? demuxer_stream->video_decoder_config().is_encrypted()
+                      : demuxer_stream->audio_decoder_config().is_encrypted();
 
   DVLOG_FUNC(1) << "stream_id=" << stream_id
-                << ", stream_type=" << DemuxerStream::GetTypeName(stream_type_);
+                << ", stream_type=" << DemuxerStream::GetTypeName(stream_type_)
+                << ", is_encrypted=" << is_encrypted_;
 
   media_log_ = std::move(media_log);
   if (base::FeatureList::IsEnabled(kMediaFoundationBatchRead)) {
@@ -127,6 +131,7 @@
   DVLOG_FUNC(1);
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
+  base::AutoLock auto_lock(lock_);
   demuxer_stream_ = nullptr;
 }
 
@@ -643,6 +648,10 @@
   return true;
 }
 
+bool MediaFoundationStreamWrapper::IsEncrypted() const {
+  return is_encrypted_;
+}
+
 GUID MediaFoundationStreamWrapper::GetLastKeyId() const {
   return last_key_id_;
 }
diff --git a/media/renderers/win/media_foundation_stream_wrapper.h b/media/renderers/win/media_foundation_stream_wrapper.h
index 39c4956..a60e60ff 100644
--- a/media/renderers/win/media_foundation_stream_wrapper.h
+++ b/media/renderers/win/media_foundation_stream_wrapper.h
@@ -79,7 +79,7 @@
   // TODO: revisting inheritance and potentially replacing it with composition.
 
   // The stream is encrypted or not.
-  virtual bool IsEncrypted() const = 0;
+  bool IsEncrypted() const;
   // Let derived class to adjust the IMFSample if necessary.
   virtual HRESULT TransformSample(Microsoft::WRL::ComPtr<IMFSample>& sample);
   // Allow derived class to tell us if we can send MEStreamFormatChanged to MF.
@@ -145,7 +145,7 @@
 
   // Need exclusive access to some members between calls from MF threadpool
   // thread and calling thread from Chromium media stack.
-  base::Lock lock_;
+  mutable base::Lock lock_;
 
   // Indicates whether the stream is selected in the MF pipeline.
   bool selected_ GUARDED_BY(lock_) = false;
@@ -209,6 +209,8 @@
 
   bool encryption_type_reported_ = false;
 
+  bool is_encrypted_ = false;
+
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<MediaFoundationStreamWrapper> weak_factory_{this};
 };
diff --git a/media/renderers/win/media_foundation_video_stream.cc b/media/renderers/win/media_foundation_video_stream.cc
index 4f48ea66..1bb7d27 100644
--- a/media/renderers/win/media_foundation_video_stream.cc
+++ b/media/renderers/win/media_foundation_video_stream.cc
@@ -352,11 +352,6 @@
   return S_OK;
 }
 
-bool MediaFoundationVideoStream::IsEncrypted() const {
-  VideoDecoderConfig decoder_config = demuxer_stream_->video_decoder_config();
-  return decoder_config.is_encrypted();
-}
-
 HRESULT MediaFoundationVideoStream::GetMediaType(
     IMFMediaType** media_type_out) {
   VideoDecoderConfig decoder_config = demuxer_stream_->video_decoder_config();
diff --git a/media/renderers/win/media_foundation_video_stream.h b/media/renderers/win/media_foundation_video_stream.h
index a356c03..dceb1ca 100644
--- a/media/renderers/win/media_foundation_video_stream.h
+++ b/media/renderers/win/media_foundation_video_stream.h
@@ -23,8 +23,6 @@
                         std::unique_ptr<MediaLog> media_log,
                         MediaFoundationStreamWrapper** stream_out);
 
-  bool IsEncrypted() const override;
-
  protected:
   HRESULT GetMediaType(IMFMediaType** media_type_out) override;
 };