6dca6330dba27ff589985db50e5e259f71a44f61  [WebGPU] drawIndirect/drawIndexedIndirect clamp into a single per-Buffer scratch slot, racing every other indirect draw in the same render pass

The bug at a glance

The bug is reachable from WebGPU-enabled web content (or a WebGPU render bundle) with no special privileges: two drawIndirect/drawIndexedIndirect calls out of one arguments buffer in a single render pass are ordinary API usage. However, it lives entirely in the sandboxed WebGPU/GPUProcess Metal backend and its direct observable effect is corrupted indirect draw arguments feeding a GPU draw, producing wrong rendering and, in the reported case, an unrecoverable AGX tiler timeout / device loss (DoS). Because the corrupted values are GPU-side draw counts rather than a CPU-controllable pointer, and the win is primarily denial-of-service within the GPU process rather than a clean memory-write primitive, severity is Medium; the 7.1 reflects easy web reachability against a GPU-process asset with integrity/availability impact but no established CPU-side corruption.

The whole clamp mechanism exists to make indirect draws safe, so it is a nice irony that its own scratch storage was the unsafe part. Every WebGPU Buffer with INDIRECT usage got exactly one per-Buffer clamp slot, sized for a single arguments struct and always handed back at offset 0, so N indirect draws out of one arguments buffer all validated into the same bytes and all fetched their arguments from the same bytes. The barrier story is what makes it genuinely un-fixable in place: emitMemoryBarrier() can order a draw’s own clamp write against that draw’s fetch, but MTLRenderStages has no name for “indirect argument fetch,” so no barrier can ever order draw N’s fetch against draw N+1’s clamp store into the shared slot, and on Apple8+ the memoryBarrierLimit is UINT32_MAX so splitRenderPass never even separates them. The result is draw N reading draw N+1’s counts, or a torn mixture of four independent stores (vertexCount, instanceCount, vertexStart, baseInstance), and a large-instance draw running with arguments its own bounds no longer describe. The fix is almost anticlimactic, per-draw pooled scratch, which is exactly what the batched executeBundles path and the non-indirect index clamp already did.

Root cause

WebGPU on Metal cannot trust application-supplied indirect draw arguments, so RenderPassEncoder::clampIndirectBufferToValidValues and clampIndirectIndexBufferToValidValues run a small clamp vertex shader that reads the app’s MTLDrawPrimitivesIndirectArguments, computes safe vertex/instance counts (from computeMininumVertexInstanceCount), and writes the clamped arguments into a scratch MTLBuffer that the real draw then consumes via drawPrimitives:indirectBuffer:. The vulnerable state is where that scratch lived: each Buffer allocated a single m_indirectBuffer and m_indirectIndexedBuffer in its constructor (when WGPUBufferUsage_Indirect was set), each sized for exactly one arguments struct, and both clamp paths returned that slot at offset 0. So every clamping draw against a given arguments Buffer was issued as drawPrimitives:indirectBuffer:<sameSlot> indirectBufferOffset:0.

The clamp path is taken whenever computeMininumVertexInstanceCount can lower a minimum, i.e. whenever the pipeline declares a required vertex buffer layout with positive stride and at least one attribute (zero-attribute/undefined-stride layouts are dropped in createVertexDescriptor and early-out to the app’s own buffer). So two ordinary drawIndirect calls in one pass with such a pipeline both enter the clamp path and both target the one shared slot.

Why it is unsafe (ordering/aliasing): emitMemoryBarrier() emits a vertex->vertex MTLBarrierScopeBuffers barrier between a draw’s own clamp dispatch and that same draw’s indirect draw, covering the read-after-write within one draw. Nothing is emitted between draw N’s indirect argument fetch and draw N+1’s clamp dispatch. On Apple8 and later memoryBarrierLimit is UINT32_MAX unless shader validation is on (HardwareCapabilities.mm:264), so splitRenderPass() never fires to separate them either. Draw N+1’s clamp vertex shader therefore stores into the very bytes draw N is fetching its arguments from, with no ordering, and because MTLRenderStages cannot name an indirect argument fetch no barrier can express the constraint. Since the clamp shader writes vertexCount, instanceCount, vertexStart and baseInstance as four separate stores, draw N can observe draw N+1’s counts or a torn mix, producing a large-instance-count draw whose arguments its own bounds no longer describe, consistent with the reported AGX tiler progress-timeout / GPU hang. Compounding it, Buffer::m_indirectCache was a single record keyed on (indirectOffset, minVertexCount, minInstanceCount, drawType), so consecutive draws at different offsets missed the cache every time and each re-ran the clamp into the shared slot.

