From 41ffa38dcb7f3ce786130a2834c51b00cc0e78a9 Mon Sep 17 00:00:00 2001
From: Sangbaek Park <[email protected]>
Date: Tue, 28 Apr 2026 11:59:07 -0700
Subject: [PATCH] media: Fix cross-thread use-after-free in MediaFoundationStreamWrapper

MediaFoundationStreamWrapper uses default COM reference counting for
lifetime management, allowing it to be destructed on a background MF
thread. This introduces a Time-of-Check to Time-of-Use (TOCTOU) race
condition when executing tasks posted to the Chromium media thread
via base::WeakPtr, potentially leading to a use-after-free.

This CL enforces that MediaFoundationStreamWrapper is always
destroyed on the correct sequenced task runner by overriding Release()
to use DeleteSoon().

The same solution is applied to MediaFoundationSourceWrapper.

Tests: { MediaFoundationStreamWrapperTest.DestructionOnTaskRunner,
MediaFoundationSourceWrapperTest.DestructionOnTaskRunner }

Bug: 503565293
Change-Id: Ib9044672195157f30e29905c8f1565a63c411ab2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7787693
Reviewed-by: Dale Curtis <[email protected]>
Commit-Queue: Sangbaek Park <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1621947}
---

diff --git a/media/renderers/BUILD.gn b/media/renderers/BUILD.gn
index 67642cb1..74361be 100644
--- a/media/renderers/BUILD.gn
+++ b/media/renderers/BUILD.gn
@@ -153,6 +153,7 @@
     sources += [
       "win/media_foundation_renderer_integration_test.cc",
       "win/media_foundation_renderer_unittest.cc",
+      "win/media_foundation_source_wrapper_unittest.cc",
       "win/media_foundation_stream_wrapper_unittest.cc",
     ]
     deps += [ "//media/test:pipeline_integration_test_base" ]
diff --git a/media/renderers/win/media_foundation_source_wrapper.cc b/media/renderers/win/media_foundation_source_wrapper.cc
index 2f7ade0..a06b3d7 100644
--- a/media/renderers/win/media_foundation_source_wrapper.cc
+++ b/media/renderers/win/media_foundation_source_wrapper.cc
@@ -35,6 +35,18 @@
   }
 }
 
