High chrome Race 🔧 Commit mapped

Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactImproper input validation in Compositing
DescriptionImproper input validation in Compositing
ComponentCompositing
Bug ClassRace
Tracker574030524
Fix commit30e4d30f53b3 (chromium/src) +21/-25
CISA KEVNot listed
Creditede7be95afd6e4ad8d8abc6076c284c79db9e35cc2
Disclosed2026-10-27

Background

ImageContextImpl / ExternalUseClient::ImageContext
A viz display-compositor object (components/viz/service/display/external_use_client.h) that wraps a gpu::Mailbox-backed resource and caches a promise sk_sp<SkImage> (the image_ member) used by Skia on the GPU thread.
SkiaOutputSurfaceImpl
Runs on the viz/compositor thread (components/viz/service/display_embedder/skia_output_surface_impl.cc); it builds the paint ops and hands ImageContextImpls to the GPU thread, where SkImages are consumed.
viz thread vs GPU thread
The compositor (“viz”) thread creates/populates ImageContext; the GPU thread reads the SkImage. The documented thread-safety contract is: only the viz thread modifies ImageContext, it is not modified after image_ is set, and the GPU thread only reads after image_ is set.
render_pass_image_cache_
A per-render-pass cache of ImageContextImpls on SkiaOutputSurfaceImpl; entries are reused across frames and only reallocated when the backing changes.
sk_sp<SkImage>
A reference-counted Skia smart pointer; image_.reset() drops this reference. If another thread is concurrently using the same SkImage, resetting it races on the shared refcount/backing.

Root Cause Analysis

The advisory labels this “Improper input validation in Compositing,” but the diff substantiates a data race / thread-safety violation, not input validation — this discrepancy is worth calling out. The patch enforces the stated cross-thread invariant that an ImageContext’s SkImage and texture info must never be mutated after they are set on the viz thread, because the GPU thread reads them concurrently. Previously SkiaOutputSurfaceImpl called ImageContextImpl::clear_image() (which does image_.reset()) from two viz-thread sites where the GPU thread could be simultaneously accessing the same SkImage. First, it reset the image just before shipping the ImageContextImpl to the GPU main thread for deletion — unnecessary, and racy. Second, in the non-overlay render-pass draw path it reset the cached render-pass SkImage on every draw “in case the backing changed,” again on the viz thread while the GPU thread may be reading it.

The fix removes both clear_image() call sites: the pre-send reset in the render-pass setup path, and the per-draw reset guarded by if (!is_overlay). It relies instead on SkiaRenderer::UpdateRenderPassTextures() already calling RemoveRenderPassResource() (which deletes the whole ImageContextImpl, so a fresh context and SkImage are created when a backing actually changes). To make the invariant enforceable, clear_image() is moved from public to protected (now only legitimate during destruction, when the context is no longer shared with the viz thread), the SetImage(...) overloads are documented as set-once, and the immutable identity fields mailbox_ and texture_target_ are made const. The promise-image creation path in MakePromiseSkImageSinglePlane is also restructured so an ImageContextImpl is only populated once at creation (with a CHECK(image_context->mailbox() == mailbox) consistency assertion on cache reuse), removing the “reset-and-repopulate existing context” shape that enabled the race.

Key insight
SkiaOutputSurfaceImpl mutated (reset) an ImageContext’s promise SkImage on the viz thread at points where the GPU thread could be concurrently reading that same SkImage, violating the set-once cross-thread contract and racing on the Skia image’s refcount/backing.

