search

mitsuba-renderer / mitsuba3

Mitsuba 3: A Retargetable Forward and Inverse Renderer
https://www.mitsuba-renderer.org/
Other
2.1k stars 246 forks source link

Embree: AddressSanitizer: heap-buffer-overflow #1283

Open dvicini opened 3 months ago

dvicini commented 3 months ago

I was running the Mitsuba tests using the AddressSanitizer and ran into an issue in test_mesh.py:

==2801657==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5040001c51c0 at pc 0x7f96e617b420 bp 0x7fff118cc410 sp 0x7fff118cc408
READ of size 4 at 0x5040001c51c0 thread T0
    #0 0x7f96e617b41f in checkPadding16 .../embree/embree_3_13_5/kernels/common/buffer.h:217:39
    #1 0x7f96e617b41f in embree::TriangleMesh::setBuffer(RTCBufferType, unsigned int, RTCFormat, embree::Ref<embree::Buffer> const&, unsigned long, unsigned long, unsigned int) .../embree/embree_3_13_5/kernels/common/scene_triangle_mesh.cpp:54:22
    #2 0x7f96e60817ac in rtcSetSharedGeometryBuffer .../embree/embree_3_13_5/kernels/common/rtcore.cpp:1434:15
    #3 0x7f96edc950c6 in mitsuba::Mesh<float, mitsuba::Color<float, 3ul>>::embree_geometry(RTCDeviceTy*) .../mitsuba/src/render/mesh.cpp:1827:5
    #4 0x7f96eddcd986 in mitsuba::Scene<float, mitsuba::Color<float, 3ul>>::accel_parameters_changed_cpu() .../mitsuba/src/render/scene_embree.inl:148:35

Embree checks that all buffers have at least a 16 byte padding: https://github.com/mitsuba-renderer/embree/blob/598978bb21d098b6a7833fa889a2b5b38be6f026/kernels/common/scene_triangle_mesh.cpp#L54

It performs this check by explicitly trying to read beyond the end of the specified array size. My understanding is that this is done in order to detect if SIMD instructions will potentially read outside the provided array.

As far as I can tell, we don't ensure that the vertex buffers we have are padded. These buffers might even have been created outside of Mitsuba (e.g., when creating vertices with NumPy).