Skip to content

Commit

Permalink
[Merge M115] [blink] Fix UAF in NonMainThreadTaskQueue
Browse files Browse the repository at this point in the history
The issue is that WorkerThreadScheduler::OnTaskCompleted's
PerformMicrotaskCheckpoint() might result in a blink heap GC which may
collect NonMainThreadWebSchedulingTaskQueueImpl (owned by
GarbageCollected<DOMTaskQueue>) which might own the last ref to
NonMainThreadTaskQueue. If the NonMainThreadTaskQueue is deleted,
there's a UAF in the follow-up call to
task_queue->OnTaskRunTimeReported(task_timing);

Retain a ref to NonMainThreadTaskQueue throughout OnTaskCompleted() to
prevent this.

The other option, proposed @ crbug.com/1464113#c3 was to bind the ref
ahead of time in the `on_task_completed_handler` but I am leery that
this might prevent deleting queues with pending tasks.

R=altimin@chromium.org

(cherry picked from commit 3463ed5)

Bug: 1464113
Change-Id: I877c609244ab90a0af1c87c317cf5a55e2fa60ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4678047
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Auto-Submit: Gabriel Charette <gab@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1170760}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4706693
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#1857}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Gabriel Charette authored and Chromium LUCI CQ committed Jul 27, 2023
1 parent 1a681ed commit b429304
Showing 1 changed file with 4 additions and 0 deletions.
Expand Up @@ -69,6 +69,10 @@ void NonMainThreadTaskQueue::OnTaskCompleted(
base::LazyNow* lazy_now) {
// |non_main_thread_scheduler_| can be nullptr in tests.
if (non_main_thread_scheduler_) {
// The last ref to `non_main_thread_scheduler_` might be released as part of
// this task's cleanup microtasks, make sure it lives through its own
// cleanup: crbug.com/1464113.
auto self_ref = WrapRefCounted(this);
non_main_thread_scheduler_->OnTaskCompleted(this, task, task_timing,
lazy_now);
}
Expand Down

0 comments on commit b429304

Please sign in to comment.