From 1029e9a303b1b2a039b4f746565096df2088531b Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Mon, 21 Sep 2026 16:23:17 +0200 Subject: [PATCH 1/6] Validate face buffer of subdivision meshes The half edge of a face is looked up through the prefix sum of the face valences stored in the face buffer, but this buffer was never validated. A face with zero vertices produced a start offset that pointed behind the half edge array, thus dereferencing the half edge of such a face during commit read out of bounds. A face buffer whose valences sum up to more than the size of an index buffer caused the half edge array, which is sized through the index buffer, to get written out of bounds. The sum was further accumulated in 32 bit, thus it could also wrap around. The face buffer now gets validated before the half edge structures are built, and inconsistent face and index buffers raise an RTC_ERROR_INVALID_OPERATION error. The check is performed before any state is updated, thus a rejected geometry is left unmodified. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 6 +++ doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md | 6 +++ kernels/common/scene_subdiv_mesh.cpp | 46 ++++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/README.md b/README.md index a6f2c6667c..d9379b2e8b 100644 --- a/README.md +++ b/README.md @@ -3627,6 +3627,12 @@ topologies, which means that the `n`-th primitive always has the same number of vertices (e.g. being a triangle or a quad) for each topology. However, the indices of the topologies themselves may be different. +The face buffer has to be consistent with the index buffers, thus each +face must have at least 3 vertices and the sum of all face vertex +counts must not exceed the size of any index buffer. Committing a +geometry that violates this raises an `RTC_ERROR_INVALID_OPERATION` +error. + #### EXIT STATUS {#exit-status} On failure `NULL` is returned and an error code is set that can be diff --git a/doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md b/doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md index ec9e2fc3a5..c1bd7d7caa 100644 --- a/doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md +++ b/doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md @@ -163,6 +163,12 @@ topologies, which means that the `n`-th primitive always has the same number of vertices (e.g. being a triangle or a quad) for each topology. However, the indices of the topologies themselves may be different. +The face buffer has to be consistent with the index buffers, thus each +face must have at least 3 vertices and the sum of all face vertex +counts must not exceed the size of any index buffer. Committing a +geometry that violates this raises an `RTC_ERROR_INVALID_OPERATION` +error. + #### EXIT STATUS On failure `NULL` is returned and an error code is set that can be diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 4dc2080d36..2349409447 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -11,6 +11,7 @@ #include "../../common/algorithms/parallel_sort.h" #include "../../common/algorithms/parallel_prefix_sum.h" #include "../../common/algorithms/parallel_for.h" +#include "../../common/algorithms/parallel_reduce.h" /*! maximum number of user vertex buffers for subdivision surfaces */ #define RTC_MAX_USER_VERTEX_BUFFERS 65536 @@ -633,6 +634,10 @@ namespace embree /* allocate half edge array */ halfEdges.resize(mesh->numEdges()); + /* the index buffer of each topology has to be large enough for all half edges */ + if (vertexIndices.size() < mesh->numHalfEdges) + throw_RTCError(RTC_ERROR_INVALID_OPERATION,"index buffer of subdivision mesh too small for face buffer"); + /* check if we have to recalculate the half edges */ bool recalculate = false; recalculate |= vertexIndices.isLocalModified(); @@ -688,10 +693,51 @@ namespace embree << std::endl; } + /*! statistics gathered over the face buffer to validate it */ + struct FaceBufferStats + { + uint64_t numHalfEdges = 0; + unsigned int minValence = unsigned(-1); + }; + void SubdivMesh::initializeHalfEdgeStructures () { double t0 = getSeconds(); + /* The half edge of a face is looked up through the prefix sum of the face + valences. We thus have to validate the face buffer before anything else, + as otherwise a face with zero vertices or a face buffer that does not + match the index buffer would index the half edge array out of bounds. */ + if (faceVertices.isLocalModified()) + { + const FaceBufferStats stats = parallel_reduce + (size_t(0), numFaces(), size_t(1024), FaceBufferStats(), + [&](const range& r) -> FaceBufferStats + { + FaceBufferStats stats; + for (size_t f=r.begin(); f FaceBufferStats { + FaceBufferStats stats; + stats.numHalfEdges = a.numHalfEdges + b.numHalfEdges; + stats.minValence = min(a.minValence,b.minValence); + return stats; + }); + + if (numFaces() && stats.minValence == 0) + throw_RTCError(RTC_ERROR_INVALID_OPERATION,"subdivision face with zero vertices"); + + if (stats.numHalfEdges > (uint64_t)numEdges()) + throw_RTCError(RTC_ERROR_INVALID_OPERATION,"index buffer of subdivision mesh too small for face buffer"); + + if (stats.numHalfEdges > (uint64_t)0xFFFFFFFF) + throw_RTCError(RTC_ERROR_INVALID_OPERATION,"too many edges in subdivision mesh"); + } + invalid_face.resize(numFaces()*numTimeSteps); /* calculate start edge of each face */ From cb5789f5dc7074f65f879a61369e1d1da34a3077 Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Mon, 12 Oct 2026 12:24:35 +0200 Subject: [PATCH 2/6] Added test for subd face valence --- tutorials/verify/verify.cpp | 70 +++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tutorials/verify/verify.cpp b/tutorials/verify/verify.cpp index a64ff6c6ed..e97b20a8ef 100644 --- a/tutorials/verify/verify.cpp +++ b/tutorials/verify/verify.cpp @@ -1173,6 +1173,75 @@ namespace embree BBox> bbv4d; }; + struct SubdivZeroValenceTest : public VerifyApplication::Test + { + SubdivZeroValenceTest(std::string name, int64_t isa) + : VerifyApplication::Test(std::move(name),isa,VerifyApplication::TEST_SHOULD_PASS) + {} + + VerifyApplication::TestReturnValue run(VerifyApplication* state, bool silent) + { + std::string cfg = state->rtcore + ",isa="+stringOfISA(isa); + RTCDeviceRef device = rtcNewDevice(cfg.c_str()); + errorHandler(nullptr,rtcGetDeviceError(device)); + + auto createGeometry = [&](bool zeroValenceFace) + { + RTCGeometry geometry = rtcNewGeometry(device,RTC_GEOMETRY_TYPE_SUBDIVISION); + float* vertices = static_cast(rtcSetNewGeometryBuffer( + geometry,RTC_BUFFER_TYPE_VERTEX,0,RTC_FORMAT_FLOAT3,3*sizeof(float),4)); + unsigned int* faces = static_cast(rtcSetNewGeometryBuffer( + geometry,RTC_BUFFER_TYPE_FACE,0,RTC_FORMAT_UINT,sizeof(unsigned int), + zeroValenceFace ? 2 : 1)); + unsigned int* indices = static_cast(rtcSetNewGeometryBuffer( + geometry,RTC_BUFFER_TYPE_INDEX,0,RTC_FORMAT_UINT,sizeof(unsigned int),4)); + if (!vertices || !faces || !indices) + throw std::runtime_error("failed to allocate subdivision geometry buffers"); + + vertices[0] = 0.0f; + vertices[1] = 0.0f; + vertices[2] = 0.0f; + vertices[3] = 1.0f; + vertices[4] = 0.0f; + vertices[5] = 0.0f; + vertices[6] = 1.0f; + vertices[7] = 1.0f; + vertices[8] = 0.0f; + vertices[9] = 0.0f; + vertices[10] = 1.0f; + vertices[11] = 0.0f; + faces[0] = 4; + if (zeroValenceFace) faces[1] = 0; + indices[0] = 0; + indices[1] = 1; + indices[2] = 2; + indices[3] = 3; + AssertNoError(device); + return geometry; + }; + + RTCScene scene = rtcNewScene(device); + RTCGeometry geometry = createGeometry(false); + rtcCommitGeometry(geometry); + AssertNoError(device); + rtcAttachGeometry(scene,geometry); + AssertNoError(device); + rtcReleaseGeometry(geometry); + rtcCommitScene(scene); + AssertNoError(device); + rtcReleaseScene(scene); + + scene = rtcNewScene(device); + geometry = createGeometry(true); + rtcCommitGeometry(geometry); + AssertError(device,RTC_ERROR_INVALID_OPERATION); + rtcReleaseGeometry(geometry); + rtcReleaseScene(scene); + + return VerifyApplication::PASSED; + } + }; + ///////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////// @@ -6444,6 +6513,7 @@ namespace embree groups.top()->add(new MultipleDevicesTest("multiple_devices",isa)); groups.top()->add(new TypesTest("types_test",isa)); + groups.top()->add(new SubdivZeroValenceTest("subdiv_zero_valence",isa)); push(new TestGroup("get_bounds",true,true)); for (auto gtype : gtypes_all) From 01f607691894ed954b77c40dc0a206c8346e143b Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Wed, 14 Oct 2026 21:46:56 +0200 Subject: [PATCH 3/6] Removed overly complex fix --- README.md | 6 --- doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md | 6 --- kernels/common/scene_subdiv_mesh.cpp | 46 -------------------- 3 files changed, 58 deletions(-) diff --git a/README.md b/README.md index d9379b2e8b..a6f2c6667c 100644 --- a/README.md +++ b/README.md @@ -3627,12 +3627,6 @@ topologies, which means that the `n`-th primitive always has the same number of vertices (e.g. being a triangle or a quad) for each topology. However, the indices of the topologies themselves may be different. -The face buffer has to be consistent with the index buffers, thus each -face must have at least 3 vertices and the sum of all face vertex -counts must not exceed the size of any index buffer. Committing a -geometry that violates this raises an `RTC_ERROR_INVALID_OPERATION` -error. - #### EXIT STATUS {#exit-status} On failure `NULL` is returned and an error code is set that can be diff --git a/doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md b/doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md index c1bd7d7caa..ec9e2fc3a5 100644 --- a/doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md +++ b/doc/src/api/RTC_GEOMETRY_TYPE_SUBDIVISION.md @@ -163,12 +163,6 @@ topologies, which means that the `n`-th primitive always has the same number of vertices (e.g. being a triangle or a quad) for each topology. However, the indices of the topologies themselves may be different. -The face buffer has to be consistent with the index buffers, thus each -face must have at least 3 vertices and the sum of all face vertex -counts must not exceed the size of any index buffer. Committing a -geometry that violates this raises an `RTC_ERROR_INVALID_OPERATION` -error. - #### EXIT STATUS On failure `NULL` is returned and an error code is set that can be diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 2349409447..4dc2080d36 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -11,7 +11,6 @@ #include "../../common/algorithms/parallel_sort.h" #include "../../common/algorithms/parallel_prefix_sum.h" #include "../../common/algorithms/parallel_for.h" -#include "../../common/algorithms/parallel_reduce.h" /*! maximum number of user vertex buffers for subdivision surfaces */ #define RTC_MAX_USER_VERTEX_BUFFERS 65536 @@ -634,10 +633,6 @@ namespace embree /* allocate half edge array */ halfEdges.resize(mesh->numEdges()); - /* the index buffer of each topology has to be large enough for all half edges */ - if (vertexIndices.size() < mesh->numHalfEdges) - throw_RTCError(RTC_ERROR_INVALID_OPERATION,"index buffer of subdivision mesh too small for face buffer"); - /* check if we have to recalculate the half edges */ bool recalculate = false; recalculate |= vertexIndices.isLocalModified(); @@ -693,51 +688,10 @@ namespace embree << std::endl; } - /*! statistics gathered over the face buffer to validate it */ - struct FaceBufferStats - { - uint64_t numHalfEdges = 0; - unsigned int minValence = unsigned(-1); - }; - void SubdivMesh::initializeHalfEdgeStructures () { double t0 = getSeconds(); - /* The half edge of a face is looked up through the prefix sum of the face - valences. We thus have to validate the face buffer before anything else, - as otherwise a face with zero vertices or a face buffer that does not - match the index buffer would index the half edge array out of bounds. */ - if (faceVertices.isLocalModified()) - { - const FaceBufferStats stats = parallel_reduce - (size_t(0), numFaces(), size_t(1024), FaceBufferStats(), - [&](const range& r) -> FaceBufferStats - { - FaceBufferStats stats; - for (size_t f=r.begin(); f FaceBufferStats { - FaceBufferStats stats; - stats.numHalfEdges = a.numHalfEdges + b.numHalfEdges; - stats.minValence = min(a.minValence,b.minValence); - return stats; - }); - - if (numFaces() && stats.minValence == 0) - throw_RTCError(RTC_ERROR_INVALID_OPERATION,"subdivision face with zero vertices"); - - if (stats.numHalfEdges > (uint64_t)numEdges()) - throw_RTCError(RTC_ERROR_INVALID_OPERATION,"index buffer of subdivision mesh too small for face buffer"); - - if (stats.numHalfEdges > (uint64_t)0xFFFFFFFF) - throw_RTCError(RTC_ERROR_INVALID_OPERATION,"too many edges in subdivision mesh"); - } - invalid_face.resize(numFaces()*numTimeSteps); /* calculate start edge of each face */ From 244638500791472e04e4ad8c795f36451e3daf21 Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Wed, 14 Oct 2026 21:48:12 +0200 Subject: [PATCH 4/6] Simple fix for zero sized subd faces at the end of a mesh. Ensure start inidices never exceed half edge array size. --- kernels/common/scene_subdiv_mesh.cpp | 11 +++++++++++ tutorials/verify/verify.cpp | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 4dc2080d36..003b21a737 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -531,6 +531,11 @@ namespace embree { for (size_t f=r.begin(); ffaceVertices[f] == 0) { + continue; + } + HalfEdge* edge = &halfEdges[mesh->faceStartEdge[f]]; /* for vertex topology we also test if vertices are valid */ @@ -701,6 +706,12 @@ namespace embree { numHalfEdges = parallel_prefix_sum(faceVertices,faceStartEdge,numFaces(),0,std::plus()); + /* Corner case: If the last face has zero edges, start edge index will be invalid. + Set it to the last valid half edge index. */ + if (numFaces() && faceVertices[numFaces()-1] == 0) { + faceStartEdge[numFaces()-1] = numHalfEdges-1; + } + /* calculate face of each half edge */ halfEdgeFace.resize(numHalfEdges); for (size_t f=0, h=0; f Date: Wed, 14 Oct 2026 22:00:16 +0200 Subject: [PATCH 5/6] Harden against the case of multiple zero-sized faces at the end of a subd mesh --- kernels/common/scene_subdiv_mesh.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 003b21a737..50fc4c7a30 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -706,10 +706,13 @@ namespace embree { numHalfEdges = parallel_prefix_sum(faceVertices,faceStartEdge,numFaces(),0,std::plus()); - /* Corner case: If the last face has zero edges, start edge index will be invalid. + /* Corner case: If the last faces have zero edges, start edge index will be invalid. Set it to the last valid half edge index. */ if (numFaces() && faceVertices[numFaces()-1] == 0) { - faceStartEdge[numFaces()-1] = numHalfEdges-1; + /* Walk back through the faces until we find one with non-zero edges */ + for (size_t i = numFaces() - 1; i > 0 && faceVertices[i] == 0; i--) { + faceStartEdge[i] = numHalfEdges - 1; + } } /* calculate face of each half edge */ From bbad099b8ee26af2a551c8d4f178b01cfc9b788f Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Thu, 15 Oct 2026 13:31:34 +0200 Subject: [PATCH 6/6] Removed face start edge sanitation --- kernels/common/scene_subdiv_mesh.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 50fc4c7a30..ebbf980d9b 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -531,7 +531,7 @@ namespace embree { for (size_t f=r.begin(); ffaceVertices[f] == 0) { continue; } @@ -706,15 +706,6 @@ namespace embree { numHalfEdges = parallel_prefix_sum(faceVertices,faceStartEdge,numFaces(),0,std::plus()); - /* Corner case: If the last faces have zero edges, start edge index will be invalid. - Set it to the last valid half edge index. */ - if (numFaces() && faceVertices[numFaces()-1] == 0) { - /* Walk back through the faces until we find one with non-zero edges */ - for (size_t i = numFaces() - 1; i > 0 && faceVertices[i] == 0; i--) { - faceStartEdge[i] = numHalfEdges - 1; - } - } - /* calculate face of each half edge */ halfEdgeFace.resize(numHalfEdges); for (size_t f=0, h=0; f