PatchGap
Chrome · Skia

CVE-2026-9892

Logic Error in Skia
Critical chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Critical
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate implementation in Skia
DescriptionInappropriate implementation in Skia
ComponentSkia
Bug ClassLogic Error
Tracker513948178
Fix commitdefc3a5a9296 (skia) +53/-27
CISA KEVNot listed
CreditedGoogle
Disclosed2026-05-27

Changed Functions

FunctionChangeNotes
if
src/gpu/ganesh/GrOpFlushState.cpp
 modified
for
src/gpu/ganesh/GrOpFlushState.cpp
 modified

Files Changed

  • src/gpu/ganesh/GrOpFlushState.cpp
  • src/gpu/ganesh/GrOpsRenderPass.h
  • src/gpu/ganesh/d3d/GrD3DOpsRenderPass.cpp
  • src/gpu/ganesh/d3d/GrD3DOpsRenderPass.h
  • src/gpu/ganesh/gl/GrGLOpsRenderPass.h
  • src/gpu/ganesh/mock/GrMockOpsRenderPass.h
  • src/gpu/ganesh/mtl/GrMtlOpsRenderPass.h
  • src/gpu/ganesh/mtl/GrMtlOpsRenderPass.mm
  • src/gpu/ganesh/vk/GrVkOpsRenderPass.cpp

Patch

view commit
From defc3a5a92966c32cb2a6a901e2fa3036a13bb8a Mon Sep 17 00:00:00 2001
From: Nicolette Prevost <nicolettep@google.com>
Date: Wed, 20 May 2026 12:05:45 -0400
Subject: [PATCH] Report and handle failure for inlineUpload(...) calls

* The bug associated with this change called out that we should not attempt to perform an upload on an externally-owned secondary command buffer.

* Exiting early does not appropriately signal that an upload failed, so modify `GrOpsRenderPass::inlineUpload(...)` base class to return a bool indicating success or failure. Upon failure, do not attempt the associated immediate draw call and report that the draw failed.

* To maintain current behavior, have `inlineUpload(...)` return true in nearly all cases except that identified in the associated bug.

Bug: b/513948178
Change-Id: Idfeb2527062818e5fa63a38112dd1c26f7847998
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1239496
Commit-Queue: Nicolette Prevost <nicolettep@google.com>
Auto-Submit: Nicolette Prevost <nicolettep@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
---

