chore: cherry-pick ecad352cd614 from chromium (#34688)

electron · Jun 29, 2022 · ad23614 · ad23614
1 parent 09ff162
commit ad23614
Show file tree

Hide file tree

Showing 3 changed files with 173 additions and 0 deletions.
diff --git a/patches/chromium/.patches b/patches/chromium/.patches
@@ -147,3 +147,5 @@ cherry-pick-f3d01ff794dc.patch
 cherry-pick-b03797bdb1df.patch
 posix_replace_doubleforkandexec_with_forkandspawn.patch
 cherry-pick-22c61cfae5d1.patch
+post_media_log_destruction_to_avoid_destruction.patch
+add_stop_method_to_batchingmedialog.patch
diff --git a/patches/chromium/add_stop_method_to_batchingmedialog.patch b/patches/chromium/add_stop_method_to_batchingmedialog.patch
@@ -0,0 +1,107 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Ted Meyer <tmathmeyer@chromium.org>
+Date: Wed, 8 Jun 2022 04:33:20 +0000
+Subject: Add Stop method to BatchingMediaLog
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Now that ~MediaLog is posted for a later destruction due to garbage
+collector ownership of CodecLogger, it's possible for the
+SendQueuedMediaEvents call from ~BatchingMediaLog to reference
+InspectorMediaEventHandler::inspector_context_ after it has been freed.
+
+This fix forces BatchingMediaLog to shut down it's logging capabilities
+when the destruction call is caused by the garbage collector deletion
+phase
+
+R=liberato
+
+(cherry picked from commit 1bbfaf23cd8a1e977cb445a82a4caae107632a59)
+
+Bug: 1333333
+Change-Id: I0bdca72a71177c4c5a6a9dc692aad3de4c25f4e2
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3689639
+Commit-Queue: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
+Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
+Cr-Original-Commit-Position: refs/heads/main@{#1011247}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3694435
+Auto-Submit: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
+Reviewed-by: Eugene Zemtsov <ezemtsov@google.com>
+Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
+Cr-Commit-Position: refs/branch-heads/5060@{#672}
+Cr-Branched-From: b83393d0f4038aeaf67f970a024d8101df7348d1-refs/heads/main@{#1002911}
+
+diff --git a/content/renderer/media/batching_media_log.cc b/content/renderer/media/batching_media_log.cc
+index ea5ba3c2e9c876b78f582cb56fb60c42d5a21ffb..148298783fc226bc376d01b152e53532c21b8c99 100644
+--- a/content/renderer/media/batching_media_log.cc
++++ b/content/renderer/media/batching_media_log.cc
+@@ -75,6 +75,11 @@ BatchingMediaLog::~BatchingMediaLog() {
+     SendQueuedMediaEvents();
+ }
+
++void BatchingMediaLog::Stop() {
++  base::AutoLock lock(lock_);
++  event_handlers_.clear();
++}
++
+ void BatchingMediaLog::OnWebMediaPlayerDestroyedLocked() {
+   base::AutoLock lock(lock_);
+   for (const auto& handler : event_handlers_)
+diff --git a/content/renderer/media/batching_media_log.h b/content/renderer/media/batching_media_log.h
+index eb757a4ca922bd5456d0e436d10cc540ee9134ae..6c2df688caee4d2fc17e973cdad12f20342e56fe 100644
+--- a/content/renderer/media/batching_media_log.h
++++ b/content/renderer/media/batching_media_log.h
+@@ -45,6 +45,8 @@ class CONTENT_EXPORT BatchingMediaLog : public media::MediaLog {
+
+   ~BatchingMediaLog() override;
+
++  void Stop() override;
++
+   // Will reset |last_ipc_send_time_| with the value of NowTicks().
+   void SetTickClockForTesting(const base::TickClock* tick_clock);
+
+diff --git a/media/base/media_log.cc b/media/base/media_log.cc
+index 6cb08ed64cb3fe9e6910e4c24c7f0280b050fc15..e3f8f22fed1c12f3466c3ebaa42a67ac1009f6b1 100644
+--- a/media/base/media_log.cc
++++ b/media/base/media_log.cc
+@@ -48,6 +48,9 @@ std::string MediaLog::GetErrorMessageLocked() {
+   return "";
+ }
+
++// Default implementation.
++void MediaLog::Stop() {}
++
+ void MediaLog::AddMessage(MediaLogMessageLevel level, std::string message) {
+   std::unique_ptr<MediaLogRecord> record(
+       CreateRecord(MediaLogRecord::Type::kMessage));
+diff --git a/media/base/media_log.h b/media/base/media_log.h
+index b6cf2bf451650509dbef318f07d04a4679bb257d..f8d3f481abdd410bbaefbc007a0f4fa79fe426cb 100644
+--- a/media/base/media_log.h
++++ b/media/base/media_log.h
+@@ -126,6 +126,10 @@ class MEDIA_EXPORT MediaLog {
+   // even if this occurs, in the "won't crash" sense.
+   virtual std::unique_ptr<MediaLog> Clone();
+
++  // Can be used for stopping a MediaLog during a garbage-collected destruction
++  // sequence.
++  virtual void Stop();
++
+  protected:
+   // Ensures only subclasses and factories (e.g. Clone()) can create MediaLog.
+   MediaLog();
+diff --git a/third_party/blink/renderer/modules/webcodecs/codec_logger.cc b/third_party/blink/renderer/modules/webcodecs/codec_logger.cc
+index 32698e3778ca40b2e0ef26f020269f9a2b0f9cb9..c9775d3da9ad4f0513a341f6fb2c56d0a337e2ee 100644
+--- a/third_party/blink/renderer/modules/webcodecs/codec_logger.cc
++++ b/third_party/blink/renderer/modules/webcodecs/codec_logger.cc
+@@ -70,7 +70,9 @@ CodecLogger::~CodecLogger() {
+   // media logs must be posted for destruction, since they can cause the
+   // garbage collector to trigger an immediate cleanup and delete the owning
+   // instance of |CodecLogger|.
+-  task_runner_->DeleteSoon(FROM_HERE, std::move(parent_media_log_));
++  if (parent_media_log_) {
++    task_runner_->DeleteSoon(FROM_HERE, std::move(parent_media_log_));
++  }
+ }
+
+ void CodecLogger::Neuter() {
diff --git a/patches/chromium/post_media_log_destruction_to_avoid_destruction.patch b/patches/chromium/post_media_log_destruction_to_avoid_destruction.patch
@@ -0,0 +1,64 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Rose <japthorp@slack-corp.com>
+Date: Tue, 28 Jun 2022 14:36:57 -0700
+Subject: Post media log destruction to avoid destruction
+
+SendQueuedMediaEvents is able to tickle oilpan just enough to cause
+the owning BatchingMediaLog to be destroyed in the middle of executing,
+causing a UAF.
+
+(cherry picked from commit 57e905d0943695fb96a1a1a251382d15a9b2fee1)
+
+Bug: 1317714
+Change-Id: Iac2f32aee70eee183be279b372beb2ff39e6c5a0
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3682060
+Reviewed-by: Frank Liberato <liberato@chromium.org>
+Auto-Submit: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
+Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
+Commit-Queue: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
+Cr-Original-Commit-Position: refs/heads/main@{#1009670}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3691325
+Reviewed-by: Dan Sanders <sandersd@chromium.org>
+Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
+Commit-Queue: Dan Sanders <sandersd@chromium.org>
+Cr-Commit-Position: refs/branch-heads/5005@{#1126}
+Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738}
+
+diff --git a/third_party/blink/renderer/modules/webcodecs/codec_logger.cc b/third_party/blink/renderer/modules/webcodecs/codec_logger.cc
+index 257f84f195b4d637445c58bf92a82a3a83836d84..32698e3778ca40b2e0ef26f020269f9a2b0f9cb9 100644
+--- a/third_party/blink/renderer/modules/webcodecs/codec_logger.cc
++++ b/third_party/blink/renderer/modules/webcodecs/codec_logger.cc
+@@ -37,6 +37,8 @@ CodecLogger::CodecLogger(
+   // This allows us to destroy |parent_media_log_| and stop logging,
+   // without causing problems to |media_log_| users.
+   media_log_ = parent_media_log_->Clone();
++
++  task_runner_ = task_runner;
+ }
+
+ DOMException* CodecLogger::MakeException(std::string error_msg,
+@@ -65,6 +67,10 @@ DOMException* CodecLogger::MakeException(std::string error_msg,
+
+ CodecLogger::~CodecLogger() {
+   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
++  // media logs must be posted for destruction, since they can cause the
++  // garbage collector to trigger an immediate cleanup and delete the owning
++  // instance of |CodecLogger|.
++  task_runner_->DeleteSoon(FROM_HERE, std::move(parent_media_log_));
+ }
+
+ void CodecLogger::Neuter() {
+diff --git a/third_party/blink/renderer/modules/webcodecs/codec_logger.h b/third_party/blink/renderer/modules/webcodecs/codec_logger.h
+index 0329c6e6ef9fa05524a685fb46b274f811672185..843b7b727cf3d30282c438ad83f82277d8849ae7 100644
+--- a/third_party/blink/renderer/modules/webcodecs/codec_logger.h
++++ b/third_party/blink/renderer/modules/webcodecs/codec_logger.h
+@@ -74,6 +74,9 @@ class MODULES_EXPORT CodecLogger final {
+   // can be safely accessed, and whose raw pointer can be given callbacks.
+   std::unique_ptr<media::MediaLog> media_log_;
+
++  // Keep task runner around for posting the media log to upon destruction.
++  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
++
+   SEQUENCE_CHECKER(sequence_checker_);
+ };
+