CVE-2026-78958
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifgpu/command_buffer/service/copy_shared_image_helper.cc |
modified | |
ifgpu/command_buffer/service/graphite_utils.cc |
modified | |
GraphiteSharedContextgpu/command_buffer/service/graphite_utils.h |
modified |
Files Changed
gpu/command_buffer/service/copy_shared_image_helper.ccgpu/command_buffer/service/graphite_utils.ccgpu/command_buffer/service/graphite_utils.hgpu/command_buffer/service/raster_decoder.cc
Patch
From 872f3f32bf81dfa4efa23352c1741267401ea14f Mon Sep 17 00:00:00 2001 From: Greg Daniel <[email protected]> Date: Thu, 16 Jul 2026 06:50:30 -0700 Subject: [PATCH] Verify Graphite recording insertion before updating SharedImage cleared state Currently, when a SharedImage is uninitialized, we record a clear command into the Graphite Recorder and immediately mark the SharedImage as cleared. However, if the Recording containing the clear fails to be inserted into the Graphite Context for any reason, the clear command never executes. Since the cleared flag was already updated on the SharedImage, Chrome never attempts to clear it again on subsequent accesses, causing uninitialized memory reads. This CL propagates the recording insertion success (by returning a bool from FlushGraphiteRecorder and FlushWriteAccess) up to the callsites, and defers calling SetCleared() / SetClearedRect() until we know the Graphite recording insertion successfully completes. Bug: b/497205529 Change-Id: I78f0028b4a2206985417e588534ef404cdbf8a8b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8102302 Reviewed-by: Vasiliy Telezhnikov <[email protected]> Commit-Queue: Greg Daniel <[email protected]> Cr-Commit-Position: refs/heads/main@{#1663118} --- diff --git a/gpu/command_buffer/service/copy_shared_image_helper.cc b/gpu/command_buffer/service/copy_shared_image_helper.cc index 31efea78..3bc48e0 100644 --- a/gpu/command_buffer/service/copy_shared_image_helper.cc +++ b/gpu/command_buffer/service/copy_shared_image_helper.cc @@ -188,12 +188,12 @@ dest_scoped_access->surface()->writePixels(subset, xoffset, yoffset); - shared_context_state->FlushWriteAccess(dest_scoped_access); + bool success = shared_context_state->FlushWriteAccess(dest_scoped_access); shared_context_state->SubmitIfNecessary( std::move(end_semaphores), dest_scoped_access->NeedGraphiteContextSubmit()); - if (!dest_shared_image->IsCleared()) { + if (success && !dest_shared_image->IsCleared()) { dest_shared_image->SetClearedRect(dest_cleared_rect); } @@ -291,14 +291,20 @@ } bool need_graphite_submit = dest_scoped_access->NeedGraphiteContextSubmit(); + bool update_cleared_rect = false; + gfx::Rect new_cleared_rect; + // Flush dest surface and submit if necessary before exiting. absl::Cleanup cleanup = [&]() { - shared_context_state_->FlushWriteAccess(dest_scoped_access.get()); + bool success = + shared_context_state_->FlushWriteAccess(dest_scoped_access.get()); shared_context_state_->SubmitIfNecessary(std::move(end_semaphores), need_graphite_submit); + if (success && update_cleared_rect) { + dest_shared_image->SetClearedRect(new_cleared_rect); + } }; - gfx::Rect new_cleared_rect; gfx::Rect old_cleared_rect = dest_shared_image->ClearedRect(); if (!gles2::TextureManager::CombineAdjacentRects(old_cleared_rect, dest_rect, &new_cleared_rect)) { @@ -360,7 +366,7 @@ } if (!dest_shared_image->IsCleared()) { - dest_shared_image->SetClearedRect(new_cleared_rect); + update_cleared_rect = true; } // Note, that we still generate error for the client to indicate there was @@ -454,20 +460,26 @@ skia::BlitRGBAToYUVA(source_image.get(), yuva_sk_surfaces, yuva_info, gfx::RectToSkRect(dest_rect), false, gfx::RectToSkRect(source_rect)); - dest_shared_image->SetCleared(); + new_cleared_rect = gfx::Rect(dest_shared_image->size()); + update_cleared_rect = true; } if (!dest_shared_image->IsCleared()) { - dest_shared_image->SetClearedRect(new_cleared_rect); + update_cleared_rect = true; } } // Cancel cleanup as the cleanup order is different here. std::move(cleanup).Cancel(); - shared_context_state_->FlushWriteAccess(dest_scoped_access.get()); + bool success = + shared_context_state_->FlushWriteAccess(dest_scoped_access.get()); source_scoped_access->ApplyBackendSurfaceEndState(); shared_context_state_->SubmitIfNecessary(std::move(end_semaphores), need_graphite_submit); + + if (success && update_cleared_rect) { + dest_shared_image->SetClearedRect(new_cleared_rect); + } return result; } @@ -818,11 +830,12 @@ } } - shared_context_state_->FlushWriteAccess(dest_scoped_access.get()); + bool success = + shared_context_state_->FlushWriteAccess(dest_scoped_access.get()); shared_context_state_->SubmitIfNecessary(std::move(end_semaphores), need_graphite_submit); - if (!dest_shared_image->IsCleared()) { + if (success && !dest_shared_image->IsCleared()) { dest_shared_image->SetClearedRect(gfx::Rect(src_width, src_height)); } return base::ok(); diff --git a/gpu/command_buffer/service/graphite_utils.cc b/gpu/command_buffer/service/graphite_utils.cc index f7518d6..32f781c 100644 --- a/gpu/command_buffer/service/graphite_utils.cc +++ b/gpu/command_buffer/service/graphite_utils.cc @@ -63,18 +63,20 @@ } // namespace -void GraphiteFlush(GraphiteSharedContext* context, +bool GraphiteFlush(GraphiteSharedContext* context, skgpu::graphite::Recorder* recorder) { auto recording = recorder->snap(); if (recording) { - context->insertRecording({recording.get()}); + return context->insertRecording({recording.get()}); } + return true; } -void GraphiteFlushAndSubmit(GraphiteSharedContext* context, +bool GraphiteFlushAndSubmit(GraphiteSharedContext* context, skgpu::graphite::Recorder* recorder) { - GraphiteFlush(context, recorder); + bool success = GraphiteFlush(context, recorder); context->submit(); + return success; } bool GraphiteReadPixelsSync(GraphiteSharedContext* context, diff --git a/gpu/command_buffer/service/graphite_utils.h b/gpu/command_buffer/service/graphite_utils.h index 4e35ea2..94f6ff2 100644 --- a/gpu/command_buffer/service/graphite_utils.h +++ b/gpu/command_buffer/service/graphite_utils.h @@ -21,11 +21,11 @@ class GraphiteSharedContext; GPU_GLES2_EXPORT -void GraphiteFlush(GraphiteSharedContext* context, +bool GraphiteFlush(GraphiteSharedContext* context, skgpu::graphite::Recorder* recorder); GPU_GLES2_EXPORT -void GraphiteFlushAndSubmit(GraphiteSharedContext* context, +bool GraphiteFlushAndSubmit(GraphiteSharedContext* context, skgpu::graphite::Recorder* recorder); // Synchronously read pixels from a graphite image. diff --git a/gpu/command_buffer/service/raster_decoder.cc b/gpu/command_buffer/service/raster_decoder.cc index 92f5fd2..058dff1 100644 --- a/gpu/command_buffer/service/raster_decoder.cc +++ b/gpu/command_buffer/service/raster_decoder.cc @@ -899,6 +899,7 @@ std::unique_ptr<SkiaImageRepresentation> shared_image_; std::unique_ptr<SkiaImageRepresentation::ScopedWriteAccess> scoped_shared_image_write_; + bool should_clear_shared_image_ = false; std::unique_ptr<RasterImageRepresentation> shared_image_raster_; std::unique_ptr<RasterImageRepresentation::ScopedWriteAccess> @@ -2059,12 +2060,13 @@ return; } - shared_context_state_->FlushWriteAccess(dest_scoped_access.get()); + bool success = + shared_context_state_->FlushWriteAccess(dest_scoped_access.get()); shared_context_state_->SubmitIfNecessary( std::move(end_semaphores), dest_scoped_access->NeedGraphiteContextSubmit()); - if (!dest_shared_image->IsCleared()) { + if (success && !dest_shared_image->IsCleared()) { dest_shared_image->SetClearedRect( gfx::Rect(x_offset, y_offset, src_width, src_height)); } @@ -2311,12 +2313,13 @@ /*numLevels=*/1, release_proc, graphite_texture_ptr); }
Original Bug Report
Potential cross-origin info leak via state desynchronization in Skia Graphite QueueManager
Project Fortify, an experimental security project, has identified the following potential security issue.
Overview: Skia Graphite’s QueueManager updates its tracking of the last added recording ID before completing the insertion process. If insertion subsequently fails, the tracker remains incremented, allowing an attacker to drop a recording (e.g., a SharedImage clear) without failing the strict ordering check for future recordings, leading to a potential leak of uninitialized GPU memory.
Affected files:
third_party/skia/src/gpu/graphite/QueueManager.cppthird_party/skia/src/gpu/graphite/QueueManager.h
Estimated timestamp from git blame: 2026-01-20
Summary
A logic error in Skia Graphite’s QueueManager causes the internal tracking of recording IDs to be updated prematurely. If a recording fails during insertion after this update occurs, the system’s state becomes desynchronized. This allows subsequent recordings to pass ordering checks and execute, even though a preceding recording was dropped. This can be exploited to bypass GPU memory initialization, leading to a reliable cross-origin information leak.
Vulnerability Details
In third_party/skia/src/gpu/graphite/QueueManager.cpp, the addRecording function is responsible for adding commands from a Recording to the current command buffer. For contexts requiring ordered recordings (such as Chrome’s), it verifies that the incoming recording ID is exactly one greater than the last added ID. However, the tracking map fLastAddedRecordingIDs is updated prematurely at line 133:
128: RETURN_FAIL_IF(recordingID && info.fRecording->priv().uniqueID() != *recordingID + 1,
129: InsertStatus::kOutOfOrderRecording,
130: "Recordings are expected to be replayed in order");
131:
132: // Note the new Recording ID.
133: fLastAddedRecordingIDs.set(recorderID, info.fRecording->priv().uniqueID());
Following this update, there are multiple RETURN_FAIL_IF checks for operations like command buffer setup (line 145) or lazy proxy instantiation (lines 173, 178). If any of these fail, the function returns an error, but the fLastAddedRecordingIDs state is not rolled back.
Because the counter has been advanced, a subsequent recording ($N+1$) will be accepted even if recording $N$ failed to execute. The failed recording $N$ is dropped.
Potential Exploitation Steps
Note: These are suggested steps based on code analysis; we do not yet have a working proof-of-concept.
- From a compromised renderer, an attacker creates a new
SharedImage. Its internalIsCleared()state is initiallyfalse. - The attacker binds the image and issues a
BeginRasterCHROMIUMcommand. - In
RasterDecoderImpl::DoBeginRasterCHROMIUM, Chrome observes that the image needs clearing and issues a clear command to the Skia Canvas. This clear operation becomes part of recording $N$. - Crucially, Chrome immediately marks the
SharedImageas cleared viashared_image_->SetCleared()(inraster_decoder.cc), assuming the recording will successfully execute. - The attacker intentionally exhausts GPU memory (e.g., by allocating many large textures) and flushes the context.
- Chrome snaps recording $N$ and submits it to
QueueManager::addRecording. The ID tracker is updated to $N$. - Due to the induced OOM,
setupCommandBuffer()or a proxy instantiation fails.addRecordingreturns a failure status (e.g.,kAddCommandsFailed). - In Chrome’s
GraphiteSharedContext::insertRecording, this failure is treated as recoverable (unlikekOutOfOrderRecording). The context is kept alive, but the clear commands are dropped. - The attacker frees memory and issues a command to read from the
SharedImage(recording $N+1$). Since Chrome’s state indicates it is cleared, no further clear is issued. QueueManager::addRecordingaccepts recording $N+1$ because the tracker expects $N+1$.- The GPU executes recording $N+1$, reading uninitialized GPU memory that may contain cross-origin data.
Recommended Fix
The update to fLastAddedRecordingIDs should be deferred until the end of QueueManager::addRecording(), after all potential failure points have passed. Alternatively, a rollback mechanism should be implemented in the RETURN_FAIL_IF macro or failure paths.
Evaluated with Chrome root at commit: a9cbf6e8b275fe4147435aa905f3b7f5a656f5f0
Results from so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.