What the fix changes: the per-Buffer m_indirectBuffer/m_indirectIndexedBuffer slots and the m_indirectCache are removed entirely. Both direct clamp paths now allocate per-draw scratch via RenderPassEncoder::newZeroedIndirectScratch -> Queue::newTemporaryBufferWithBytes, which bump-allocates a distinct 64-byte-aligned offset out of a pooled buffer (satisfying Metal’s 4-byte indirectBufferOffset requirement with headroom), and return (scratch, offset) instead of (slot, 0). Because each draw’s clamped arguments now live at their own address, draw N+1’s clamp can no longer overwrite what draw N fetches. The recomputation cache and the skip paths (skippedDrawIndirectValidation/skippedDrawIndirectIndexedValidation, takeSlowIndirectValidationPath/takeSlowIndirectIndexValidationPath, indirectBufferRequiresRecomputation/Recomputed, verifyIndirectBufferData) are deleted because a cache hit would have to hand back the same recycled pooled scratch it validated; every clamping indirect draw now runs its own point dispatch. To avoid N device-loss readback handlers, RenderPassEncoder::trackIndirectDeviceLostCheck accumulates scratch records in a ThreadSafeRefCounted IndirectDeviceLostChecks holder and installs a single completion handler on the first clamp of the pass. Finally drawIndirect/drawIndexedIndirect now bound offset + sizeof(args) against the buffer length (checkedSum) instead of size alone, because the arguments now sit at a non-zero offset in pooled scratch, and a nil buffer from a failed clamp (length 0) is still rejected.

Key code

clampIndirectBufferToValidValues: shared per-Buffer slot at offset 0 becomes per-draw pooled scratch.

// before: one slot per Buffer, always offset 0 -> every draw aliases it
// if (!indirectBuffer.indirectBufferRequiresRecomputation(indirectOffset, minVertexCount, minInstanceCount)) {
//     indirectBuffer.skippedDrawIndirectValidation(encoder.parentEncoder(), indirectOffset, minVertexCount, minInstanceCount);
//     return std::make_pair(indirectBuffer.indirectBuffer(), 0ull);
// }
// encoder.setVertexBuffer(renderCommandEncoder, indirectBuffer.indirectBuffer(), 0, 1);
// ...
// checkForIndirectDrawDeviceLost(device, encoder, indirectBuffer.indirectBuffer());
// return std::make_pair(indirectBuffer.indirectBuffer(), 0ull);

// after: distinct 64-byte-aligned scratch per draw; nothing to alias
auto [scratch, scratchOffset] = newZeroedIndirectScratch(device, sizeof(WebKitMTLDrawPrimitivesIndirectArguments));
if (!scratch)
    return std::make_pair(nil, 0ull);
encoder.setVertexBuffer(renderCommandEncoder, indirectBuffer.buffer(), indirectOffset, 0);
encoder.setVertexBuffer(renderCommandEncoder, scratch, scratchOffset, 1);
uint32_t data[] = { minVertexCount, minInstanceCount };
encoder.setVertexBytes(renderCommandEncoder, asByteSpan(data), 2);
[renderCommandEncoder drawPrimitives:MTLPrimitiveTypePoint vertexStart:0 vertexCount:1];
encoder.emitMemoryBarrier(renderCommandEncoder);
splitEncoder = true;
encoder.parentEncoder().addBuffer(scratch);
encoder.trackIndirectDeviceLostCheck(scratch, scratchOffset, nil);
return std::make_pair(scratch, scratchOffset);

Patch walkthrough

