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

Fix use-after-free in WorkerThreadPool #94832

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

alvinhochun
Copy link
Contributor

p_task can be freed in one of the branches above (line 133/135), so it should store the value earlier to not dereference a freed object.

Brought to you by Address Sanitizer (with PagedAllocator dummied out).

=================================================================
==14784==ERROR: AddressSanitizer: heap-use-after-free on address 0x124a8b720ae0 at pc 0x7ff6698f1d02 bp 0x00b1be3fea00 sp 0x00b1be3fea48
READ of size 1 at 0x124a8b720ae0 thread T3
    #0 0x7ff6698f1d01 in WorkerThreadPool::_process_task(WorkerThreadPool::Task*) /home/alvin/godot-dev/src-mingw/core/object/worker_thread_pool.cpp:162:15
    #1 0x7ff6698f3c4c in WorkerThreadPool::_thread_function(void*) /home/alvin/godot-dev/src-mingw/core/object/worker_thread_pool.cpp:201:15
    #2 0x7ff6684b5ed9 in Thread::callback(unsigned long long, Thread::Settings const&, void (*)(void*), void*) /home/alvin/godot-dev/src-mingw/core/os/thread.cpp:64:3
    #3 0x7ff6684b9db5 in decltype(std::declval<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*)>()(std::declval<unsigned long long>(), std::declval<Thread::Settings>(), std5
    #4 0x7ff6684b9910 in std::__1::invoke_result<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*), unsigned long long, Thread::Settings, void (*)(void*), void*>::type std::0
    #5 0x7ff6684b939c in mingw_stdthread::detail::ThreadFuncCall<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*), mingw_stdthread::detail::IntSeq<0ull, 1ull, 2ull, 3ull>, 3
    #6 0x7ff6684b864d in unsigned int mingw_stdthread::thread::threadfunc<mingw_stdthread::detail::ThreadFuncCall<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*), mingw_st5
    #7 0x7ffe310c1bb1  (C:\WINDOWS\System32\ucrtbase.dll+0x180021bb1)
    #8 0x7ffdbde745c9 in asan_thread_start(void*) /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_win.cpp:147:14
    #9 0x7ffe32947373  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017373)
    #10 0x7ffe33a3cc90  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18004cc90)

0x124a8b720ae0 is located 160 bytes inside of 184-byte region [0x124a8b720a40,0x124a8b720af8)
freed by thread T3 here:
    #0 0x7ffdbde64e51 in free /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_malloc_win.cpp:82:3
    #1 0x7ff668494b3b in Memory::free_static(void*, bool) /home/alvin/godot-dev/src-mingw/core/os/memory.cpp:168:3
    #2 0x7ff669917056 in void memdelete<WorkerThreadPool::Task>(WorkerThreadPool::Task*) /home/alvin/godot-dev/src-mingw/./core/os/memory.h:119:2
    #3 0x7ff669902b3c in PagedAllocator<WorkerThreadPool::Task, false, 1024u>::free(WorkerThreadPool::Task*) /home/alvin/godot-dev/src-mingw/./core/templates/paged_allocator.h:94:3
    #4 0x7ff6698f09d5 in WorkerThreadPool::_process_task(WorkerThreadPool::Task*) /home/alvin/godot-dev/src-mingw/core/object/worker_thread_pool.cpp:133:18
    #5 0x7ff6698f3c4c in WorkerThreadPool::_thread_function(void*) /home/alvin/godot-dev/src-mingw/core/object/worker_thread_pool.cpp:201:15
    #6 0x7ff6684b5ed9 in Thread::callback(unsigned long long, Thread::Settings const&, void (*)(void*), void*) /home/alvin/godot-dev/src-mingw/core/os/thread.cpp:64:3
    #7 0x7ff6684b9db5 in decltype(std::declval<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*)>()(std::declval<unsigned long long>(), std::declval<Thread::Settings>(), std5
    #8 0x7ff6684b9910 in std::__1::invoke_result<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*), unsigned long long, Thread::Settings, void (*)(void*), void*>::type std::0
    #9 0x7ff6684b939c in mingw_stdthread::detail::ThreadFuncCall<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*), mingw_stdthread::detail::IntSeq<0ull, 1ull, 2ull, 3ull>, 3
    #10 0x7ff6684b864d in unsigned int mingw_stdthread::thread::threadfunc<mingw_stdthread::detail::ThreadFuncCall<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*), mingw_s5
    #11 0x7ffe310c1bb1  (C:\WINDOWS\System32\ucrtbase.dll+0x180021bb1)
    #12 0x7ffdbde745c9 in asan_thread_start(void*) /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_win.cpp:147:14
    #13 0x7ffe32947373  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017373)
    #14 0x7ffe33a3cc90  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18004cc90)

