From ad23614bb8a4a08bff2467a64b106d57d87e270c Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 29 Jun 2022 13:07:19 -0700 Subject: [PATCH] chore: cherry-pick ecad352cd614 from chromium (#34688) --- patches/chromium/.patches | 2 + .../add_stop_method_to_batchingmedialog.patch | 107 ++++++++++++++++++ ...log_destruction_to_avoid_destruction.patch | 64 +++++++++++ 3 files changed, 173 insertions(+) create mode 100644 patches/chromium/add_stop_method_to_batchingmedialog.patch create mode 100644 patches/chromium/post_media_log_destruction_to_avoid_destruction.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 7f286478bca28..cc251fe0d627b 100644 --- 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 new file mode 100644 index 0000000000000..5a7421c93a0ba --- /dev/null +++ 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 +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 +Reviewed-by: Eugene Zemtsov +Cr-Original-Commit-Position: refs/heads/main@{#1011247} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3694435 +Auto-Submit: Ted (Chromium) Meyer +Reviewed-by: Eugene Zemtsov +Commit-Queue: Eugene Zemtsov +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 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 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 new file mode 100644 index 0000000000000..c3fe371b425eb --- /dev/null +++ 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 +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 +Auto-Submit: Ted (Chromium) Meyer +Reviewed-by: Thomas Guilbert +Commit-Queue: Ted (Chromium) Meyer +Cr-Original-Commit-Position: refs/heads/main@{#1009670} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3691325 +Reviewed-by: Dan Sanders +Bot-Commit: Rubber Stamper +Commit-Queue: Dan Sanders +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_log_; + ++ // Keep task runner around for posting the media log to upon destruction. ++ scoped_refptr task_runner_; ++ + SEQUENCE_CHECKER(sequence_checker_); + }; +