From d798f3bad2546fb2ec8d75c0fbdcfb866c4fb442 Mon Sep 17 00:00:00 2001
From: Saifuddin Hitawala <hitawala@chromium.org>
Date: Wed, 15 Jul 2026 12:34:32 -0700
Subject: [PATCH] [viz] Harden SurfaceManager recursive commit to avoid re-entrancy

Harden SurfaceManager and Surface for committing frames recursively
by passing SurfaceRange by value instead of ref and iterating over
a copy of vectors so that we can avoid potential UAF through
re-entrancy.

Bug: 534858939
Change-Id: I2ec6ce6752eb540fa290dfd13f717d087e4115ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8096884
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1662786}
---

diff --git a/components/viz/service/surfaces/surface.cc b/components/viz/service/surfaces/surface.cc
index 0abab86..ae6755d 100644
--- a/components/viz/service/surfaces/surface.cc
+++ b/components/viz/service/surfaces/surface.cc
@@ -534,13 +534,19 @@
   }
 
   if (HasPendingFrame()) {
-    for (auto& range : pending_frame_data_->frame.metadata.referenced_surfaces)
+    const std::vector<SurfaceRange> referenced_surfaces =
+        pending_frame_data_->frame.metadata.referenced_surfaces;
+    for (auto& range : referenced_surfaces) {
       surface_manager_->CommitFramesInRangeRecursively(range, predicate);
+    }
   }
 
   if (HasActiveFrame()) {
-    for (auto& range : active_frame_data_->frame.metadata.referenced_surfaces)
+    const std::vector<SurfaceRange> referenced_surfaces =
+        active_frame_data_->frame.metadata.referenced_surfaces;
+    for (auto& range : referenced_surfaces) {
       surface_manager_->CommitFramesInRangeRecursively(range, predicate);
+    }
   }
 
   // If we freed up some space in queue send ack for the last frame if it's
diff --git a/components/viz/service/surfaces/surface_manager.cc b/components/viz/service/surfaces/surface_manager.cc
index 43e7b1f..9d1c97c 100644
--- a/components/viz/service/surfaces/surface_manager.cc
+++ b/components/viz/service/surfaces/surface_manager.cc
@@ -757,7 +757,7 @@
 }
 
 void SurfaceManager::CommitFramesInRangeRecursively(
-    const SurfaceRange& range,
+    SurfaceRange range,
     const CommitPredicate& predicate) {
   // Technically we need only latest active surface, but because activation will
   // happen during commit, it's impossible to predict which one will be active,
diff --git a/components/viz/service/surfaces/surface_manager.h b/components/viz/service/surfaces/surface_manager.h
index 117ce59..13c62e7 100644
--- a/components/viz/service/surfaces/surface_manager.h
+++ b/components/viz/service/surfaces/surface_manager.h
@@ -227,7 +227,11 @@
   // surface processed calls `predicate` for each uncommitted frame from oldest
   // to newest. If predicate returns true, surface is committed. If not the
   // surface processing stops and we go to the next surface.
-  void CommitFramesInRangeRecursively(const SurfaceRange& range,
+  // |range| is passed by value because CommitFramesRecursively can
+  // synchronously activate a caller's pending frame, replacing
+  // active_frame_data_ and freeing the referenced_surfaces vector that the
+  // caller passed |range| out of.
+  void CommitFramesInRangeRecursively(SurfaceRange range,
                                       const CommitPredicate& predicate);
 
  private: