Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AddressSanitizer: heap-buffer-overflow caused by embree buffer padding added using resize() instead of reserve() #94548

Open
jamie-pate opened this issue Jul 19, 2024 · 1 comment
Open

AddressSanitizer: heap-buffer-overflow caused by embree buffer padding added using resize() instead of reserve() #94548

jamie-pate opened this issue Jul 19, 2024 · 1 comment
Labels
bug topic:rendering topic:3d

Comments

@jamie-pate
Copy link
Contributor

Tested versions

Reproducible in current HEAD: ff8a278

System information

Godot Engine v4.3.beta.custom_build.eb5f1299b (2024-07-19 02:22:03 UTC) - https://godotengine.org Vulkan 1.3.242 - Forward+ - Using Device #2: NVIDIA - NVIDIA GeForce RTX 3050 Laptop GPU

Issue description

==592286==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61b00011a990 at pc 0x55555994a0b5 bp 0x7fffffff8d50 sp 0x7fffffff8d40
READ of size 4 at 0x61b00011a990 thread T0
    #0 0x55555994a0b4 in embree::RawBufferView::checkPadding16() const thirdparty/embree/kernels/common/buffer.h:215
    #1 0x55555994a0b4 in embree::TriangleMesh::setBuffer(RTCBufferType, unsigned int, RTCFormat, embree::Ref<embree::Buffer> const&, unsigned long, unsigned long, unsigned int) thirdparty/embree/kernels/common/scene_triangle_mesh.cpp:54
    #2 0x55555991d0d9 in rtcSetSharedGeometryBuffer thirdparty/embree/kernels/common/rtcore.cpp:1670
    #3 0x555559fd6575 in RaycastOcclusionCull::Scenario::update() modules/raycast/raycast_occlusion_cull.cpp:483
    #4 0x555559fd97d5 in RaycastOcclusionCull::buffer_update(RID, Transform3D const&, Projection const&, bool) modules/raycast/raycast_occlusion_cull.cpp:616
    #5 0x555566c6dc4b in RendererSceneCull::render_camera(Ref<RenderSceneBuffers> const&, RID, RID, RID, Vector2, unsigned int, float, RID, Ref<XRInterface>&, RenderingMethod::RenderInfo*) servers/rendering/renderer_scene_cull.cpp:2644
    #6 0x555566d29b7e in RendererViewport::_draw_3d(RendererViewport::Viewport*) servers/rendering/renderer_viewport.cpp:248
    #7 0x555566d2b690 in RendererViewport::_draw_viewport(RendererViewport::Viewport*) servers/rendering/renderer_viewport.cpp:314
    #8 0x555566d3e26d in RendererViewport::draw_viewports(bool) servers/rendering/renderer_viewport.cpp:808
    #9 0x5555656015a4 in RenderingServerDefault::_draw(bool, double) servers/rendering/rendering_server_default.cpp:87
    #10 0x555565608d47 in RenderingServerDefault::draw(bool, double) servers/rendering/rendering_server_default.cpp:410
    #11 0x55555707db84 in Main::iteration() main/main.cpp:4118
    #12 0x555556ea86a9 in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:962
    #13 0x555556e8e776 in main platform/linuxbsd/godot_linuxbsd.cpp:85
    #14 0x7ffff7029d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #15 0x7ffff7029e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #16 0x555556e8e364 in _start (/home/jpate/build/src/godot/bin/godot.linuxbsd.editor.x86_64.san+0x193a364)