Attack Path

  1. Render content A page submits compositor frames that produce non-overlay render passes whose backings are cached as ImageContextImpls in render_pass_image_cache_ and consumed by the GPU thread.
  2. Drive concurrent reuse Through animation/content that causes repeated redraws of cached render passes, the viz thread re-enters the draw path and calls clear_image() on a cached context (old code) while the GPU thread is mid-read of the same SkImage. (Precise web-controllable timing is inferred; the diff proves the unsynchronized access, not a specific JS recipe.)
  3. Race window The viz-thread image_.reset() drops the sk_sp reference concurrently with the GPU thread’s use of the image, corrupting the shared refcount / tearing the image state.
  4. Primitive Under the right interleaving this yields a use-after-free / memory corruption of the SkImage backing shared across the viz and GPU threads (the exploitability/escalation is inferred; the diff proves only the unsynchronized mutation).

Impact Assessment

The affected code runs in the GPU/viz process of the display compositor, not the sandboxed renderer, so a renderer would need to drive compositor content to influence timing; the race itself is between the viz thread and GPU thread within that process. The diff substantiates a non-deterministic data race on a shared sk_sp<SkImage>: concurrent reset() on one thread while another reads is undefined behavior and can corrupt the Skia image/refcount, with memory corruption or UAF as the realistic consequence. Because it is timing-dependent, reliability is low and a crash is the most certain outcome; promotion to a controllable primitive is inferred. The “input validation” advisory class is not supported by the code, which is purely a threading/ownership fix.

Changed Functions

FunctionChangeNotes
if
components/viz/service/display_embedder/skia_output_surface_impl.cc
modified

Files Changed

  • components/viz/service/display_embedder/skia_output_surface_impl.cc
  • components/viz/service/display/external_use_client.h

Audit Directions

  • Cross-thread SkImage/sk_sp mutation
    Grep viz display/display_embedder for other clear_image()/image_.reset() or SetImage() calls reachable off the viz thread while the GPU thread may read; the invariant is set-once-then-read-only.
  • Reused GPU resource caches
    Audit other per-frame caches (render_pass_image_cache_, overlay paths, ImageContextImpl reuse) for places that repopulate instead of recreating the context, which reintroduces the mutate-while-shared pattern.
  • const-correctness as invariant enforcement
    Look for other viz resource wrappers whose identity fields (mailbox, texture target, size, format) are mutable; making them const surfaces illegitimate post-set mutation.
From 30e4d30f53b3811a509ad899be7a38abf748af06 Mon Sep 17 00:00:00 2001
From: kylechar <kylechar@chromium.org>
Date: Wed, 21 Oct 2026 06:30:22 -0700
Subject: [PATCH] Fix ImageContext thread safety

SkiaOutputSurfaceImpl was resetting the ImageContextImpl SkImage in two
places. This isn't thread safe as it's possible SkImage is being
accessed on the GPU thread at the same time.

First, before sending the ImageContextImpl to the GPU main thread to be
deleted. This just wasn't needed.

Second, it was resetting non-overlay render pass SkImages each time they
were drawn. This was in case the render pass backing changed and a new
SkImage was needed. SkiaRenderer::UpdateRenderPassTextures() already
calls RemoveRenderPassResource() for any render pass backings that need
to be reallocated which deletes the ImageContextImpl. A new
ImageContextImpl and SkImage will be created in that case.

Bug: 574030524
Change-Id: Ic25d3d10daf8f719757a42d73710dbab7095328e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8608392
Commit-Queue: Kyle Charbonneau <kylechar@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1723735}
---
diff --git a/components/viz/service/display/external_use_client.h b/components/viz/service/display/external_use_client.h
index 1f2f104..1657489 100644
--- a/components/viz/service/display/external_use_client.h
+++ b/components/viz/service/display/external_use_client.h
@@ -51,10 +51,9 @@
     virtual void OnContextLost();
 
     //
-    // Thread safety is guaranteed by these invariants: (a) only the compositor
-    // thread modifies ImageContext, (b) ImageContext is not modified after
-    // |image| is set, and (c) GPU thread only reads ImageContext after |image|
-    // is set.
+    // Thread safety is guaranteed by these invariants: (a) only the viz thread
+    // modifies ImageContext, (b) ImageContext is not modified after |image| is
+    // set, and (c) GPU thread only reads ImageContext after |image| is set.
     //
     const gpu::Mailbox& mailbox() const { return mailbox_; }
     uint32_t texture_target() const { return texture_target_; }
