Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix buffer overflow in RenderPass command batching (#9619)

Browse Source 
The command batching logic in RenderPass::Executor::execute calculates a
conservative `maxCommandSizeInBytes` to determine how many commands can
safely fit in the driver's circular buffer.

However, this calculation failed to account for the dynamic memory
allocation incurred by the Per-Material descriptor set binding (via
`MaterialInstance::use`). This binding allocates a `DescriptorSetOffsetArray`
in the command stream, which was unbudgeted.

In scenarios with frequent material and scissor changes, the accumulated
deficit from these unbudgeted allocations could exceed the buffer 
capacity, leading to a crash.

This change:
1. Adds the missing accounting for the Per-Material dynamic allocation 
   to `maxCommandSizeInBytes`.
2. Adds a 20% safety margin to the calculated size to robustly handle
   alignment padding and worst-case scenarios.


FIXES=[474264976]
This commit is contained in:
Mathias Agopian
2026-01-21 21:03:49 -08:00
committed by GitHub
parent febcfc8f2d
commit 57767b7f27
1 changed files with 8 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
filament/src/RenderPass.cpp
View File

@@ -940,31 +940,32 @@ void RenderPass::Executor::execute(FEngine const& engine, DriverApi& driver,
// Maximum space occupied in the CircularBuffer by a single `Command`. This must be
// reevaluated when the inner loop below adds DriverApi commands or when we change the
// CommandStream protocol. Currently, the maximum is 248 bytes.
// CommandStream protocol. Currently, the maximum is 272 bytes.
// The batch size is calculated by adding the size of all commands that can possibly be
// emitted per draw call:
constexpr size_t maxCommandSizeInBytes =
sizeof(COMMAND_TYPE(scissor)) +
sizeof(COMMAND_TYPE(bindDescriptorSet)) +
sizeof(COMMAND_TYPE(bindDescriptorSet)) +
sizeof(COMMAND_TYPE(bindDescriptorSet)) + backend::CustomCommand::align(sizeof(NoopCommand) + 4) +
sizeof(COMMAND_TYPE(bindPipeline)) +
sizeof(COMMAND_TYPE(bindRenderPrimitive)) +
sizeof(COMMAND_TYPE(bindDescriptorSet)) + backend::CustomCommand::align(sizeof(NoopCommand) + 8) +
sizeof(COMMAND_TYPE(setPushConstant)) +
sizeof(COMMAND_TYPE(draw2));
// Add 20% margin for safety
constexpr size_t safeCommandSizeInBytes = maxCommandSizeInBytes + (maxCommandSizeInBytes / 5);
// Number of Commands that can be issued and guaranteed to fit in the current
// CircularBuffer allocation. In practice, we'll have tons of headroom especially if
// skinning and morphing aren't used. With a 2 MiB buffer (the default) a batch is
// 6553 commands (i.e. draw calls).
size_t const batchCommandCount = capacity / maxCommandSizeInBytes;
// CircularBuffer allocation. With a 2 MiB buffer (the default) a batch is
// 6432 commands (i.e. draw calls).
size_t const batchCommandCount = capacity / safeCommandSizeInBytes;
while(first != last) {
Command const* const batchLast = std::min(first + batchCommandCount, last);
// actual number of commands we need to write (can be smaller than batchCommandCount)
size_t const commandCount = batchLast - first;
size_t const commandSizeInBytes = commandCount * maxCommandSizeInBytes;
size_t const commandSizeInBytes = commandCount * safeCommandSizeInBytes;
// check we have enough capacity to write these commandCount commands, if not,
// request a new CircularBuffer allocation of `capacity` bytes.