diff --git a/src/gpu/ganesh/GrOpFlushState.cpp b/src/gpu/ganesh/GrOpFlushState.cpp
index 27768cd..68cb7d5 100644
--- a/src/gpu/ganesh/GrOpFlushState.cpp
+++ b/src/gpu/ganesh/GrOpFlushState.cpp
@@ -53,27 +53,39 @@
 
     while (fCurrDraw != fDraws.end() && fCurrDraw->fOp == op) {
         skgpu::Token drawToken = fTokenTracker->nextFlushToken();
+
+        bool drawUploadFailure = false;
         while (fCurrUpload != fInlineUploads.end() &&
                fCurrUpload->fUploadBeforeToken == drawToken) {
-            this->opsRenderPass()->inlineUpload(this, fCurrUpload->fUpload);
+            if (!this->opsRenderPass()->inlineUpload(this, fCurrUpload->fUpload)) {
+                drawUploadFailure = true;
+            }
+            // Attempt subsequent uploads even if one fails (future draws may depend upon them).
             ++fCurrUpload;
         }
 
-        GrProgramInfo programInfo(this->caps(),
-                                  this->writeView(),
-                                  this->usesMSAASurface(),
-                                  pipeline,
-                                  userStencilSettings,
-                                  fCurrDraw->fGeometryProcessor,
-                                  fCurrDraw->fPrimitiveType,
-                                  this->renderPassBarriers(),
-                                  this->colorLoadOp());
+        // If not all of the uploads succeeded, do not attempt to execute the draw and relevant
+        // preparations. Continue on to the next draw operation (which may also fail if subsequent
+        // draw calls depended upon these uploads, but we cannot know that at this point).
+        if (drawUploadFailure) {
+            fGpu->stats()->incNumFailedDraws();
+        } else {
+            GrProgramInfo programInfo(this->caps(),
+                                      this->writeView(),
+                                      this->usesMSAASurface(),
+                                      pipeline,
+                                      userStencilSettings,
+                                      fCurrDraw->fGeometryProcessor,
+                                      fCurrDraw->fPrimitiveType,
+                                      this->renderPassBarriers(),
+                                      this->colorLoadOp());
 
-        this->bindPipelineAndScissorClip(programInfo, chainBounds);
-        this->bindTextures(programInfo.geomProc(), fCurrDraw->fGeomProcProxies,
-                           programInfo.pipeline());
-        for (int i = 0; i < fCurrDraw->fMeshCnt; ++i) {
-            this->drawMesh(fCurrDraw->fMeshes[i]);
+            this->bindPipelineAndScissorClip(programInfo, chainBounds);
+            this->bindTextures(programInfo.geomProc(), fCurrDraw->fGeomProcProxies,
+                            programInfo.pipeline());
+            for (int i = 0; i < fCurrDraw->fMeshCnt; ++i) {
+                this->drawMesh(fCurrDraw->fMeshes[i]);
+            }
         }
 
         fTokenTracker->issueFlushToken();
diff --git a/src/gpu/ganesh/GrOpsRenderPass.h b/src/gpu/ganesh/GrOpsRenderPass.h
index 656e906..516931f 100644
--- a/src/gpu/ganesh/GrOpsRenderPass.h
+++ b/src/gpu/ganesh/GrOpsRenderPass.h
@@ -129,7 +129,7 @@
                           int baseVertex);
 
     // Performs an upload of vertex data in the middle of a set of a set of draws
-    virtual void inlineUpload(GrOpFlushState*, GrDeferredTextureUploadFn&) = 0;
+    virtual bool inlineUpload(GrOpFlushState*, GrDeferredTextureUploadFn&) = 0;
 
     /**
      * Clear the owned render target. Clears the full target if 'scissor' is disabled, otherwise it
diff --git a/src/gpu/ganesh/d3d/GrD3DOpsRenderPass.cpp b/src/gpu/ganesh/d3d/GrD3DOpsRenderPass.cpp
index cbf1387..5fcea05 100644
--- a/src/gpu/ganesh/d3d/GrD3DOpsRenderPass.cpp
+++ b/src/gpu/ganesh/d3d/GrD3DOpsRenderPass.cpp
@@ -339,16 +339,17 @@
     fGpu->currentCommandList()->clearDepthStencilView(d3dStencil, stencilColor, &clearRect);
 }
 
-void GrD3DOpsRenderPass::inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) {
+bool GrD3DOpsRenderPass::inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) {
     // If we ever start using copy command lists for doing uploads, then we'll need to make sure
     // we submit our main command list before doing the copy here and then start a new main command
     // list.
-
     fGpu->endRenderPass(fRenderTarget, fOrigin, fBounds);
 
     // We pass in true here to signal that after the upload we need to set the upload texture's
     // resource state back to D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE.
     state->doUpload(upload, true);
+
+    return true;
 }
 
 void GrD3DOpsRenderPass::submit() {
diff --git a/src/gpu/ganesh/d3d/GrD3DOpsRenderPass.h b/src/gpu/ganesh/d3d/GrD3DOpsRenderPass.h
index 5da3d75..65db27b 100644
--- a/src/gpu/ganesh/d3d/GrD3DOpsRenderPass.h
+++ b/src/gpu/ganesh/d3d/GrD3DOpsRenderPass.h
@@ -22,7 +22,7 @@
 
     ~GrD3DOpsRenderPass() override;
 
-    void inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override;
+    bool inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override;
 
     void onExecuteDrawable(std::unique_ptr<SkDrawable::GpuDrawHandler>) override {}
 
diff --git a/src/gpu/ganesh/gl/GrGLOpsRenderPass.h b/src/gpu/ganesh/gl/GrGLOpsRenderPass.h
index c136713..11642e5 100644
--- a/src/gpu/ganesh/gl/GrGLOpsRenderPass.h
+++ b/src/gpu/ganesh/gl/GrGLOpsRenderPass.h
@@ -42,8 +42,9 @@
 public:
     GrGLOpsRenderPass(GrGLGpu* gpu) : fGpu(gpu) {}
 
-    void inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override {
+    bool inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override {
         state->doUpload(upload);
+        return true;
     }
 
     void set(GrRenderTarget*, bool useMSAASurface, const SkIRect& contentBounds, GrSurfaceOrigin,
diff --git a/src/gpu/ganesh/mock/GrMockOpsRenderPass.h b/src/gpu/ganesh/mock/GrMockOpsRenderPass.h
index c4ca1467..f40c581 100644
--- a/src/gpu/ganesh/mock/GrMockOpsRenderPass.h
+++ b/src/gpu/ganesh/mock/GrMockOpsRenderPass.h
@@ -42,7 +42,7 @@
     }
 
     GrGpu* gpu() override { return fGpu; }
-    void inlineUpload(GrOpFlushState*, GrDeferredTextureUploadFn&) override {}
+    bool inlineUpload(GrOpFlushState*, GrDeferredTextureUploadFn&) override { return true; }
 
     int numDraws() const { return fNumDraws; }
 
diff --git a/src/gpu/ganesh/mtl/GrMtlOpsRenderPass.h b/src/gpu/ganesh/mtl/GrMtlOpsRenderPass.h
index 4b27579..73f5ef4 100644
--- a/src/gpu/ganesh/mtl/GrMtlOpsRenderPass.h
+++ b/src/gpu/ganesh/mtl/GrMtlOpsRenderPass.h
@@ -31,7 +31,7 @@
 
     void initRenderState(GrMtlRenderCommandEncoder*);
 
-    void inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override;
+    bool inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override;
     void submit();
 
 private:
diff --git a/src/gpu/ganesh/mtl/GrMtlOpsRenderPass.mm b/src/gpu/ganesh/mtl/GrMtlOpsRenderPass.mm
index 3bf4195..2460fd5 100644
--- a/src/gpu/ganesh/mtl/GrMtlOpsRenderPass.mm
+++ b/src/gpu/ganesh/mtl/GrMtlOpsRenderPass.mm
@@ -186,7 +186,7 @@
     }
 }
 
-void GrMtlOpsRenderPass::inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) {
+bool GrMtlOpsRenderPass::inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) {
     state->doUpload(upload);
 
     // If the previous renderCommandEncoder did a resolve without an MSAA store
@@ -197,6 +197,8 @@
         // create a new encoder at this point, though maybe not necessary.
         this->setupRenderCommandEncoder(nullptr);
     }
+
+    return true;
 }
 
 void GrMtlOpsRenderPass::initRenderState(GrMtlRenderCommandEncoder* encoder) {
diff --git a/src/gpu/ganesh/vk/GrVkOpsRenderPass.cpp b/src/gpu/ganesh/vk/GrVkOpsRenderPass.cpp
index 16d85a9..5f9fddd 100644
--- a/src/gpu/ganesh/vk/GrVkOpsRenderPass.cpp
+++ b/src/gpu/ganesh/vk/GrVkOpsRenderPass.cpp
@@ -616,11 +616,18 @@
     this->beginRenderPass(vkClearColor, loadFromResolve);
 }
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential Vulkan specification violation in Skia GrVkOpsRenderPass::inlineUpload

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: A missing logic guard in Skia’s Vulkan backend can cause invalid command sequences when operating on externally-managed command buffers. In Android WebView, this leads to driver-level undefined behavior within the unsandboxed browser process. An attacker-controlled renderer could potentially trigger this by exhausting the glyph atlas to force an inline upload.

Affected files:

  • third_party/skia/src/gpu/ganesh/vk/GrVkOpsRenderPass.cpp
  • third_party/skia/src/gpu/ganesh/vk/GrVkCommandBuffer.cpp
  • third_party/skia/src/gpu/ganesh/vk/GrVkRenderTarget.cpp

Estimated timestamp from git blame: 2019-09-05

Summary

A logic error in GrVkOpsRenderPass::inlineUpload in Skia’s Ganesh Vulkan backend allows for invalid Vulkan command sequences when drawing into a wrapped secondary command buffer. The method fails to check if it is operating on an externally-owned buffer before attempting to end it and submit it. This results in multiple Vulkan specification violations (VUIDs) and potential driver-level memory corruption.

Root Cause

In third_party/skia/src/gpu/ganesh/vk/GrVkOpsRenderPass.cpp, the inlineUpload method lacks a wrapsSecondaryCommandBuffer() guard. Other methods that interact with secondary command buffers, such as submit() and onEnd(), correctly include this check to prevent improper operations on buffers owned by external systems (like Android’s HWUI).

void GrVkOpsRenderPass::inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) {
    if (!fCurrentRenderPass) { ...; return; }
    if (fCurrentSecondaryCommandBuffer) {
        // Missing wrapsSecondaryCommandBuffer() check here
        fCurrentSecondaryCommandBuffer->end(fGpu);
        fGpu->submitSecondaryCommandBuffer(std::move(fCurrentSecondaryCommandBuffer));
    }
    fGpu->endRenderPass(fRenderTarget, fOrigin, fBounds);
    state->doUpload(upload, true);
    this->addAdditionalRenderPass(false);
}

When wrapsSecondaryCommandBuffer() is true, the following sequence of invalid operations occurs in release builds:

  1. Invalid vkEndCommandBuffer: Skia calls end() on a secondary command buffer it does not own.
  2. VUID-vkCmdExecuteCommands-pCommandBuffers-00100: Skia’s primary command buffer attempts to execute the secondary buffer without an active render pass.
  3. VUID-vkCmdEndRenderPass-renderpass: Skia attempts to end a render pass on its primary command buffer despite no render pass being active on that buffer.
  4. State Corruption: addAdditionalRenderPass(false) proceeds despite being invalid for external render targets, leading to further desynchronization of the Vulkan state machine.

Potential Attack Path

This issue is particularly relevant to Android WebView, where GPU operations run within the unsandboxed browser process. A potential attack could proceed as follows:

  1. A compromised renderer process saturates the GrDrawOpAtlas (e.g., by sending a flood of unique glyphs or small textures).
  2. This forces an inlineUpload to occur within the root render pass.
  3. In the context of WebView, the root render pass often uses a wrapped secondary command buffer provided by the Android framework.
  4. The missing guard causes the invalid Vulkan command sequence described above, leading to driver-level undefined behavior.
  5. Because the GPU thread in this context is unsandboxed, an attacker might leverage the resulting memory corruption to achieve code execution in the browser process.

Suggested Fix

Add a wrapsSecondaryCommandBuffer() guard to GrVkOpsRenderPass::inlineUpload to match the logic used in submit(). If the pass wraps a secondary command buffer, the method should handle the upload without attempting to end the current command buffer or the primary render pass record.

Evaluated with Chrome root at commit: 1a8d40fc44df2088d5945c0bf53584038aa1614a

Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:

  • If you are familiar with the severity guidelines, you may adjust the severity.
  • If this is a false positive, and there’s no work to be done, please close as WAI.
  • If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.

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.

View on issue tracker
On This Page