Fix heap buffer overflow for high valence subdivision vertices

kernels/common/scene_subdiv_mesh.cpp

@@ -543,7 +543,7 @@ namespace embree
         {
           /* calculate if face is valid */
           for (size_t t=0; t<mesh->numTimeSteps; t++)
-            mesh->invalidFace(f,t) = !edge->valid(mesh->vertices[t]) || mesh->holeSet->holeSet.lookup(unsigned(f));
+            mesh->invalidFace(f,t) = !edge->valid(&mesh->vertices[t]) || mesh->holeSet->holeSet.lookup(unsigned(f));
         }
 
         /* pin some edges and vertices */
@@ -566,8 +566,11 @@ namespace embree
 
         /* we have to calculate patch_type last! */
         HalfEdge::PatchType patch_type = edge->patchType();
-        for (size_t i=0; i<mesh->faceVertices[f]; i++) 
+        bool valid = edge->valid(nullptr);
+        for (size_t i=0; i<mesh->faceVertices[f]; i++) {
           edge[i].patch_type = patch_type;
+          edge[i].valid_patch = valid;
+        }
       }
     });
   }
@@ -885,12 +888,25 @@ namespace embree
       bool has_P = P;
       bool has_dP = dPdu;     assert(!has_dP  || dPdv);
       bool has_ddP = ddPdudu; assert(!has_ddP || (ddPdvdv && ddPdudu));
