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.

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

Betsy: Fix stack-use-after-scope when using BC3 and BC5 #100588

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

timothyqiu
Copy link
Member

@timothyqiu timothyqiu commented Dec 19, 2024

dst_texture_rid points to dst_texture_combined, which goes out of scope. So dereferencing dst_texture_rid is undefined behavior.

ASan Output
=================================================================
==29839==ERROR: AddressSanitizer: stack-use-after-scope on address 0x75884aca2140 at pc 0x61d11d139138 bp 0x75885c0f2ba0 sp 0x75885c0f2b90
READ of size 8 at 0x75884aca2140 thread T2
    #0 0x61d11d139137 in BetsyCompressor::_compress(BetsyFormat, Image*) modules/betsy/image_compress_betsy.cpp:648
    #1 0x61d11d156c77 in CommandQueueMT::CommandRet2<BetsyCompressor, Error (BetsyCompressor::*)(BetsyFormat, Image*), BetsyFormat, Image*, Error>::call() core/templates/command_queue_mt.h:320
    #2 0x61d11d13a9bc in CommandQueueMT::_flush() core/templates/command_queue_mt.h:374
    #3 0x61d11d13aea5 in CommandQueueMT::flush_all() core/templates/command_queue_mt.h:432
    #4 0x61d11d134b10 in BetsyCompressor::_thread_loop() modules/betsy/image_compress_betsy.cpp:242
    #5 0x61d11d15ecf2 in void call_with_variant_args_helper<BetsyCompressor>(BetsyCompressor*, void (BetsyCompressor::*)(), Variant const**, Callable::CallError&, IndexSequence<>) core/variant/binder_common.h:304
    #6 0x61d11d15a7e4 in void call_with_variant_args<BetsyCompressor>(BetsyCompressor*, void (BetsyCompressor::*)(), Variant const**, int, Callable::CallError&) core/variant/binder_common.h:418
    #7 0x61d11d15698e in CallableCustomMethodPointer<BetsyCompressor, void>::call(Variant const**, int, Variant&, Callable::CallError&) const core/object/callable_method_pointer.h:107
    #8 0x61d128c01c86 in Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const core/variant/callable.cpp:57
    #9 0x61d120df06b1 in Variant Callable::call<>() const core/variant/variant.h:920
    #10 0x61d12951f69e in WorkerThreadPool::_process_task(WorkerThreadPool::Task*) core/object/worker_thread_pool.cpp:142
    #11 0x61d12951fe15 in WorkerThreadPool::_thread_function(void*) core/object/worker_thread_pool.cpp:205
    #12 0x61d1285b426b in Thread::callback(unsigned long, Thread::Settings const&, void (*)(void*), void*) core/os/thread.cpp:64
    #13 0x61d1285b57a8 in void std::__invoke_impl<void, void (*)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long, Thread::Settings, void (*)(void*), void*>(std::__invoke_other, void (*&&)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long&&, Thread::Settings&&, void (*&&)(void*), void*&&) /usr/include/c++/14.2.1/bits/invoke.h:61
    #14 0x61d1285b55ff in std::__invoke_result<void (*)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long, Thread::Settings, void (*)(void*), void*>::type std::__invoke<void (*)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long, Thread::Settings, void (*)(void*), void*>(void (*&&)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long&&, Thread::Settings&&, void (*&&)(void*), void*&&) /usr/include/c++/14.2.1/bits/invoke.h:96
    #15 0x61d1285b54ba in void std::thread::_Invoker<std::tuple<void (*)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long, Thread::Settings, void (*)(void*), void*> >::_M_invoke<0ul, 1ul, 2ul, 3ul, 4ul>(std::_Index_tuple<0ul, 1ul, 2ul, 3ul, 4ul>) /usr/include/c++/14.2.1/bits/std_thread.h:301
    #16 0x61d1285b541f in std::thread::_Invoker<std::tuple<void (*)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long, Thread::Settings, void (*)(void*), void*> >::operator()() /usr/include/c++/14.2.1/bits/std_thread.h:308
    #17 0x61d1285b5403 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long, Thread::Settings, void (*)(void*), void*> > >::_M_run() /usr/include/c++/14.2.1/bits/std_thread.h:253
    #18 0x61d12a0c5223 in execute_native_thread_routine (/home/timothy/repos/godot-master/bin/godot.linuxbsd.editor.dev.x86_64.san+0x16ca3223) (BuildId: 57ac3e361495ba55ee35a72b80777054f9509aa2)
    #19 0x75886145d109 in asan_thread_start /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:234
    #20 0x7588612a339c  (/usr/lib/libc.so.6+0x9439c) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)
    #21 0x75886132849b  (/usr/lib/libc.so.6+0x11949b) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)