0x61b00011a990 is located 0 bytes to the right of 1552-byte region [0x61b00011a380,0x61b00011a990)
allocated by thread T0 here:
    #0 0x7ffff74b4887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x555568a46fe6 in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:75
    #2 0x555568a4744b in Memory::realloc_static(void*, unsigned long, bool) core/os/memory.cpp:99
    #3 0x555558b4f757 in LocalVector<Vector3, unsigned int, false, false>::resize(unsigned int) core/templates/local_vector.h:161
    #4 0x555559fd33e3 in RaycastOcclusionCull::Scenario::_update_dirty_instance(int, RID*) modules/raycast/raycast_occlusion_cull.cpp:359
    #5 0x555559fd57f7 in RaycastOcclusionCull::Scenario::update() modules/raycast/raycast_occlusion_cull.cpp:452
    #6 0x555559fd97d5 in RaycastOcclusionCull::buffer_update(RID, Transform3D const&, Projection const&, bool) modules/raycast/raycast_occlusion_cull.cpp:616
    #7 0x555566c6dc4b in RendererSceneCull::render_camera(Ref<RenderSceneBuffers> const&, RID, RID, RID, Vector2, unsigned int, float, RID, Ref<XRInterface>&, RenderingMethod::RenderInfo*) servers/rendering/renderer_scene_cull.cpp:2644
    #8 0x555566d29b7e in RendererViewport::_draw_3d(RendererViewport::Viewport*) servers/rendering/renderer_viewport.cpp:248
    #9 0x555566d2b690 in RendererViewport::_draw_viewport(RendererViewport::Viewport*) servers/rendering/renderer_viewport.cpp:314
    #10 0x555566d3e26d in RendererViewport::draw_viewports(bool) servers/rendering/renderer_viewport.cpp:808
    #11 0x5555656015a4 in RenderingServerDefault::_draw(bool, double) servers/rendering/rendering_server_default.cpp:87
    #12 0x555565608d47 in RenderingServerDefault::draw(bool, double) servers/rendering/rendering_server_default.cpp:410
    #13 0x55555707db84 in Main::iteration() main/main.cpp:4118
    #14 0x555556ea86a9 in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:962
    #15 0x555556e8e776 in main platform/linuxbsd/godot_linuxbsd.cpp:85
    #16 0x7ffff7029d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: heap-buffer-overflow thirdparty/embree/kernels/common/buffer.h:215 in embree::RawBufferView::checkPadding16() const
Shadow bytes around the buggy address:
  0x0c368001b4e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c368001b4f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c368001b500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c368001b510: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c368001b520: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c368001b530: 00 00[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c368001b540: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c368001b550: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c368001b560: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c368001b570: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c368001b580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==592286==ABORTING

Steps to reproduce

Enable asan and run a scene that uses occlusion culling? It only happens under certain conditions, even with asan enabled.

Minimal reproduction project (MRP)

asan-embree-output.txt
@jamie-pate jamie-pate mentioned this issue Jul 19, 2024
Closed
@jamie-pate
Copy link
Contributor Author

jamie-pate commented Jul 19, 2024 •
edited
Loading

An alternate solution would involve upstreaming this fix to embree I suppose, since their buffer check calculation dereferencing at 24 bytes instead of 16 is the actual cause.. (due to the size of int* changing on x86_64 vs x86_32)

(upstream issue reported RenderKit/embree#495)

@jamie-pate jamie-pate mentioned this issue Jul 22, 2024
Closed
@jamie-pate jamie-pate changed the title AddressSanitizer: heap-buffer-overflow caused by embree buffer check miscalculation AddressSanitizer: heap-buffer-overflow caused by embree buffer padding added using resize() instead of reserve() Jul 22, 2024
jamie-pate added a commit to jamie-pate/godot that referenced this issue Jul 24, 2024
@jamie-pate
Fix AddressSanitizer: heap-buffer-overflow caused by incorrect padding
Loading
Loading status checks…
3f29ea4 
Fixes godotengine#94548

Use reserve() instead of resize() when padding the buffer passed to
embree, otherwise it will look past the end of the buffer since we pass
the size() to rtcSetSharedGeometryBuffer and it dereferences
`(int*)getPtr(size()-1) + 3`
could cause a segfault if `capacity < size + 12`(bytes)

When the address sanitizer is enabled it triggers an assertion

    /*! checks padding to 16 byte check, fails hard */
    __forceinline void checkPadding16() const
    {
      if (ptr_ofs && num)
volatile int MAYBE_UNUSED w = *((int*)getPtr(size()-1)+3); //
FIXME: is failing hard avoidable?
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug topic:rendering topic:3d
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix AddressSanitizer: heap-buffer-overflow from embree buffer check jamie-pate/godot
2 participants
@Calinou @jamie-pate