@@ -83,11 +82,6 @@
 
     bool has_image() { return !!image_; }
     sk_sp<SkImage> image() { return image_; }
-    void SetImage(sk_sp<SkImage> image,
-                  std::vector<GrBackendFormat> backend_formats);
-    void SetImage(sk_sp<SkImage> image,
-                  std::vector<skgpu::graphite::TextureInfo> texture_infos);
-    void clear_image() { image_.reset(); }
     const std::vector<GrBackendFormat>& backend_formats() {
       return backend_formats_;
     }
@@ -95,10 +89,22 @@
       return texture_infos_;
     }
 
+    // SkImage and texture info MUST not be modified after they are initially
+    // set on the viz thread.
+    void SetImage(sk_sp<SkImage> image,
+                  std::vector<GrBackendFormat> backend_formats);
+    void SetImage(sk_sp<SkImage> image,
+                  std::vector<skgpu::graphite::TextureInfo> texture_infos);
+
+   protected:
+    // Resetting the image is only safe to do when ImageContext is no longer
+    // held on the viz thread, eg. during the destructor.
+    void clear_image() { image_.reset(); }
+
    private:
-    gpu::Mailbox mailbox_;
+    const gpu::Mailbox mailbox_;
     std::vector<gpu::SyncToken> sync_tokens_;
-    uint32_t texture_target_;
+    const uint32_t texture_target_;
 
     const gfx::Size size_;
     const SharedImageFormat format_;
@@ -113,7 +119,7 @@
     // android video.
     std::optional<gpu::VulkanYCbCrInfo> ycbcr_info_;
 
-    // The promise image which is used on display thread.
+    // The promise image which is used on the GPU thread.
     sk_sp<SkImage> image_;
     std::vector<GrBackendFormat> backend_formats_;
     std::vector<skgpu::graphite::TextureInfo> texture_infos_;
diff --git a/components/viz/service/display_embedder/skia_output_surface_impl.cc b/components/viz/service/display_embedder/skia_output_surface_impl.cc
index 1ce7523..33821d5 100644
--- a/components/viz/service/display_embedder/skia_output_surface_impl.cc
+++ b/components/viz/service/display_embedder/skia_output_surface_impl.cc
@@ -793,16 +793,6 @@
     current_paint_.emplace(characterization, mailbox, skip_draw_for_tests_);
   }
 
-  // We are going to overwrite the render pass when it is not for overlay, so we
-  // need to reset the image_context and a new promise image will be created
-  // when MakePromiseSkImageFromRenderPass() is called.
-  if (!is_overlay) {
-    auto it = render_pass_image_cache_.find(id);
-    if (it != render_pass_image_cache_.end()) {
-      it->second->clear_image();
-    }
-  }
-
   return current_paint_->canvas();
 }
 
@@ -902,12 +892,13 @@
   if (!image_context) {
     image_context =
         std::make_unique<ImageContextImpl>(mailbox, size, format, color_space);
-  }
-  if (!image_context->has_image()) {
     MakePromiseSkImageSinglePlane(image_context.get(), mipmap, false);
     if (!image_context->has_image()) {
+      image_context.reset();
       return nullptr;
     }
+  } else {
+    CHECK(image_context->mailbox() == mailbox);
   }
   images_in_current_paint_.push_back(image_context.get());
   return image_context->image();
@@ -925,7 +916,6 @@
     // If the render pass was only used for a copy request, there won't be a
     // matching entry in |render_pass_image_cache_|.
     if (it != render_pass_image_cache_.end()) {
-      it->second->clear_image();
       image_contexts.push_back(std::move(it->second));
       render_pass_image_cache_.erase(it);
     }
Loading diff…

Original Bug Report

The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.