Address 0x75884aca2140 is located in stack of thread T2 at offset 320 in frame
    #0 0x61d11d1354af in BetsyCompressor::_compress(BetsyFormat, Image*) modules/betsy/image_compress_betsy.cpp:364

  This frame has 58 object(s):
    [32, 36) 'width' (line 456)
    [48, 52) 'height' (line 456)
    [64, 72) '<unknown>'
    [96, 104) 'ofs' (line 455)
    [128, 136) 'size' (line 455)
    [160, 168) 'src_texture' (line 472)
    [192, 200) 'dst_texture_primary' (line 473)
    [224, 232) 'uniform_set' (line 503)
    [256, 264) 'dst_texture_alpha' (line 555)
    [288, 296) 'uniform_set' (line 577)
    [320, 328) 'dst_texture_combined' (line 598) <== Memory access at offset 320 is inside this variable
    [352, 360) 'uniform_set' (line 628)
    [384, 392) '<unknown>'
    [416, 424) '<unknown>'
    [448, 456) '<unknown>'
    [480, 496) 'shader' (line 383)
    [512, 528) 'secondary_shader' (line 384)
    [544, 560) 'stitch_shader' (line 385)
    [576, 592) 'data' (line 435)
    [608, 624) 'dst_data' (line 445)
    [640, 656) 'src_images' (line 449)
    [672, 688) '<unknown>'
    [704, 720) '<unknown>'
    [736, 752) 'uniforms' (line 476)
    [768, 784) '<unknown>'
    [800, 816) 'uniforms' (line 558)
    [832, 848) '<unknown>'
    [864, 880) 'uniforms' (line 601)
    [896, 912) 'texture_data' (line 648)
    [928, 948) '<unknown>'
    [992, 1012) '<unknown>'
    [1056, 1076) '<unknown>'
    [1120, 1140) '<unknown>'
    [1184, 1224) 'u' (line 479)
    [1264, 1304) '<unknown>'
    [1344, 1384) 'u' (line 487)
    [1424, 1464) '<unknown>'
    [1504, 1544) 'u' (line 495)
    [1584, 1624) '<unknown>'
    [1664, 1704) 'u' (line 561)
    [1744, 1784) '<unknown>'
    [1824, 1864) 'u' (line 569)
    [1904, 1944) '<unknown>'
    [1984, 2024) 'u' (line 604)
    [2064, 2104) '<unknown>'
    [2144, 2184) 'u' (line 612)
    [2224, 2264) '<unknown>'
    [2304, 2344) 'u' (line 620)
    [2384, 2424) '<unknown>'
    [2464, 2528) 'src_texture_format' (line 400)
    [2560, 2624) 'dst_texture_format' (line 416)
    [2656, 2720) 'dst_texture_format_alpha' (line 420)
    [2752, 2816) 'dst_texture_format_combined' (line 421)
    [2848, 2864) 'push_constant' (line 512)
    [2880, 2896) 'push_constant' (line 521)
    [2912, 2928) 'push_constant' (line 529)
    [2944, 2960) 'push_constant' (line 583)
    [2976, 3000) '<unknown>'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
Thread T2 created by T0 here:
    #0 0x7588614f468b in pthread_create /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:245
    #1 0x61d12a0c5341 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/home/timothy/repos/godot-master/bin/godot.linuxbsd.editor.dev.x86_64.san+0x16ca3341) (BuildId: 57ac3e361495ba55ee35a72b80777054f9509aa2)
    #2 0x61d1285b4c56 in std::thread::thread<void (*)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long&, Thread::Settings const&, void (*&)(void*), void*&, void>(void (*&&)(unsigned long, Thread::Settings const&, void (*)(void*), void*), unsigned long&, Thread::Settings const&, void (*&)(void*), void*&) /usr/include/c++/14.2.1/bits/std_thread.h:173
    #3 0x61d1285b44d9 in Thread::start(void (*)(void*), void*, Thread::Settings const&) core/os/thread.cpp:75
    #4 0x61d129524f32 in WorkerThreadPool::init(int, float) core/object/worker_thread_pool.cpp:769
    #5 0x61d11cbc17c4 in Main::setup(char const*, int, char**, bool) main/main.cpp:1870
    #6 0x61d11c9eab1c in main platform/linuxbsd/godot_linuxbsd.cpp:74
    #7 0x758861234e07  (/usr/lib/libc.so.6+0x25e07) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)
    #8 0x758861234ecb in __libc_start_main (/usr/lib/libc.so.6+0x25ecb) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)
    #9 0x61d11c9ea764 in _start (/home/timothy/repos/godot-master/bin/godot.linuxbsd.editor.dev.x86_64.san+0x95c8764) (BuildId: 57ac3e361495ba55ee35a72b80777054f9509aa2)

SUMMARY: AddressSanitizer: stack-use-after-scope modules/betsy/image_compress_betsy.cpp:648 in BetsyCompressor::_compress(BetsyFormat, Image*)
Shadow bytes around the buggy address:
  0x75884aca1e80: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x75884aca1f00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x75884aca1f80: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x75884aca2000: f1 f1 f1 f1 04 f2 04 f2 f8 f2 f2 f2 00 f2 f2 f2
  0x75884aca2080: 00 f2 f2 f2 00 f2 f2 f2 00 f2 f2 f2 f8 f2 f2 f2
=>0x75884aca2100: f8 f2 f2 f2 f8 f2 f2 f2[f8]f2 f2 f2 f8 f2 f2 f2
  0x75884aca2180: 00 f2 f2 f2 00 f2 f2 f2 00 f2 f2 f2 00 00 f2 f2
  0x75884aca2200: 00 00 f2 f2 00 00 f2 f2 f8 f8 f2 f2 00 00 f2 f2
  0x75884aca2280: 00 00 f2 f2 f8 f8 f2 f2 f8 f8 f2 f2 f8 f8 f2 f2
  0x75884aca2300: f8 f8 f2 f2 f8 f8 f2 f2 f8 f8 f2 f2 f8 f8 f2 f2
  0x75884aca2380: 00 00 f2 f2 f8 f8 f8 f2 f2 f2 f2 f2 f8 f8 f8 f2
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
==29839==ABORTING

RIDs can be assigned directly, so dst_texture_rid does not have to be a pointer.

@timothyqiu timothyqiu added this to the 4.4 milestone Dec 19, 2024
@timothyqiu timothyqiu requested a review from a team as a code owner December 19, 2024 00:51
@Repiteo Repiteo merged commit 8b0b38f into godotengine:master Dec 20, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 20, 2024

Thanks!

@timothyqiu timothyqiu deleted the wild-rid branch December 20, 2024 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants