Critical watch CVSS 8.3 chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Critical
Severity
8.3
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate implementation in Skia
DescriptionInappropriate implementation in Skia
ComponentSkia
Bug ClassLogic Error
Action tierwatch — corruption primitive but gated (uncommon) — not default-reachable
Computed by PatchGap's deterministic exploitability ruler from the fixing commit. patch-now = RCE-class & attacker-reachable in a default config, or exploited in the wild; watch = real security signal below that bar; noise = no security relevance.
Tracker513948178
Fix commitdefc3a5a9296 (skia) +53/-27
CWECWE-269
CVSS vectorCVSS:3.1/AV:N/AC:H/PR:N/UI:R/S:C/C:H/I:H/A:H
CISA KEVNot listed
CreditedGoogle
Reported2026-05-16 (per vendor advisory)
Fix landed2026-05-20 (public git)
Disclosed2026-05-27
Patch gap7 days — fix public in source before the advisory

Background

Inline upload (`GrDeferredTextureUploadFn`)
Ganesh’s mechanism for uploading texture data, such as atlas contents, in the middle of a flush. It runs between draws, just before the draw token that depends on it (fUploadBeforeToken).
`GrOpsRenderPass::inlineUpload`
the backend hook (GL, Vulkan, Metal, D3D, Mock) that performs such an upload while a render pass is open. On Vulkan this means ending the current secondary command buffer, uploading, and starting an additional render pass.
Wrapped secondary command buffer
a Vulkan render target whose secondary VkCommandBuffer is owned by an external client (wrapsSecondaryCommandBuffer()), so Skia records into it but must not end, submit or replace it.
`GrOpFlushState`
drives execution of recorded draws, interleaving them with pending inline uploads and issuing flush tokens.

Root Cause Analysis

Before the patch, GrOpsRenderPass::inlineUpload returned void, so GrOpFlushState had no way to learn that an upload had not happened. It always went on to build GrProgramInfo, bind the pipeline and textures, and drawMesh for the draw that depended on that upload. In GrVkOpsRenderPass::inlineUpload, the existing device-lost early return silently skipped the upload. More importantly, nothing stopped the upload sequence from running on a render pass that wraps an externally owned secondary command buffer. That path ends and submits fCurrentSecondaryCommandBuffer, uploads, then calls addAdditionalRenderPass, all of which manipulate a command buffer Skia does not own.

The patch closes both gaps. inlineUpload now returns bool, and the Vulkan override returns false for device-lost and for wrapsSecondaryCommandBuffer(), before touching the command buffer. The flush loop records drawUploadFailure, still attempts later uploads, and skips the dependent draw, counting it with incNumFailedDraws() instead of executing it. The GL, Metal, D3D and Mock backends simply return true, so their behavior is unchanged.

Key insight
A required precondition, the inline upload, could fail or be unsafe to perform, but the API discarded that fact, so Skia both mutated an externally owned Vulkan secondary command buffer and executed draws that depended on data it never uploaded. Making inlineUpload fallible, refusing on wrapped secondary command buffers, and skipping dependent draws on failure fixes both.

Attack Path

  1. Render into a wrapped secondary CB Content is rasterized by Ganesh on Vulkan into a render target that wraps an external secondary command buffer; that this is reachable from web content, for example through an embedder’s Vulkan draw functor, is inferred.
  2. Force an inline upload The content uses draw ops that need mid-flush texture uploads, such as atlas-backed glyphs or paths (inferred), so GrOpFlushState meets an upload whose fUploadBeforeToken matches the current draw token.
  3. Unsafe upload sequence Before the patch, GrVkOpsRenderPass::inlineUpload ended and submitted the current secondary command buffer and began an additional render pass even though the command buffer was externally owned. The patch now refuses this with wrapsSecondaryCommandBuffer().
  4. Draw with missing data Whether the upload was skipped (device lost) or unsafe, the dependent draw still bound textures and issued drawMesh. Consequences such as sampling stale or uninitialized texture memory, or corrupting the external command stream, are inferred and not shown in the diff.

Impact Assessment

The advisory classes this as a critical “Inappropriate implementation in Skia”. The demonstrated defect is in the GPU-process Ganesh flush path: Skia manipulated a command buffer it did not own and executed draws despite a failed upload. Preconditions are the Vulkan backend with a wrapped secondary command buffer (or a lost device) plus content that triggers inline uploads. Concrete outcomes such as GPU-memory disclosure or memory corruption in the GPU process are (inferred), and the advisory does not report in-the-wild exploitation.

Changed Functions

FunctionChangeNotes
if
src/gpu/ganesh/GrOpFlushState.cpp
modified
for
src/gpu/ganesh/GrOpFlushState.cpp
modified
if
src/gpu/ganesh/vk/GrVkOpsRenderPass.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
  • src/gpu/ganesh/vk/GrVkOpsRenderPass.h

Audit Directions

  • Void-returning preconditions
    Look for other Ganesh and Graphite backend hooks that return void but can fail (uploads, copies, transfers, onExecuteDrawable), and check whether callers proceed with dependent draws regardless.
  • Externally owned GPU objects
    Audit every Vulkan path that ends, submits, resets or restarts fCurrentSecondaryCommandBuffer or a render pass, and check that it bails out when wrapsSecondaryCommandBuffer() or a similar wrapped-resource flag is set.
  • Device-lost early returns
    Review early returns on isDeviceLost() that silently skip work, ensuring later steps do not assume the skipped work completed.
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);
 }
 
-void GrVkOpsRenderPass::inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) {
+bool GrVkOpsRenderPass::inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) {
     if (!fCurrentRenderPass) {
         SkASSERT(fGpu->isDeviceLost());
-        return;
+        return false;
     }
+
+    // Skia should not manipulate a wrapped (externally-owned) secondary command buffer, which
+    // is necessary in order to perform the requested inline upload.
+    if (this->wrapsSecondaryCommandBuffer()) {
+        return false;
+    }
+
     if (fCurrentSecondaryCommandBuffer) {
         fCurrentSecondaryCommandBuffer->end(fGpu);
         fGpu->submitSecondaryCommandBuffer(std::move(fCurrentSecondaryCommandBuffer));
@@ -629,9 +636,11 @@
 
     // We pass in true here to signal that after the upload we need to set the upload textures
     // layout back to VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL.
-    state->doUpload(upload, true);
+    state->doUpload(upload, /*shouldPrepareSurfaceForSampling=*/true);
 
     this->addAdditionalRenderPass(false);
+
+    return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/ganesh/vk/GrVkOpsRenderPass.h b/src/gpu/ganesh/vk/GrVkOpsRenderPass.h
index ad145a6..a74222a 100644
--- a/src/gpu/ganesh/vk/GrVkOpsRenderPass.h
+++ b/src/gpu/ganesh/vk/GrVkOpsRenderPass.h
@@ -46,7 +46,8 @@
 
     ~GrVkOpsRenderPass() override;
 
-    void inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override;
+    // Returns true upon successful upload and false upon failure.
+    bool inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override;
 
     void onExecuteDrawable(std::unique_ptr<SkDrawable::GpuDrawHandler>) override;
 
Loading diff…

Original Bug Report

reported by [email protected]

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