+
+      const HalfEdge* halfEdge = topo->getHalfEdge(primID);
+
+      /* invalid patches cannot get evaluated */
+      if (unlikely(!halfEdge->valid_patch))
+      {
+        for (unsigned int j=0; j<valueCount; j++) {
+          if (has_P) P[j] = 0.0f;
+          if (has_dP) { dPdu[j] = 0.0f; dPdv[j] = 0.0f; }
+          if (has_ddP) { ddPdudu[j] = 0.0f; ddPdvdv[j] = 0.0f; ddPdudv[j] = 0.0f; }
+        }
+        return;
+      }
 
       for (unsigned int i=0; i<valueCount; i+=4)
       {
         vfloat4 Pt, dPdut, dPdvt, ddPdudut, ddPdvdvt, ddPdudvt;
         isa::PatchEval<vfloat4,vfloat4>(baseEntry->at(interpolationSlot(primID,i/4,stride)),commitCounter,
-                                        topo->getHalfEdge(primID),src+i*sizeof(float),stride,u,v,
+                                        halfEdge,src+i*sizeof(float),stride,u,v,
                                         has_P ? &Pt : nullptr, 
                                         has_dP ? &dPdut : nullptr, 
                                         has_dP ? &dPdvt : nullptr,
@@ -975,11 +991,27 @@ namespace embree
 
         foreach_unique(valid1,primID,[&](const vbool4& valid1, const unsigned int primID)
                        {
+                         const HalfEdge* halfEdge = topo->getHalfEdge(primID);
+
+                         /* invalid patches cannot get evaluated */
+                         if (unlikely(!halfEdge->valid_patch))
+                         {
+                           for (unsigned int j=0; j<valueCount; j++) {
+                             if (P) vfloat4::storeu(valid1,P+j*N+i,vfloat4(0.0f));
+                             if (dPdu) vfloat4::storeu(valid1,dPdu+j*N+i,vfloat4(0.0f));
+                             if (dPdv) vfloat4::storeu(valid1,dPdv+j*N+i,vfloat4(0.0f));
+                             if (ddPdudu) vfloat4::storeu(valid1,ddPdudu+j*N+i,vfloat4(0.0f));
+                             if (ddPdvdv) vfloat4::storeu(valid1,ddPdvdv+j*N+i,vfloat4(0.0f));
+                             if (ddPdudv) vfloat4::storeu(valid1,ddPdudv+j*N+i,vfloat4(0.0f));
+                           }
+                           return;
+                         }
+
                          for (unsigned int j=0; j<valueCount; j+=4) 
                          {
                            const size_t M = min(4u,valueCount-j);
                            isa::PatchEvalSimd<vbool4,vint4,vfloat4,vfloat4>(baseEntry->at(interpolationSlot(primID,j/4,stride)),commitCounter,
-                                                                            topo->getHalfEdge(primID),src+j*sizeof(float),stride,valid1,uu,vv,
+                                                                            halfEdge,src+j*sizeof(float),stride,valid1,uu,vv,
                                                                             P ? P+j*N+i : nullptr,
                                                                             dPdu ? dPdu+j*N+i : nullptr,
                                                                             dPdv ? dPdv+j*N+i : nullptr,

kernels/common/scene_subdiv_mesh.h

@@ -169,6 +169,8 @@ namespace embree
       /*! check if the i'th primitive is valid in this topology */
       __forceinline bool valid(size_t i) const 
       {
+        if (unlikely(!getHalfEdge(i)->valid_patch))
+          return false;
         if (unlikely(subdiv_mode == RTC_SUBDIVISION_MODE_NO_BOUNDARY)) {
           if (getHalfEdge(i)->faceHasBorder()) return false;
         }

kernels/subdiv/half_edge.h

@@ -48,7 +48,7 @@ namespace embree
 
     HalfEdge () 
       : vtx_index(-1), next_half_edge_ofs(0), prev_half_edge_ofs(0), opposite_half_edge_ofs(0), edge_crease_weight(0), 
-      vertex_crease_weight(0), edge_level(0), patch_type(COMPLEX_PATCH), vertex_type(REGULAR_VERTEX)
+      vertex_crease_weight(0), edge_level(0), patch_type(COMPLEX_PATCH), vertex_type(REGULAR_VERTEX), valid_patch(false)
     {
       static_assert(sizeof(HalfEdge) == 32, "invalid half edge size");
     }
@@ -201,8 +201,8 @@ namespace embree
       return bounds;
     }
 
-    /*! tests if this is a valid patch */
-    __forceinline bool valid(const BufferView<Vec3fa>& vertices) const
+    /*! tests if this is a valid patch, the vertex check is skipped when vertices is NULL */
+    __forceinline bool valid(const BufferView<Vec3fa>* vertices) const
     {
       size_t N = 1;
       if (!this->validRing(vertices)) return false;
@@ -306,33 +306,28 @@ namespace embree
       return bounds;
     }
 
-    /*! tests if this is a valid face */
-    __forceinline bool validFace(const BufferView<Vec3fa>& vertices, size_t& N) const 
-    {
-      const Vec3fa v = vertices[getStartVertexIndex()];
-      if (!isvalid(v)) return false;
-      size_t n = 1;
-      for (const HalfEdge* p = next(); p!=this; p=p->next(), n++) {
-        const Vec3fa v = vertices[p->getStartVertexIndex()];
-        if (!isvalid(v)) return false;
-      }
-      if (n < 2) return false;
-      N += n-2;
-      return n >= 3 && n <= MAX_PATCH_VALENCE;
-    }
-
-    /*! tests if this is a valid ring */
-    __forceinline bool validRing(const BufferView<Vec3fa>& vertices) const 
+  private:
+    __forceinline bool validRing(const BufferView<Vec3fa>* vertices) const
     {
       size_t faceValence = 0;
       size_t edgeValence = 0;
 
       const HalfEdge* p = this;
       do 
       {
-        /* calculate bounds of current face */
-        if (!p->validFace(vertices,edgeValence)) 
+        size_t n = 0;
+        const HalfEdge* q = p;
+        do
+        {
+          if (vertices && !isvalid((*vertices)[q->getStartVertexIndex()]))
+            return false;
+          n++;
+          q = q->next();
+        } while (q != p);
+
+        if (n < 3 || n > MAX_PATCH_VALENCE)
           return false;
+        edgeValence += n-2;
 
         faceValence++;
         p = p->prev();
@@ -349,12 +344,16 @@ namespace embree
           while (p->hasOpposite()) 
             p = p->opposite()->next();
         }
-
-      } while (p != this); 
-
-      return faceValence <= MAX_RING_FACE_VALENCE && edgeValence <= MAX_RING_EDGE_VALENCE;
+
+        /* stop early for degenerated topology */
+        if (faceValence > MAX_RING_FACE_VALENCE || edgeValence > MAX_RING_EDGE_VALENCE)
+          return false;
+
+      } while (p != this);
+
+      return true;
     }
-    
+
   private:
     unsigned int vtx_index;         //!< index of edge start vertex
     int next_half_edge_ofs;         //!< relative offset to next half edge of face
@@ -367,6 +366,7 @@ namespace embree
     float edge_level;               //!< subdivision factor for edge
     PatchType patch_type;           //!< stores type of subdiv patch
     VertexType vertex_type;         //!< stores type of the start vertex
-    char align[2];
+    bool valid_patch;               //!< stores if the patch can be evaluated
+    char align[1];
   };
 }