Repository navigation
Fix heap buffer overflow for high valence subdivision vertices - #632
Open
stefanatwork wants to merge 9 commits into
Open
stefanatwork wants to merge 9 commits into
stefanatwork wants to merge 9 commits into
Conversation
The one-ring construction in CatmullClark1RingT::init() and GeneralCatmullClark1RingT::init() wrote into the fixed capacity ring buffers (MAX_RING_FACE_VALENCE / MAX_RING_EDGE_VALENCE) without any runtime bound, only asserts guarded these writes. A vertex shared by more than MAX_RING_FACE_VALENCE faces (or a ring with more than MAX_RING_EDGE_VALENCE edges) thus corrupted the heap in release builds. The ring construction now stops when the ring buffers are exhausted and clamps the evaluation start indices accordingly, so no out of bounds write can happen for any topology. In addition patches whose rings exceed the supported size limits can now get detected in constant time through a flag that is calculated at commit time for every topology. rtcInterpolate and rtcInterpolateN, which did not perform any validity check before, use this flag to reject such patches and return zeros. The ray tracing path already rejected these patches through SubdivMesh::valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
svenwoop
requested changes
Sep 23, 2026
|
|
||
| do | ||
| { | ||
| /* stop when the ring buffers are exhausted to avoid writing out of bounds */ |
Collaborator
There was a problem hiding this comment.
Do you understand all these changes in this file? As we now validate the size of the ring upfront, there should be no need for these checks at all!?
svenwoop
requested changes
Sep 23, 2026
Fold the high-valence subdivision patch check into the existing Topology::valid() path instead of carrying a separate hasValidSizes() query at interpolation sites. The cached half-edge flag is now a bool and interpolation uses the same validity model as the builders, while still checking the selected attribute topology when vertex attributes are bound to a non-default topology. With invalid patches filtered before PatchEval, remove the defensive ring truncation code from the Catmull-Clark ring constructors and keep assertions for the invariants that commit-time validation guarantees. This avoids constructing partially truncated rings and keeps invalid patch handling centralized in the topology validity path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
svenwoop
requested changes
Sep 23, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
svenwoop
requested changes
Oct 13, 2026
Keep m128i() and m128d() available when compiling with Emscripten. SSE2 equality, shuffle, and masked integer loads call the explicit accessors regardless of platform. Continue excluding the implicit integer and double conversion operators for Emscripten, and leave the MSVC ARM64 path unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The one-ring construction in CatmullClark1RingT::init() and GeneralCatmullClark1RingT::init() wrote into the fixed capacity ring buffers (MAX_RING_FACE_VALENCE / MAX_RING_EDGE_VALENCE) without any runtime bound, only asserts guarded these writes. A vertex shared by more than MAX_RING_FACE_VALENCE faces (or a ring with more than MAX_RING_EDGE_VALENCE edges) thus corrupted the heap in release builds.
The ring construction now stops when the ring buffers are exhausted and clamps the evaluation start indices accordingly, so no out of bounds write can happen for any topology.
In addition patches whose rings exceed the supported size limits can now get detected in constant time through a flag that is calculated at commit time for every topology. rtcInterpolate and rtcInterpolateN, which did not perform any validity check before, use this flag to reject such patches and return zeros. The ray tracing path already rejected these patches through SubdivMesh::valid.