+IFACEMETHODIMP_(ULONG) MediaFoundationSourceWrapper::Release() {
+  ULONG ref_count = InternalRelease();
+  if (ref_count == 0) {
+    if (!task_runner_->RunsTasksInCurrentSequence()) {
+      task_runner_->DeleteSoon(FROM_HERE, this);
+    } else {
+      delete this;
+    }
+  }
+  return ref_count;
+}
+
 HRESULT MediaFoundationSourceWrapper::RuntimeClassInitialize(
     MediaResource* media_resource,
     MediaLog* media_log,
diff --git a/media/renderers/win/media_foundation_source_wrapper.h b/media/renderers/win/media_foundation_source_wrapper.h
index 4bc0657..1a14e52e 100644
--- a/media/renderers/win/media_foundation_source_wrapper.h
+++ b/media/renderers/win/media_foundation_source_wrapper.h
@@ -14,6 +14,7 @@
 
 #include "base/memory/scoped_refptr.h"
 #include "base/task/sequenced_task_runner.h"
+#include "media/base/media_export.h"
 #include "media/base/media_resource.h"
 #include "media/base/win/media_foundation_cdm_proxy.h"
 #include "media/renderers/win/media_foundation_stream_wrapper.h"
@@ -31,7 +32,7 @@
 // Note: The methods in this class can be called on two different threads -
 //       Chromium thread and MF threadpool thread.
 //
-class MediaFoundationSourceWrapper
+class MEDIA_EXPORT MediaFoundationSourceWrapper
     : public Microsoft::WRL::RuntimeClass<
           Microsoft::WRL::RuntimeClassFlags<
               Microsoft::WRL::RuntimeClassType::ClassicCom>,
@@ -44,6 +45,8 @@
   MediaFoundationSourceWrapper();
   ~MediaFoundationSourceWrapper() override;
 
+  IFACEMETHODIMP_(ULONG) Release() override;
+
   // This is only called on |task_runner|.
   void DetachResource();
 
diff --git a/media/renderers/win/media_foundation_source_wrapper_unittest.cc b/media/renderers/win/media_foundation_source_wrapper_unittest.cc
new file mode 100644
index 0000000..fd90b344
--- /dev/null
+++ b/media/renderers/win/media_foundation_source_wrapper_unittest.cc
@@ -0,0 +1,74 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+#include "media/renderers/win/media_foundation_source_wrapper.h"
+
+#include <mfapi.h>
+
+#include <memory>
+
+#include "base/synchronization/waitable_event.h"
+#include "base/task/sequenced_task_runner.h"
+#include "base/task/thread_pool.h"
+#include "base/test/task_environment.h"
+#include "media/base/media_util.h"
+#include "media/base/mock_filters.h"
+#include "media/base/test_helpers.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+using Microsoft::WRL::ComPtr;
+using Microsoft::WRL::MakeAndInitialize;
+
+class MediaFoundationSourceWrapperTest : public testing::Test {
+ public:
+  MediaFoundationSourceWrapperTest() {
+    source_wrapper_task_runner_ = base::ThreadPool::CreateSequencedTaskRunner(
+        {base::MayBlock(), base::TaskPriority::BEST_EFFORT});
+
+    null_media_log_ = std::make_unique<NullMediaLog>();
+
+    MakeAndInitialize<MediaFoundationSourceWrapper>(
+        &mf_source_wrapper_, &media_resource_, null_media_log_.get(),
+        source_wrapper_task_runner_);
+  }
+
+  ~MediaFoundationSourceWrapperTest() override {
+    mf_source_wrapper_.Reset();
+    task_environment_.RunUntilIdle();
+  }
+
+ protected:
+  ComPtr<MediaFoundationSourceWrapper> mf_source_wrapper_;
+  base::test::TaskEnvironment task_environment_;
+  scoped_refptr<base::SequencedTaskRunner> source_wrapper_task_runner_;
+  testing::NiceMock<MockMediaResource> media_resource_;
+  std::unique_ptr<MediaLog> null_media_log_;
+};
+
+// Initializes a MediaFoundationSourceWrapper inside a simulated task runner
+// environment, then manually triggers its destruction from a background thread
+// to verify that it successfully bounces the destruction back to the sequenced
+// task runner.
+TEST_F(MediaFoundationSourceWrapperTest, DestructionOnTaskRunner) {
+  auto wrapper = mf_source_wrapper_;
+  mf_source_wrapper_.Reset();
+
+  base::WaitableEvent event;
+  base::ThreadPool::PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](ComPtr<MediaFoundationSourceWrapper> wrapper,
+                        base::WaitableEvent* event) {
+                       wrapper.Reset();
+                       event->Signal();
+                     },
+                     std::move(wrapper), &event));
+  event.Wait();
+
+  // Wait for the task runner to process the deletion.
+  task_environment_.RunUntilIdle();
+}
+
+}  // namespace media
diff --git a/media/renderers/win/media_foundation_stream_wrapper.cc b/media/renderers/win/media_foundation_stream_wrapper.cc
index 18bb9c19..3633ea79b9 100644
--- a/media/renderers/win/media_foundation_stream_wrapper.cc
+++ b/media/renderers/win/media_foundation_stream_wrapper.cc
@@ -39,6 +39,18 @@
 MediaFoundationStreamWrapper::MediaFoundationStreamWrapper() = default;
 MediaFoundationStreamWrapper::~MediaFoundationStreamWrapper() = default;
 
