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 connection after free #479

Merged
merged 1 commit into from
Feb 13, 2022
Merged

Fix use connection after free #479

merged 1 commit into from
Feb 13, 2022

Conversation

git-hulk
Copy link
Member

@git-hulk git-hulk commented Feb 12, 2022

This bug was reported by the address sanitizer which we want to integrate into Kvrocks ci workflow, after investigating the error and found it's imported by #430. The solution was thatclose_cb should be ran before removing the connection.

==4657==ERROR: AddressSanitizer: heap-use-after-free on address 0x614000010070 at pc 0x00010bf183cb bp 0x70000f255700 sp 0x70000f2556f8
READ of size 8 at 0x614000010070 thread T88
    #0 0x10bf183ca in std::__1::__function::__value_func<void (int)>::operator bool() const functional:1927
    #1 0x10bf04eb4 in std::__1::function<void (int)>::operator bool() const functional:2412
    #2 0x10bf04e5f in Redis::Connection::Close() redis_connection.cc:50
    #3 0x10bf093a7 in Redis::Connection::OnEvent(bufferevent*, short, void*) redis_connection.cc:96
    #4 0x10d1316d9 in bufferevent_run_deferred_callbacks_unlocked bufferevent.c:225
    #5 0x10d142b00 in event_process_active_single_queue event.c:1703
    #6 0x10d13d440 in event_process_active event.c:1789
    #7 0x10d13c37a in event_base_loop event.c:2012
    #8 0x10d13c056 in event_base_dispatch event.c:1823
    #9 0x10bf2c181 in Worker::Run(std::__1::__thread_id) worker.cc:131
    #10 0x10bf46b9e in WorkerThread::Start()::$_0::operator()() const worker.cc:332
    #11 0x10bf4699c in decltype(std::__1::forward<WorkerThread::Start()::$_0>(fp)()) std::__1::__invoke<WorkerThread::Start()::$_0>(WorkerThread::Start()::$_0&&) type_traits:3694
    #12 0x10bf46934 in void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0>&, std::__1::__tuple_indices<>) thread:286
    #13 0x10bf45a72 in void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0> >(void*) thread:297
    #14 0x7ff8038d74f3 in _pthread_start+0x7c (libsystem_pthread.dylib:x86_64+0x64f3)
    #15 0x7ff8038d300e in thread_start+0xe (libsystem_pthread.dylib:x86_64+0x200e)

0x614000010070 is located 48 bytes inside of 432-byte region [0x614000010040,0x6140000101f0)
freed by thread T88 here:
    #0 0x1110c2d1d in wrap__ZdlPv+0x7d (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x50d1d)
    #1 0x10bf2dfa1 in Worker::FreeConnection(Redis::Connection*) worker.cc:199
    #2 0x10bf04e52 in Redis::Connection::Close() redis_connection.cc:49
    #3 0x10bf093a7 in Redis::Connection::OnEvent(bufferevent*, short, void*) redis_connection.cc:96
    #4 0x10d1316d9 in bufferevent_run_deferred_callbacks_unlocked bufferevent.c:225
    #5 0x10d142b00 in event_process_active_single_queue event.c:1703
    #6 0x10d13d440 in event_process_active event.c:1789
    #7 0x10d13c37a in event_base_loop event.c:2012
    #8 0x10d13c056 in event_base_dispatch event.c:1823
    #9 0x10bf2c181 in Worker::Run(std::__1::__thread_id) worker.cc:131
    #10 0x10bf46b9e in WorkerThread::Start()::$_0::operator()() const worker.cc:332
    #11 0x10bf4699c in decltype(std::__1::forward<WorkerThread::Start()::$_0>(fp)()) std::__1::__invoke<WorkerThread::Start()::$_0>(WorkerThread::Start()::$_0&&) type_traits:3694
    #12 0x10bf46934 in void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0>&, std::__1::__tuple_indices<>) thread:286
    #13 0x10bf45a72 in void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0> >(void*) thread:297
    #14 0x7ff8038d74f3 in _pthread_start+0x7c (libsystem_pthread.dylib:x86_64+0x64f3)
    #15 0x7ff8038d300e in thread_start+0xe (libsystem_pthread.dylib:x86_64+0x200e)

previously allocated by thread T88 here:
    #0 0x1110c28fd in wrap__Znwm+0x7d (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x508fd)
    #1 0x10bf2a9ff in Worker::newConnection(evconnlistener*, int, sockaddr*, int, void*) worker.cc:84
    #2 0x10d14cf2a in listener_read_cb listener.c:424
    #3 0x10d14335f in event_persist_closure event.c:1629
    #4 0x10d1429d8 in event_process_active_single_queue event.c:1688
    #5 0x10d13d440 in event_process_active event.c:1789
    #6 0x10d13c37a in event_base_loop event.c:2012
    #7 0x10d13c056 in event_base_dispatch event.c:1823
    #8 0x10bf2c181 in Worker::Run(std::__1::__thread_id) worker.cc:131
    #9 0x10bf46b9e in WorkerThread::Start()::$_0::operator()() const worker.cc:332
    #10 0x10bf4699c in decltype(std::__1::forward<WorkerThread::Start()::$_0>(fp)()) std::__1::__invoke<WorkerThread::Start()::$_0>(WorkerThread::Start()::$_0&&) type_traits:3694
    #11 0x10bf46934 in void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0>&, std::__1::__tuple_indices<>) thread:286
    #12 0x10bf45a72 in void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, WorkerThread::Start()::$_0> >(void*) thread:297
    #13 0x7ff8038d74f3 in _pthread_start+0x7c (libsystem_pthread.dylib:x86_64+0x64f3)
    #14 0x7ff8038d300e in thread_start+0xe (libsystem_pthread.dylib:x86_64+0x200e)

Thread T88 created by T0 here:
    #0 0x1110b04fc in wrap_pthread_create+0x5c (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x3e4fc)
    #1 0x10bf4593b in std::__1::__libcpp_thread_create(_opaque_pthread_t**, void* (*)(void*), void*) __threading_support:510
    #2 0x10bf454d2 in std::__1::thread::thread<WorkerThread::Start()::$_0, void>(WorkerThread::Start()::$_0&&) thread:313
    #3 0x10bf3394c in std::__1::thread::thread<WorkerThread::Start()::$_0, void>(WorkerThread::Start()::$_0&&) thread:305
    #4 0x10bf334df in WorkerThread::Start() worker.cc:330
    #5 0x10c5d003a in Server::Start() server.cc:77
    #6 0x10bf4a63b in main main.cc:327
    #7 0x11d05f4fd in start+0x1cd (dyld:x86_64+0x54fd)

SUMMARY: AddressSanitizer: heap-use-after-free functional:1927 in std::__1::__function::__value_func<void (int)>::operator bool() const
Shadow bytes around the buggy address:
  0x1c2800001fb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2800001fc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2800001fd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2800001fe0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2800001ff0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x1c2800002000: fa fa fa fa fa fa fa fa fd fd fd fd fd fd[fd]fd
  0x1c2800002010: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2800002020: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2800002030: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa
  0x1c2800002040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2800002050: 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
  Shadow gap:              cc
==4657==ABORTING

@git-hulk git-hulk merged commit f3a1fb5 into apache:unstable Feb 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants