From 3525521ad375fc4a0bf4b95b427d2f1dd0f016d2 Mon Sep 17 00:00:00 2001 From: Corentin Pescheloche Date: Tue, 10 May 2022 14:05:53 +0000 Subject: [Backport] CVE-2022-1636: Use after free in Performance APIs Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/3620956: Cleanup profiler group detached profilers ProfilerGroup keeps track of detached profilers to be able to gracefully stop leaked profilers when the corresponding ExecutionContext is destroyed. (cherry picked from commit 9f9d5fd2f3085414fc8776bf556fb5c4fa2dac2c) Change-Id: I4fdbbc3a5208819397d742c9ecbff117f839691c Bug: chromium:1297283 Commit-Queue: Corentin Pescheloche Cr-Original-Commit-Position: refs/heads/main@{#994316} Reviewed-by: Oleh Lamzin Owners-Override: Oleh Lamzin Commit-Queue: Roger Felipe Zanoni da Silva Auto-Submit: Roger Felipe Zanoni da Silva Cr-Commit-Position: refs/branch-heads/4664@{#1629} Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512} Reviewed-by: Allan Sandfeld Jensen --- .../blink/renderer/core/timing/profiler_group.cc | 38 ++++++++++++++++++++-- .../blink/renderer/core/timing/profiler_group.h | 8 ++++- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/chromium/third_party/blink/renderer/core/timing/profiler_group.cc b/chromium/third_party/blink/renderer/core/timing/profiler_group.cc index 790ebdc3311..97c372ba878 100644 --- a/chromium/third_party/blink/renderer/core/timing/profiler_group.cc +++ b/chromium/third_party/blink/renderer/core/timing/profiler_group.cc @@ -57,8 +57,7 @@ ProfilerGroup::ProfilerGroup(v8::Isolate* isolate) : isolate_(isolate), cpu_profiler_(nullptr), next_profiler_id_(0), - num_active_profilers_(0) { -} + num_active_profilers_(0) {} Profiler* ProfilerGroup::CreateProfiler(ScriptState* script_state, const ProfilerInitOptions& init_options, @@ -151,6 +150,8 @@ void ProfilerGroup::WillBeDestroyed() { DCHECK(!profilers_.Contains(profiler)); } + StopDetachedProfilers(); + if (cpu_profiler_) TeardownV8Profiler(); } @@ -213,18 +214,49 @@ void ProfilerGroup::CancelProfiler(Profiler* profiler) { void ProfilerGroup::CancelProfilerAsync(ScriptState* script_state, Profiler* profiler) { + DCHECK(IsMainThread()); DCHECK(cpu_profiler_); DCHECK(!profiler->stopped()); profilers_.erase(profiler); + // register the profiler to be cleaned up in case its associated context + // gets destroyed before the cleanup task is executed. + detached_profiler_ids_.push_back(profiler->ProfilerId()); + // Since it's possible for the profiler to get destructed along with its // associated context, dispatch a task to cleanup context-independent isolate // resources (rather than use the context's task runner). ThreadScheduler::Current()->V8TaskRunner()->PostTask( - FROM_HERE, WTF::Bind(&ProfilerGroup::CancelProfilerImpl, + FROM_HERE, WTF::Bind(&ProfilerGroup::StopDetachedProfiler, WrapPersistent(this), profiler->ProfilerId())); } +void ProfilerGroup::StopDetachedProfiler(String profiler_id) { + DCHECK(IsMainThread()); + + // we use a vector instead of a map because the expected number of profiler + // is expected to be very small + auto* it = std::find(detached_profiler_ids_.begin(), + detached_profiler_ids_.end(), profiler_id); + + if (it == detached_profiler_ids_.end()) { + // Profiler already stopped + return; + } + + CancelProfilerImpl(profiler_id); + detached_profiler_ids_.erase(it); +} + +void ProfilerGroup::StopDetachedProfilers() { + DCHECK(IsMainThread()); + + for (auto& detached_profiler_id : detached_profiler_ids_) { + CancelProfilerImpl(detached_profiler_id); + } + detached_profiler_ids_.clear(); +} + void ProfilerGroup::CancelProfilerImpl(String profiler_id) { if (!cpu_profiler_) return; diff --git a/chromium/third_party/blink/renderer/core/timing/profiler_group.h b/chromium/third_party/blink/renderer/core/timing/profiler_group.h index da540b773de..d201ae0d0c1 100644 --- a/chromium/third_party/blink/renderer/core/timing/profiler_group.h +++ b/chromium/third_party/blink/renderer/core/timing/profiler_group.h @@ -58,6 +58,10 @@ class CORE_EXPORT ProfilerGroup // Internal implementation of cancel. void CancelProfilerImpl(String profiler_id); + // Clean context independent resources for leaked profilers + void StopDetachedProfiler(String profiler_id); + void StopDetachedProfilers(); + // Generates an unused string identifier to use for a new profiling session. String NextProfilerId(); @@ -65,9 +69,11 @@ class CORE_EXPORT ProfilerGroup v8::CpuProfiler* cpu_profiler_; int next_profiler_id_; int num_active_profilers_; - HeapHashSet> profilers_; + // Store the ids of leaked collected profilers that needs to be stopped + Vector detached_profiler_ids_; + DISALLOW_COPY_AND_ASSIGN(ProfilerGroup); }; -- cgit v1.2.1