Skip to content

Fix out-of-bounds read in blendLineCoverageMaskAvx2 - #592

Merged
treeform merged 2 commits into
treeform:masterfrom
Graveflo:avx-bug
Sep 13, 2026
Merged

treeform merged 2 commits into
treeform:masterfrom
Graveflo:avx-bug

Conversation

@Graveflo

Copy link
Copy Markdown
Contributor

FYI

  • discovered and implemented by AI (GPT-5.6-Sol & Opus 5)
  • test fails on master
  • does not seem to negatively affect performance

Summary

blendLineCoverageMaskAvx2 advances 32 pixels per iteration but guards the loop
with i < len - 16. An iteration can therefore start within 32 pixels of the
end and read 32 coverage bytes from a buffer holding only len, then blend and
store up to 16 pixels past the end of the coverage run.

Any MaskBlend fill whose path is wide enough to reach the vector loop can hit
it. Context.clip uses MaskBlend for every clip after the first, so this is
reachable from ordinary Canvas2D code.

Reproduction

import pixie

const w = 60
let
  image = newImage(w, w)
  paint = newPaint(SolidPaint)
  path = newPath()
paint.color = color(1, 0, 0, 1)
paint.blendMode = MaskBlend
path.circle(circle(vec2(w / 2, w / 2), w / 2))
image.fillPath(path, paint)
nim c -r --mm:arc -d:useMalloc -d:release --passC:-fsanitize=address --passL:-fsanitize=address repro.nim

Without a sanitizer the read lands on readable heap and the damage is visible
only as a few stray pixels past the right end of each scanline's coverage run —
which is why it has gone unnoticed.

The width that triggers it depends on where the 32-byte alignment prologue
leaves the index, so a single width is not a reliable reproduction.

@treeform

Copy link
Copy Markdown
Owner

Thanks for catching this! I reproduced both the coverage-buffer overread and writes past the requested pixel run.

I've made one small adjustment to the loop condition: i <= len - 32. That keeps the final complete 32-pixel block vectorized while still fixing the overread. The strict < comparison was 17–30% slower in the aligned circular-mask cases I benchmarked; <= removed that slowdown.

I also added a regression test covering 5,152 combinations of lengths, alignments, and coverage values. It checks the output against the scalar blend and checks that the surrounding pixels stay untouched. It fails on the original code and passes with the fix in C and C++ builds.

The update is pushed to this PR. I'll merge it once CI passes.

@treeform
treeform merged commit f0bb52f into treeform:master Sep 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants