Chrome · Skia › Ganesh
CVE-2026-91733
Input Validation in Skia (Ganesh)
Overview
High
Severity
8.3
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- Ganesh
- Skia’s GPU backend, whose
GrDirectContextrecords and issues draw commands to an underlying 3D API such as Vulkan or GL. - `flush`/`submit`
flushtranslates recorded draws into backend command buffers whilesubmitsends them to the GPU; both can fail independently.- `GrSemaphoresSubmitted`
- an enum (
kYes/kNo) that historically doubled as the return value offlush, indicating whether GPU semaphores were submitted. - `FlushResult`
- the new struct added by this patch that carries a distinct
fSuccessboolean alongside thefSubmittedsemaphore status.
Root Cause Analysis
Before this patch, GrDirectContext::flush and the related helpers in SkSurfaceGanesh.h and GrVkSecondaryCBDrawContext.h returned only a GrSemaphoresSubmitted value, which conflated two distinct outcomes: whether semaphores were submitted and whether the flush itself actually succeeded. Because a returned GrSemaphoresSubmitted::kNo is documented to be a normal, expected result whenever no semaphores were requested, a genuine flush failure was indistinguishable from a benign no-semaphore flush, so failure was never plumbed up the call stack and callers proceeded as if rendering had completed — the improper state validation the advisory describes.
The fix introduces FlushResult, separating fSuccess (did the flush succeed) from fSubmitted (semaphore status), and threads that success bit through flushAndSubmit via result.fSuccess &= this->submit(...) so that a failed flush or failed submit is now observable.
The patch also hardens Device::asyncRescaleAndReadPixels and Device::asyncRescaleAndReadPixelsYUV420, which previously returned silently when no GrDirectContext was available, leaving the read-pixels callback never invoked; they now call callback(callbackContext, nullptr) so the caller’s pending state is resolved instead of left dangling.
Key insight
The single core mistake was overloading one enum (
GrSemaphoresSubmitted) to signal both semaphore submission and overall flush success, which silently swallowed flush failures; the fix disentangles the two into FlushResult::fSuccess and FlushResult::fSubmitted and propagates the real success value all the way to the API.Attack Path
- Trigger a flush failure
An attacker drives GPU work (via canvas/WebGL-style rendering fed into a Ganesh
GrDirectContext) into a state where the backendflushorsubmitfails at the driver level. - Failure is masked
Because
flushreturned onlyGrSemaphoresSubmitted, the failure is reported (if at all) askNo, which callers legitimately treat as “no semaphores requested” rather than an error, so no error propagates up the stack. - Caller consumes invalid state
Clients of
skgpu::ganesh::Flush/FlushAndSubmit,GrVkSecondaryCBDrawContext::flush, orDeviceread-pixels paths continue as though rendering succeeded, operating on surface/image contents whose GPU state was never validly produced. - Downstream memory corruption
Acting on the improperly validated GPU state can lead to use of stale or uninitialized backend resources; any specific out-of-bounds read/write sink, exploit primitive, or sandbox escape beyond this is not shown in the diff
(inferred).
Impact Assessment
An attacker who can force a flush/submit failure gains the ability to make Skia’s Ganesh backend proceed on unvalidated GPU state within the GPU/rendering process that hosts the
GrDirectContext, with the precondition of being able to submit rendering work that fails at flush time. The advisory classifies this as a LogicError (improper state validation in Skia) and states no exploited-in-the-wild status, so none is claimed here. Any escalation to a concrete read/write primitive or code execution is (inferred) and not demonstrated by the patch itself.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifsrc/gpu/ganesh/Device.cpp |
modified | |
ifsrc/gpu/ganesh/GrDirectContext.cpp |
modified |
Files Changed
include/gpu/ganesh/SkSurfaceGanesh.hsrc/gpu/ganesh/vk/GrVkSecondaryCBDrawContext.cppsrc/gpu/ganesh/SurfaceContext.cppsrc/gpu/ganesh/GrDrawingManager.cppsrc/gpu/ganesh/surface/SkSurface_Ganesh.cppsrc/gpu/ganesh/image/SkImage_GaneshFactories.cppsrc/gpu/ganesh/image/SkImage_GaneshFactories_Android.cppsrc/gpu/ganesh/GrDirectContextPriv.hinclude/private/chromium/GrVkSecondaryCBDrawContext.hrelnotes/flush-error.mdsrc/gpu/ganesh/image/SkImage_GaneshBase.hsrc/gpu/ganesh/GrGpu.hsrc/gpu/ganesh/image/SkImage_Ganesh.hsrc/gpu/ganesh/image/SkImage_GaneshYUVA.hsrc/gpu/ganesh/GrGpu.cppsrc/gpu/ganesh/GrDirectContext.cppinclude/gpu/ganesh/GrDirectContext.hsrc/gpu/ganesh/image/SkImage_Ganesh.cppsrc/gpu/ganesh/GrDrawingManager.htests/DefaultPathRendererTest.cppsrc/gpu/ganesh/image/SkImage_GaneshYUVA.cpptests/SurfaceSemaphoreTest.cppsrc/gpu/ganesh/GrDirectContextPriv.cppsrc/gpu/ganesh/Device.cpp
Audit Directions
- Conflated return codesSearch for APIs where a single enum or boolean encodes two orthogonal outcomes (e.g. “did work happen” vs. “was a side effect performed”), since callers cannot distinguish real failure from a benign state.
- Dropped flush/submit resultsAudit call sites that
(void)-cast or ignoreflush/submit/FlushResultreturn values, especially where subsequent code reads back surface or image contents assuming success. - Callbacks skipped on early returnLook for async read-pixels or completion-callback paths that
returnearly on a missing context or resource without invoking their callback, leaving caller state permanently pending.
Patch
From 4b2fad13ffe43c3099bcf6d9c432bed53a4cfadc Mon Sep 17 00:00:00 2001 From: Robert Phillips <[email protected]> Date: Tue, 08 Sep 2026 11:51:48 -0400 Subject: [PATCH] [ganesh] Add a new GrDirectContext::FlushResult and plumb flush failure up the call stack The plumbing of flush failure up the call stack, from the DrawingManager, fused semaphore submission and flush failure. This CL separates out the two concerns and plumbs it all the way up to the API. Bug: b/553122373 Bug: b/540021213 Change-Id: I55d2c3685fcf755e2997a46d9de78e447ccf9db3 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1348276 Commit-Queue: Robert Phillips <[email protected]> Reviewed-by: Michael Ludwig <[email protected]> --- diff --git a/include/gpu/ganesh/GrDirectContext.h b/include/gpu/ganesh/GrDirectContext.h index 9dc6f62..8ccf5a4 100644 --- a/include/gpu/ganesh/GrDirectContext.h +++ b/include/gpu/ganesh/GrDirectContext.h @@ -320,14 +320,34 @@ bool wait(int numSemaphores, const GrBackendSemaphore* waitSemaphores, bool deleteSemaphoresAfterWait = true); + /* + * Result of a flush call. A flush can be successful w/ or w/o any semaphores being flushed. + * In some circumstances an unsuccessful flush can still have flushed the semaphores but + * the rendering results should be discarded. + */ + struct FlushResult { + // This implicit cast is temporary and recreates the success/semaphore-submission + // conflation of the old API. This can be removed when all clients have been + // updated to the new return value. + operator GrSemaphoresSubmitted() const { + return fSuccess ? GrSemaphoresSubmitted::kYes : GrSemaphoresSubmitted::kNo; + } + + bool fSuccess = false; // Did the flush succeed + // Were any semaphores submitted during the flush process. Will be kNo if no + // semaphores were specified. + GrSemaphoresSubmitted fSubmitted = GrSemaphoresSubmitted::kNo; + }; + /** * Call to ensure all drawing to the context has been flushed and submitted to the underlying 3D * API. This is equivalent to calling GrContext::flush with a default GrFlushInfo followed by * GrContext::submit(sync). */ - void flushAndSubmit(GrSyncCpu sync = GrSyncCpu::kNo) { - this->flush(GrFlushInfo()); - this->submit(sync); + FlushResult flushAndSubmit(GrSyncCpu sync = GrSyncCpu::kNo) { + FlushResult result = this->flush(GrFlushInfo()); + result.fSuccess &= this->submit(sync); + return result; } /** @@ -341,22 +361,22 @@ * buffer or encoder objects. However, these objects are not sent to the gpu until a * submission occurs. * - * If the return is GrSemaphoresSubmitted::kYes, only initialized GrBackendSemaphores will be - * submitted to the gpu during the next submit call (it is possible Skia failed to create a - * subset of the semaphores). The client should not wait on these semaphores until after submit - * has been called, and must keep them alive until then. If this call returns - * GrSemaphoresSubmitted::kNo, the GPU backend will not submit any semaphores to be signaled on - * the GPU. Thus the client should not have the GPU wait on any of the semaphores passed in with - * the GrFlushInfo. Regardless of whether semaphores were submitted to the GPU or not, the - * client is still responsible for deleting any initialized semaphores. + * If FlushResult.fSubmitted is GrSemaphoresSubmitted::kYes, only initialized + * GrBackendSemaphores will be submitted to the gpu during the next submit call (it is possible + * Skia failed to create a subset of the semaphores). The client should not wait on these + * semaphores until after submit has been called, and must keep them alive until then. If + * fSubmitted is GrSemaphoresSubmitted::kNo, the GPU backend will not submit any semaphores + * to be signaled on the GPU. Thus the client should not have the GPU wait on any of the + * semaphores passed in with the GrFlushInfo. Regardless of whether semaphores were submitted + * to the GPU or not, the client is still responsible for deleting any initialized semaphores. * Regardless of semaphore submission the context will still be flushed. It should be - * emphasized that a return value of GrSemaphoresSubmitted::kNo does not mean the flush did not - * happen. It simply means there were no semaphores submitted to the GPU. A caller should only - * take this as a failure if they passed in semaphores to be submitted. + * emphasized that a fSubmitted value of GrSemaphoresSubmitted::kNo does not mean the flush + * did not happen. It simply means there were no semaphores submitted to the GPU. A caller + * should only take this as a failure if they passed in semaphores to be submitted. */ - GrSemaphoresSubmitted flush(const GrFlushInfo& info); + FlushResult flush(const GrFlushInfo& info); - void flush() { this->flush(GrFlushInfo()); } + void flush() { (void) this->flush(GrFlushInfo()); } /** Flushes any pending uses of texture-backed images in the GPU backend. If the image is not * texture-backed (including promise texture images) or if the GrDirectContext does not @@ -367,13 +387,18 @@ * @param image the non-null image to flush. * @param info flush options */ - GrSemaphoresSubmitted flush(const sk_sp<const SkImage>& image, const GrFlushInfo& info); - void flush(const sk_sp<const SkImage>& image); + FlushResult flush(const sk_sp<const SkImage>& image, const GrFlushInfo& info); + + void flush(const sk_sp<const SkImage>& image) { (void) this->flush(image, {}); } /** Version of flush() that uses a default GrFlushInfo. Also submits the flushed work to the - * GPU. + * GPU. */ - void flushAndSubmit(const sk_sp<const SkImage>& image); + FlushResult flushAndSubmit(const sk_sp<const SkImage>& image) { + FlushResult result = this->flush(image, {}); + result.fSuccess &= this->submit(); + return result; + } /** Issues pending SkSurface commands to the GPU-backed API objects and resolves any SkSurface * MSAA. A call to GrDirectContext::submit is always required to ensure work is actually sent @@ -400,19 +425,20 @@ * The GrFlushInfo describes additional options to flush. Please see documentation at * GrFlushInfo for more info. * - * If the return is GrSemaphoresSubmitted::kYes, only initialized GrBackendSemaphores will be - * submitted to the gpu during the next submit call (it is possible Skia failed to create a - * subset of the semaphores). The client should not wait on these semaphores until after submit - * has been called, but must keep them alive until then. If a submit flag was passed in with - * the flush these valid semaphores can we waited on immediately. If this call returns - * GrSemaphoresSubmitted::kNo, the GPU backend will not submit any semaphores to be signaled on - * the GPU. Thus the client should not have the GPU wait on any of the semaphores passed in - * with the GrFlushInfo. Regardless of whether semaphores were submitted to the GPU or not, the - * client is still responsible for deleting any initialized semaphores. - * Regardless of semaphore submission the context will still be flushed. It should be - * emphasized that a return value of GrSemaphoresSubmitted::kNo does not mean the flush did not - * happen. It simply means there were no semaphores submitted to the GPU. A caller should only - * take this as a failure if they passed in semaphores to be submitted. + * If FlushResult.fSubmitted is GrSemaphoresSubmitted::kYes, only initialized + * GrBackendSemaphores will be submitted to the gpu during the next submit call (it is + * possible Skia failed to create a subset of the semaphores). The client should not wait on + * these semaphores until after submit has been called, but must keep them alive until then. + * If a submit flag was passed in with the flush these valid semaphores can we waited on + * immediately. If this fSubmitted is GrSemaphoresSubmitted::kNo, the GPU backend will not + * submit any semaphores to be signaled on the GPU. Thus the client should not have the GPU + * wait on any of the semaphores passed in with the GrFlushInfo. Regardless of whether + * semaphores were submitted to the GPU or not, the client is still responsible for deleting + * any initialized semaphores. Regardless of semaphore submission the context will still be + * flushed. It should be emphasized that a fSubmitted value of GrSemaphoresSubmitted::kNo + * does not mean the flush did not happen. It simply means there were no semaphores submitted + * to the GPU. A caller should only take this as a failure if they passed in semaphores to + * be submitted. * * Pending surface commands are flushed regardless of the return result. * @@ -420,9 +446,9 @@ * @param access type of access the call will do on the backend object after flush * @param info flush options */ - GrSemaphoresSubmitted flush(SkSurface* surface, - SkSurfaces::BackendSurfaceAccess access, - const GrFlushInfo& info); + FlushResult flush(SkSurface* surface, + SkSurfaces::BackendSurfaceAccess access, + const GrFlushInfo& info); /** * Same as above except: @@ -443,9 +469,16 @@ * @param info flush options * @param newState optional state change request after flush */ - GrSemaphoresSubmitted flush(SkSurface* surface, - const GrFlushInfo& info, - const skgpu::MutableTextureState* newState = nullptr); + FlushResult flush(SkSurface* surface, + const GrFlushInfo& info, + const skgpu::MutableTextureState* newState = nullptr); + + /** + * Flushes the given surface with the default GrFlushInfo. + * + * Has no effect on a CPU-backed surface. + */ + void flush(SkSurface* surface) { (void) this->flush(surface, GrFlushInfo(), nullptr); } /** Call to ensure all reads/writes of the surface have been issued to the underlying 3D API. * Skia will correctly order its own draws and pixel operations. This must to be used to ensure @@ -455,14 +488,7 @@ * * Has no effect on a CPU-backed surface. */ - void flushAndSubmit(SkSurface* surface, GrSyncCpu sync = GrSyncCpu::kNo); - - /** - * Flushes the given surface with the default GrFlushInfo. - * - * Has no effect on a CPU-backed surface. - */ - void flush(SkSurface* surface); + FlushResult flushAndSubmit(SkSurface* surface, GrSyncCpu sync = GrSyncCpu::kNo); /** * Submit outstanding work to the gpu from all previously un-submitted flushes. The return diff --git a/include/gpu/ganesh/SkSurfaceGanesh.h b/include/gpu/ganesh/SkSurfaceGanesh.h index 3021bca..5ec91ea 100644 --- a/include/gpu/ganesh/SkSurfaceGanesh.h +++ b/include/gpu/ganesh/SkSurfaceGanesh.h @@ -11,6 +11,7 @@ #include "include/core/SkImageInfo.h" #include "include/core/SkRefCnt.h" #include "include/core/SkSurface.h" +#include "include/gpu/ganesh/GrDirectContext.h" #include "include/gpu/ganesh/GrTypes.h" #include "include/private/SkAPI.h" @@ -209,10 +210,12 @@ // Clients should strive to call GrDirectContext::flush directly. However, there exist some // places where the GrDirectContext is hard to find, these helpers allow for the flushing of the // provided surface. This is a no-op if the surface is nullptr or not GPU backed. -SK_API GrSemaphoresSubmitted Flush(sk_sp<SkSurface>); -SK_API GrSemaphoresSubmitted Flush(SkSurface*); -SK_API void FlushAndSubmit(sk_sp<SkSurface>); -SK_API void FlushAndSubmit(SkSurface*); +SK_API GrDirectContext::FlushResult Flush(sk_sp<SkSurface>); +SK_API GrDirectContext::FlushResult Flush(SkSurface*); + +SK_API GrDirectContext::FlushResult FlushAndSubmit(sk_sp<SkSurface>); +SK_API GrDirectContext::FlushResult FlushAndSubmit(SkSurface*); + } // namespace skgpu::ganesh #endif diff --git a/include/private/chromium/GrVkSecondaryCBDrawContext.h b/include/private/chromium/GrVkSecondaryCBDrawContext.h index 80bed15..ede4db8 100644 --- a/include/private/chromium/GrVkSecondaryCBDrawContext.h +++ b/include/private/chromium/GrVkSecondaryCBDrawContext.h @@ -11,6 +11,7 @@ #include "include/core/SkRefCnt.h" #include "include/core/SkSurfaceProps.h" #include "include/core/SkTypes.h" +#include "include/gpu/ganesh/GrDirectContext.h" #include <memory> @@ -72,7 +73,7 @@ // Records all the draws to the imported secondary command buffer and sets any dependent // offscreen draws to the GPU. - void flush(); + GrDirectContext::FlushResult flush(); /** Inserts a list of GPU semaphores that Skia will have the driver wait on before executing commands for this secondary CB. The wait semaphores will get added to the VkCommandBuffer diff --git a/relnotes/flush-error.md b/relnotes/flush-error.md new file mode 100644 index 0000000..2f436c3 --- /dev/null +++ b/relnotes/flush-error.md @@ -0,0 +1 @@ +The error handling associated with Ganesh's GrDirectContext flushing has been updated. Instead of returning a GrSemaphoresSubmitted value which conflates semaphore submission and flush success, a new FlushResult struct is returned - which explicitly separates the two concerns. FlushResult has an implicit cast which allows clients to still use the old GrSemaphoresSubmitted-based API. diff --git a/src/gpu/ganesh/Device.cpp b/src/gpu/ganesh/Device.cpp index f30d578..d61e230 100644 --- a/src/gpu/ganesh/Device.cpp +++ b/src/gpu/ganesh/Device.cpp @@ -1274,15 +1274,16 @@ RescaleGamma rescaleGamma, RescaleMode rescaleMode, ReadPixelsCallback callback, - ReadPixelsContext context) { + ReadPixelsContext callbackContext) { auto* sdc = fSurfaceDrawContext.get(); // Context TODO: Elevate direct context requirement to public API. auto dContext = sdc->recordingContext()->asDirectContext(); if (!dContext) { + callback(callbackContext, nullptr); return; } sdc->asyncRescaleAndReadPixels(dContext, info, srcRect, rescaleGamma, rescaleMode, callback, - context); + callbackContext); } void Device::asyncRescaleAndReadPixelsYUV420(SkYUVColorSpace yuvColorSpace, @@ -1293,11 +1294,12 @@ RescaleGamma rescaleGamma, RescaleMode rescaleMode, ReadPixelsCallback callback, - ReadPixelsContext context) { + ReadPixelsContext callbackContext) { auto* sdc = fSurfaceDrawContext.get(); // Context TODO: Elevate direct context requirement to public API. auto dContext = sdc->recordingContext()->asDirectContext(); if (!dContext) { + callback(callbackContext, nullptr); return; } sdc->asyncRescaleAndReadPixelsYUV420(dContext, @@ -1309,7 +1311,7 @@ rescaleGamma, rescaleMode, callback, - context); + callbackContext); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/ganesh/GrDirectContext.cpp b/src/gpu/ganesh/GrDirectContext.cpp index 7f02376..bac84f3 100644 --- a/src/gpu/ganesh/GrDirectContext.cpp +++ b/src/gpu/ganesh/GrDirectContext.cpp @@ -223,7 +223,8 @@ return; } - this->flushAndSubmit(); + // TODO: bubble this error up to the client + (void) this->flushAndSubmit(); #if !defined(SK_ENABLE_OPTIMIZE_SIZE) if (fSmallPathAtlasMgr) { fSmallPathAtlasMgr->reset(); @@ -432,7 +433,7 @@ return this->caps()->supportedGpuStats(); } -GrSemaphoresSubmitted GrDirectContext::flush(const GrFlushInfo& info) { +GrDirectContext::FlushResult GrDirectContext::flush(const GrFlushInfo& info) { ASSERT_SINGLE_OWNER if (this->abandoned()) { if (info.fFinishedProc) { @@ -441,7 +442,7 @@ if (info.fSubmittedProc) { info.fSubmittedProc(info.fSubmittedContext, false); } - return GrSemaphoresSubmitted::kNo; + return {false, GrSemaphoresSubmitted::kNo}; } return this->drawingManager()->flushSurfaces( @@ -463,37 +464,28 @@ return result; } -GrSemaphoresSubmitted GrDirectContext::flush(const sk_sp<const SkImage>& image, - const GrFlushInfo& flushInfo) { +GrDirectContext::FlushResult GrDirectContext::flush(const sk_sp<const SkImage>& image, + const GrFlushInfo& flushInfo) { if (!image) { - return GrSemaphoresSubmitted::kNo; + return {false, GrSemaphoresSubmitted::kNo}; } auto ib = as_IB(image); if (!ib->isGaneshBacked()) { - return GrSemaphoresSubmitted::kNo; + return {false, GrSemaphoresSubmitted::kNo}; } auto igb = static_cast<const SkImage_GaneshBase*>(image.get()); return igb->flush(this, flushInfo); } -void GrDirectContext::flush(const sk_sp<const SkImage>& image) { - this->flush(image, {}); -} - -void GrDirectContext::flushAndSubmit(const sk_sp<const SkImage>& image) { - this->flush(image, {}); - this->submit(); -} - -GrSemaphoresSubmitted GrDirectContext::flush(SkSurface* surface, - SkSurfaces::BackendSurfaceAccess access, - const GrFlushInfo& info) { +GrDirectContext::FlushResult GrDirectContext::flush(SkSurface* surface, + SkSurfaces::BackendSurfaceAccess access, + const GrFlushInfo& info) { if (!surface) { - return GrSemaphoresSubmitted::kNo; + return {false, GrSemaphoresSubmitted::kNo}; } auto sb = asSB(surface); if (!sb->isGaneshBacked()) { - return GrSemaphoresSubmitted::kNo; + return {false, GrSemaphoresSubmitted::kNo}; } auto gs = static_cast<SkSurface_Ganesh*>(surface); @@ -503,15 +495,15 @@ return this->priv().flushSurface(rtp, access, info, nullptr); } -GrSemaphoresSubmitted GrDirectContext::flush(SkSurface* surface, - const GrFlushInfo& info, - const skgpu::MutableTextureState* newState) { +GrDirectContext::FlushResult GrDirectContext::flush(SkSurface* surface, + const GrFlushInfo& info, + const skgpu::MutableTextureState* newState) { if (!surface) { - return GrSemaphoresSubmitted::kNo; + return {false, GrSemaphoresSubmitted::kNo}; } auto sb = asSB(surface); if (!sb->isGaneshBacked()) { - return GrSemaphoresSubmitted::kNo; + return {false, GrSemaphoresSubmitted::kNo}; } ... (file diff truncated)
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.
References
On This Page