+IFACEMETHODIMP_(ULONG) MediaFoundationStreamWrapper::Release() {
+  ULONG ref_count = InternalRelease();
+  if (ref_count == 0) {
+    if (!task_runner_->RunsTasksInCurrentSequence()) {
+      task_runner_->DeleteSoon(FROM_HERE, this);
+    } else {
+      delete this;
+    }
+  }
+  return ref_count;
+}
+
 /*static*/
 HRESULT MediaFoundationStreamWrapper::Create(
     int stream_id,
diff --git a/media/renderers/win/media_foundation_stream_wrapper.h b/media/renderers/win/media_foundation_stream_wrapper.h
index a60e60ff..5a4d44d 100644
--- a/media/renderers/win/media_foundation_stream_wrapper.h
+++ b/media/renderers/win/media_foundation_stream_wrapper.h

diff --git a/media/renderers/win/media_foundation_source_wrapper_unittest.cc b/media/renderers/win/media_foundation_source_wrapper_unittest.cc
new file mode 100644
index 0000000..fd90b344
--- /dev/null
+++ b/media/renderers/win/media_foundation_source_wrapper_unittest.cc
@@ -0,0 +1,74 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+#include "media/renderers/win/media_foundation_source_wrapper.h"
+
+#include <mfapi.h>
+
+#include <memory>
+
+#include "base/synchronization/waitable_event.h"
+#include "base/task/sequenced_task_runner.h"
+#include "base/task/thread_pool.h"
+#include "base/test/task_environment.h"
+#include "media/base/media_util.h"
+#include "media/base/mock_filters.h"
+#include "media/base/test_helpers.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+using Microsoft::WRL::ComPtr;
+using Microsoft::WRL::MakeAndInitialize;
+
+class MediaFoundationSourceWrapperTest : public testing::Test {
+ public:
+  MediaFoundationSourceWrapperTest() {
+    source_wrapper_task_runner_ = base::ThreadPool::CreateSequencedTaskRunner(
+        {base::MayBlock(), base::TaskPriority::BEST_EFFORT});
+
+    null_media_log_ = std::make_unique<NullMediaLog>();
+
+    MakeAndInitialize<MediaFoundationSourceWrapper>(
+        &mf_source_wrapper_, &media_resource_, null_media_log_.get(),
+        source_wrapper_task_runner_);
+  }
+
+  ~MediaFoundationSourceWrapperTest() override {
+    mf_source_wrapper_.Reset();
+    task_environment_.RunUntilIdle();
+  }
+
+ protected:
+  ComPtr<MediaFoundationSourceWrapper> mf_source_wrapper_;
+  base::test::TaskEnvironment task_environment_;
+  scoped_refptr<base::SequencedTaskRunner> source_wrapper_task_runner_;
+  testing::NiceMock<MockMediaResource> media_resource_;
+  std::unique_ptr<MediaLog> null_media_log_;
+};
+
+// Initializes a MediaFoundationSourceWrapper inside a simulated task runner
+// environment, then manually triggers its destruction from a background thread
+// to verify that it successfully bounces the destruction back to the sequenced
+// task runner.
+TEST_F(MediaFoundationSourceWrapperTest, DestructionOnTaskRunner) {
+  auto wrapper = mf_source_wrapper_;
+  mf_source_wrapper_.Reset();
+
+  base::WaitableEvent event;
+  base::ThreadPool::PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](ComPtr<MediaFoundationSourceWrapper> wrapper,
+                        base::WaitableEvent* event) {
+                       wrapper.Reset();
+                       event->Signal();
+                     },
+                     std::move(wrapper), &event));
+  event.Wait();
+
+  // Wait for the task runner to process the deletion.
+  task_environment_.RunUntilIdle();
+}
+
+}  // namespace media
diff --git a/media/renderers/win/media_foundation_stream_wrapper_unittest.cc b/media/renderers/win/media_foundation_stream_wrapper_unittest.cc
index 38bbc4e..0b317d75 100644
--- a/media/renderers/win/media_foundation_stream_wrapper_unittest.cc
+++ b/media/renderers/win/media_foundation_stream_wrapper_unittest.cc
@@ -86,6 +86,7 @@
 
   ~MediaFoundationStreamWrapperTest() override {
     mf_video_stream_wrapper_.Reset();
+    task_environment_.RunUntilIdle();
   }
 
   // Polls the MediaFoundationStreamWrapper object for queued media events.
@@ -342,4 +343,27 @@
   VerifyBufferedPostFlushSamplesProcessed(/*startEvent*/ false);
 }
 
+// Initializes a MediaFoundationStreamWrapper inside a simulated task runner
+// environment, then manually triggers its destruction from a background thread
+// to verify that it successfully bounces the destruction back to the sequenced
+// task runner.
+TEST_F(MediaFoundationStreamWrapperTest, DestructionOnTaskRunner) {
+  auto wrapper = mf_video_stream_wrapper_;
+  mf_video_stream_wrapper_.Reset();
+
+  base::WaitableEvent event;
+  base::ThreadPool::PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](ComPtr<MediaFoundationStreamWrapper> wrapper,
+                        base::WaitableEvent* event) {
+                       wrapper.Reset();
+                       event->Signal();
+                     },
+                     std::move(wrapper), &event));
+  event.Wait();
+
+  // Wait for the task runner to process the deletion.
+  task_environment_.RunUntilIdle();
+}
+
 }  // namespace media