previously allocated by thread T0 here:
    #0 0x7ffdbde64f71 in malloc /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_malloc_win.cpp:98:3
    #1 0x7ff668493332 in Memory::alloc_static(unsigned long long, bool) /home/alvin/godot-dev/src-mingw/core/os/memory.cpp:75:14
    #2 0x7ff6684931a1 in operator new(unsigned long long, char const*) /home/alvin/godot-dev/src-mingw/core/os/memory.cpp:40:9
    #3 0x7ff6699046ae in WorkerThreadPool::Task* PagedAllocator<WorkerThreadPool::Task, false, 1024u>::alloc<>() /home/alvin/godot-dev/src-mingw/./core/templates/paged_allocator.h:61:10
    #4 0x7ff6698fc1fc in WorkerThreadPool::_add_group_task(Callable const&, void (*)(void*, unsigned int), void*, WorkerThreadPool::BaseTemplateUserdata*, int, int, bool, String const&) /home/alvin/god2
    #5 0x7ff66bcce9dc in long long WorkerThreadPool::add_template_group_task<ShaderRD, void (ShaderRD::*)(unsigned int, ShaderRD::CompileData const*), ShaderRD::CompileData*>(ShaderRD*, void (ShaderRD:0
    #6 0x7ff66bcc1a25 in ShaderRD::_compile_version(ShaderRD::Version*, int) /home/alvin/godot-dev/src-mingw/servers/rendering/renderer_rd/shader_rd.cpp:521:76
    #7 0x7ff668486cb2 in ShaderRD::version_get_shader(RID, int) /home/alvin/godot-dev/src-mingw/./servers/rendering/renderer_rd/shader_rd.h:175:5
    #8 0x7ff66bbf2de0 in RendererRD::TextureStorage::TextureStorage() /home/alvin/godot-dev/src-mingw/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp:533:85
    #9 0x7ff668483e9e in RendererCompositorRD::RendererCompositorRD() /home/alvin/godot-dev/src-mingw/servers/rendering/renderer_rd/renderer_compositor_rd.cpp:306:20
    #10 0x7ff66721a53b in RendererCompositorRD::_create_current() /home/alvin/godot-dev/src-mingw/./servers/rendering/renderer_rd/renderer_compositor_rd.h:138:10
    #11 0x7ff66800b893 in RendererCompositor::create() /home/alvin/godot-dev/src-mingw/servers/rendering/renderer_compositor.cpp:42:9
    #12 0x7ff669c5a019 in RenderingServerDefault::_init() /home/alvin/godot-dev/src-mingw/servers/rendering/rendering_server_default.cpp:220:20
    #13 0x7ff669c5b6fc in RenderingServerDefault::init() /home/alvin/godot-dev/src-mingw/servers/rendering/rendering_server_default.cpp:259:3
    #14 0x7ff66738fd1f in Main::setup2(bool) /home/alvin/godot-dev/src-mingw/main/main.cpp:2833:21
    #15 0x7ff66738812d in Main::setup(char const*, int, char**, bool) /home/alvin/godot-dev/src-mingw/main/main.cpp:2465:10
    #16 0x7ff66710204e in widechar_main(int, wchar_t**) /home/alvin/godot-dev/src-mingw/platform/windows/godot_windows.cpp:90:14
    #17 0x7ff6671026c5 in _main() /home/alvin/godot-dev/src-mingw/platform/windows/godot_windows.cpp:131:11
    #18 0x7ff667102787 in main /home/alvin/godot-dev/src-mingw/platform/windows/godot_windows.cpp:150:9
    #19 0x7ff667101310 in __tmainCRTStartup /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:267:15
    #20 0x7ff667101155 in .l_startw /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:157:9
    #21 0x7ffe32947373  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017373)
    #22 0x7ffe33a3cc90  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18004cc90)

Thread T3 created by T0 here:
    #0 0x7ffdbde744f6 in CreateThread /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_win.cpp:158:3
    #1 0x7ffe310c1896  (C:\WINDOWS\System32\ucrtbase.dll+0x180021896)
    #2 0x7ff6684b6f69 in mingw_stdthread::thread::thread<void (*)(unsigned long long, Thread::Settings const&, void (*)(void*), void*), unsigned long long&, Thread::Settings const&, void (*&)(void*), v7
    #3 0x7ff6684b6399 in Thread::start(void (*)(void*), void*, Thread::Settings const&) /home/alvin/godot-dev/src-mingw/core/os/thread.cpp:75:11
    #4 0x7ff6698ffce0 in WorkerThreadPool::init(int, float) /home/alvin/godot-dev/src-mingw/core/object/worker_thread_pool.cpp:711:21
    #5 0x7ff66736ea22 in Main::setup(char const*, int, char**, bool) /home/alvin/godot-dev/src-mingw/main/main.cpp:1804:39
    #6 0x7ff66710204e in widechar_main(int, wchar_t**) /home/alvin/godot-dev/src-mingw/platform/windows/godot_windows.cpp:90:14
    #7 0x7ff6671026c5 in _main() /home/alvin/godot-dev/src-mingw/platform/windows/godot_windows.cpp:131:11
    #8 0x7ff667102787 in main /home/alvin/godot-dev/src-mingw/platform/windows/godot_windows.cpp:150:9
    #9 0x7ff667101310 in __tmainCRTStartup /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:267:15
    #10 0x7ff667101155 in .l_startw /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:157:9
    #11 0x7ffe32947373  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017373)
    #12 0x7ffe33a3cc90  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18004cc90)

SUMMARY: AddressSanitizer: heap-use-after-free /home/alvin/godot-dev/src-mingw/core/object/worker_thread_pool.cpp:162:15 in WorkerThreadPool::_process_task(WorkerThreadPool::Task*)
Shadow bytes around the buggy address:
  0x124a8b720800: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x124a8b720880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
  0x124a8b720900: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x124a8b720980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
  0x124a8b720a00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
=>0x124a8b720a80: fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fa
  0x124a8b720b00: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x124a8b720b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
  0x124a8b720c00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x124a8b720c80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x124a8b720d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==14784==ABORTING

@alvinhochun alvinhochun requested a review from a team as a code owner July 27, 2024 13:30
@akien-mga akien-mga merged commit 5793d79 into godotengine:master Jul 29, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@alvinhochun alvinhochun deleted the workerthreadpool-uaf branch July 29, 2024 16:23
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