• Source/WebGPU/WebGPU/RenderPassEncoder.mm — The heart of the fix. clampIndirectBufferToValidValues and clampIndirectIndexBufferToValidValues stop using indirectBuffer.indirectBuffer()/indirectIndexedBuffer() at offset 0 and instead call newZeroedIndirectScratch to obtain per-draw (scratch, offset) pairs, set them as the clamp shader’s output/fetch buffers, and return (scratch, offset). The recomputation-skip fast paths are deleted. checkForIndirectDrawDeviceLost is replaced by trackIndirectDeviceLostCheck, which lazily creates a ThreadSafeRefCounted IndirectDeviceLostChecks, appends each scratch record, and installs exactly one addCompletedHandler per pass that iterates entries and calls loseTheDevice if any args.lostOrOOBRead is set. newZeroedIndirectScratch is moved earlier and given an inline 32-byte Vector capacity.
• Source/WebGPU/WebGPU/RenderPassEncoder.h — Adds trackIndirectDeviceLostCheck and the IndirectDeviceLostChecks struct (a ThreadSafeRefCounted holder of {RetainPtr scratch, offset, RetainPtr alsoRetain} entries) plus the m_indirectDeviceLostChecks member, with RetainPtr/ThreadSafeRefCounted includes. This is the single-handler-per-pass accounting that per-draw scratch made necessary.
• Source/WebGPU/WebGPU/Buffer.h — Removes the per-Buffer scratch API and state: indirectBuffer()/indirectIndexedBuffer() accessors, the m_indirectBuffer/m_indirectIndexedBuffer members, the IndirectArgsCache struct and m_indirectCache, the requiresRecomputation/Recomputed helpers, skippedDrawIndirectValidation and takeSlowIndirectValidationPath declarations. These all existed to manage and re-verify the shared slot.
• Source/WebGPU/WebGPU/Buffer.mm — Deletes the corresponding definitions: the constructor no longer allocates m_indirectBuffer/m_indirectIndexedBuffer for INDIRECT buffers; indirectBuffer(), verifyIndirectBufferData, takeSlowIndirectValidationPath, takeSlowIndirectIndexValidationPath, skippedDrawIndirectValidation, indirectBufferRequiresRecomputation/Recomputed are removed; and indirectBufferInvalidated no longer resets m_indirectCache. The non-indirect index validation cache and m_mustTakeSlowIndexValidationPath are deliberately left intact.
• Source/WebGPU/WebGPU/RenderPassEncoder.mm (drawIndirect/drawIndexedIndirect range checks) — Both draw entry points now compute checkedSum<uint64_t>(offset, sizeof(args)) and reject when it overflows or exceeds mtlIndirectBuffer.length, replacing the old ’length < sizeof(args)’ check. This is required because clamped arguments now live at a non-zero pooled offset, and it also subsumes the removed !indirectBuffer early-out since a failed clamp returns a nil (length 0) buffer.
• LayoutTests/fast/webgpu/draw-indirect-repeated-shared-args-buffer.html — Correctness test: two drawIndirect calls in one pass read two argument records (offsets 0 and 16) from one GPUBuffer, using a pipeline with a real @location vertex buffer so both take the clamp path, each selecting a differently coloured triangle via firstVertex and scissored to its own pixel. With per-Buffer scratch, the second draw’s clamp clobbers the first’s record and the left pixel renders the wrong colour; the expected result asserts left=green, right=blue. The commit notes this is a shape/correctness test, not a deterministic pre-patch failure, since the underlying defect is a race.

Background

Indirect draw argument clamping — In WebGPU-on-Metal, drawIndirect/drawIndexedIndirect take their vertex/instance/index counts from a GPU buffer the app can also write. WebKit inserts a GPU ‘clamp’ shader that reads those arguments, clamps them to the bound resources’ real sizes, and writes safe values into a scratch buffer that the actual draw consumes, so the GPU never draws past the vertex/index buffers.

emitMemoryBarrier / MTLRenderStages limitation — emitMemoryBarrier() issues an MTLBarrierScopeBuffers vertex->vertex barrier so a clamp dispatch’s writes are visible to the draw that consumes them. But Metal’s barriers are named by render stages, and there is no stage naming the fixed-function indirect argument fetch, so no barrier can order one draw’s argument fetch against another draw’s clamp write.

splitRenderPass / memoryBarrierLimit — When too many barriers are needed, WebKit splits the render pass into separate encoders. On Apple8+ hardware memoryBarrierLimit is UINT32_MAX (unless shader validation is on), so the pass is effectively never split; Apple7 gets 512 (via mac2()), which merely bounds how many draws share the slot between splits rather than fixing the aliasing.

newTemporaryBufferWithBytes / per-draw scratch — Queue::newTemporaryBufferWithBytes bump-allocates a distinct 64-byte-aligned region out of a pooled MTLBuffer and returns (buffer, offset). It is the allocation the non-indirect index clamp and the batched executeBundles helpers already used; the fix makes the direct indirect clamp paths use it too so each draw gets private argument storage.

Vulnerability window

1. Setup — Web content creates one GPUBuffer with INDIRECT usage holding two argument records and a pipeline that declares a required vertex buffer layout (positive stride, >=1 attribute), so clamp is not short-circuited.
2. Draw N clamp — drawIndirect(argsBuffer, offsetN) enters clampIndirectBufferToValidValues, which (pre-patch) writes clamped arguments into the Buffer’s single m_indirectBuffer at offset 0 and emits a barrier before draw N’s fetch.
3. Draw N fetch vs Draw N+1 clamp — draw N’s indirect draw fetches arguments from the shared slot; draw N+1’s clamp dispatch stores its clamped arguments into the same slot. No barrier orders these, and on Apple8+ the pass is not split.
4. Torn / wrong arguments — Because the clamp writes vertexCount/instanceCount/vertexStart/baseInstance as four separate stores, draw N reads draw N+1’s counts or a torn mixture, so a draw runs with arguments its bounds no longer describe.
5. Effect — The bad counts (e.g. a large instance count) drive a runaway GPU draw, causing wrong rendering and, as reported, an AGX tiler progress timeout / unrecoverable GPU hang (device loss).
6. Fixed — Each clamping draw now allocates its own pooled scratch at a distinct offset, so no draw’s fetch aliases another’s clamp write; a single per-pass completion handler still performs the device-loss readback.

Proof of concept

Reconstructed from the committed layout test. It is a correctness oracle for the shared-slot aliasing: two drawIndirect calls read distinct records from one arguments buffer via a clamp-taking pipeline; pre-patch the second draw’s clamp writes the same scratch bytes the first draw fetches from, so the scissored left pixel can show the wrong colour. The commit is explicit that this is not a deterministic pre-patch failure because the defect is a race; to reproduce the GPU-hang the bug report’s own reproduction (many indirect draws driving a runaway instance count) is the stronger trigger. No CPU memory-corruption primitive is claimed.

// Reconstructed from fast/webgpu/draw-indirect-repeated-shared-args-buffer.html.
// Two drawIndirect calls, one args buffer, a pipeline with a real vertex buffer
// layout so both take the clamp path and (pre-patch) share one scratch slot.
const adapter = await navigator.gpu.requestAdapter();
const device = await adapter.requestDevice();
const format = 'rgba8unorm';
const module = device.createShaderModule({ code: `
  struct VSOut { @builtin(position) position: vec4f, @location(0) color: vec4f, };
  @vertex fn vs(@location(0) p: vec2f, @location(1) c: vec4f) -> VSOut { return VSOut(vec4f(p,0,1), c); }
  @fragment fn fs(i: VSOut) -> @location(0) vec4f { return i.color; }` });
const pipeline = device.createRenderPipeline({
  layout: 'auto',
  vertex: { module, entryPoint: 'vs', buffers: [{ arrayStride: 24, attributes: [
    { shaderLocation: 0, offset: 0, format: 'float32x2' },
    { shaderLocation: 1, offset: 8, format: 'float32x4' } ] }] },
  fragment: { module, entryPoint: 'fs', targets: [{ format }] },
  primitive: { topology: 'triangle-list' } });
const v = [];
for (const c of [[0,1,0,1],[0,0,1,1]]) for (const p of [[-1,-3],[-1,1],[3,1]]) v.push(p[0],p[1],...c);
const vertexBuffer = device.createBuffer({ size: 4*v.length, usage: GPUBufferUsage.VERTEX|GPUBufferUsage.COPY_DST });
device.queue.writeBuffer(vertexBuffer, 0, new Float32Array(v));
// two arg records in one buffer, read at offsets 0 and 16
const args = new Uint32Array([3,1,0,0, 3,1,3,0]);
const argsBuffer = device.createBuffer({ size: args.byteLength, usage: GPUBufferUsage.INDIRECT|GPUBufferUsage.COPY_DST });
device.queue.writeBuffer(argsBuffer, 0, args);
const texture = device.createTexture({ size:[2,1], format, usage: GPUTextureUsage.RENDER_ATTACHMENT|GPUTextureUsage.COPY_SRC });
const enc = device.createCommandEncoder();
const pass = enc.beginRenderPass({ colorAttachments:[{ view: texture.createView(), loadOp:'clear', storeOp:'store', clearValue:{r:1,g:0,b:0,a:1} }] });
pass.setPipeline(pipeline); pass.setVertexBuffer(0, vertexBuffer);
pass.setScissorRect(0,0,1,1); pass.drawIndirect(argsBuffer, 0);
pass.setScissorRect(1,0,1,1); pass.drawIndirect(argsBuffer, 16);
pass.end();
device.queue.submit([enc.finish()]);
// pre-patch: left pixel may render the wrong colour (second clamp clobbers the first record).

Exploitation

1. Force the clamp path and the alias — From web content, issue repeated drawIndirect/drawIndexedIndirect out of one arguments buffer in a single render pass with a pipeline declaring a required vertex buffer layout, so every draw enters the clamp path and (pre-patch) shares one per-Buffer slot. This is straightforward and needs no exotic primitives.
2. Win the race toward corrupt counts — Because the four argument fields are stored separately and unordered relative to the neighbouring fetch, some draw fetches a torn or wrong set of counts. Timing is nondeterministic, but repeating many draws in the pass makes an observable clobber likely; the practical, reliable outcome is a large-instance/large-vertex draw running past its intended bounds.
3. Impact: device loss / GPU hang — A runaway indirect draw triggers the AGX tiler progress timeout and an unrecoverable GPU hang (device loss), a denial of service affecting the GPU process and potentially the system compositor. There is no demonstrated path from these corrupted GPU-side draw counts to a CPU-controllable read/write primitive, so escalation beyond DoS/rendering-integrity is not established.

Detection & hunting

For defenders and SOC / detection engineers:

• AGX tiler progress timeout / device loss — Repeated WGPUDeviceLostReason device-loss events or GPU restart / ‘AGX tiler progress timeout’ entries in the GPUProcess logs correlated with pages issuing many indirect draws in one pass are the primary in-the-wild signal.
• Indirect-draw usage shape — A WebGPU render pass with multiple drawIndirect/drawIndexedIndirect calls sourcing one INDIRECT GPUBuffer at differing offsets, under a pipeline with a required (positive-stride, attributed) vertex buffer layout, is the fingerprint of the vulnerable path; fuzzers should generate exactly this shape.
• Rendering-integrity oracle — A conformance/regression test that scissors each of several same-buffer indirect draws to its own pixel and checks colours (as the committed layout test does) will flag clobbered argument records on affected builds.

Audit directions

• Other per-Buffer/per-object GPU scratch slots — Hunt for any remaining single scratch buffer owned by a resource and reused across multiple draws/dispatches in a pass (the m_indirectBuffer pattern this patch removed); each is a candidate for the same intra-pass aliasing race.
• Barrier coverage of GPU-generated arguments — Review every place a GPU shader writes data later consumed by a fixed-function stage that cannot be named by MTLRenderStages (indirect argument fetch, index buffer fetch); confirm ordering is achieved by separate allocations or pass splits rather than by a barrier that cannot express the constraint.
• Recomputation/skip caches keyed too coarsely — Audit remaining validation caches (e.g. the non-indirect index cache and m_mustTakeSlowIndexValidationPath left intact here) for keys that miss on legitimate parameter variation and, conversely, for hits that would hand back recycled/pooled storage.
• Offset+size range checks for pooled scratch — Now that clamped arguments live at non-zero pooled offsets, verify every consumer of indirect/scratch buffers bounds offset + sizeof(args) (checkedSum) against buffer length rather than size alone, and that nil (length-0) buffers from failed clamps are rejected everywhere.