Hello Dan Burkert, Alexey Serbin,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5853
to review the following change.
Change subject: openssl_util: avoid non-POD vector in static storage
......................................................................
openssl_util: avoid non-POD vector in static storage
This addresses an ASAN crash I've seen a couple times now in test runs
during static destruction. We previously used a vector<Mutex*> for the
OpenSSL locks, but the order of static destruction isn't very easy to
predict, and the destructor of things like SSL sockets may end up
needing to acquire these locks.
This patch switches to a C-style dynamic array.
An example ASAN trace follows.
=================================================================
==28629==ERROR: AddressSanitizer: heap-use-after-free on address 0x61300000b6c0
at pc 0x7f25c7d03339 bp 0x7f25bbdc3af0 sp 0x7f25bbdc3ae8
READ of size 8 at 0x61300000b6c0 thread T4 (rpc reactor-286)
#0 0x7f25c7d03338 in kudu::security::(anonymous namespace)::LockingCB(int,
int, char const*, int)
/home/jenkins-slave/workspace/kudu-1/src/kudu/security/openssl_util.cc:54:14
#1 0x7f25c4c41836 in CRYPTO_add_lock
(/lib/x86_64-linux-gnu/libcrypto.so.1.0.0+0x5f836)
#2 0x7f25c49bcedb in SSL_free
(/lib/x86_64-linux-gnu/libssl.so.1.0.0+0x39edb)
#3 0x7f25c7d10f20 in std::_Function_handler<void (ssl_st*), void
(*)(ssl_st*)>::_M_invoke(std::_Any_data const&, ssl_st*)
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/functional:2071:2
#4 0x7f25c8764ed4 in std::function<void (ssl_st*)>::operator()(ssl_st*)
const
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/functional:2471:14
#5 0x7f25c7d0daf7 in std::unique_ptr<ssl_st, std::function<void (ssl_st*)>
>::reset(ssl_st*)
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:262:4
#6 0x7f25c7d14f78 in kudu::security::TlsSocket::Close()
/home/jenkins-slave/workspace/kudu-1/src/kudu/security/tls_socket.cc:115:8
#7 0x7f25c8773072 in kudu::rpc::Connection::Shutdown(kudu::Status const&)
/home/jenkins-slave/workspace/kudu-1/src/kudu/rpc/connection.cc:174:5
#8 0x7f25c87bfb25 in kudu::rpc::ReactorThread::ShutdownInternal()
/home/jenkins-slave/workspace/kudu-1/src/kudu/rpc/reactor.cc:134:11
#9 0x7f25c87c0f3f in kudu::rpc::ReactorThread::AsyncHandler(ev::async&,
int) /home/jenkins-slave/workspace/kudu-1/src/kudu/rpc/reactor.cc:198:5
...
0x61300000b6c0 is located 128 bytes inside of 328-byte region
[0x61300000b640,0x61300000b788)
freed by thread T0 here:
#0 0x5c1f30 in operator delete(void*)
/home/jenkins-slave/workspace/kudu-1/thirdparty/src/llvm-3.9.1.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:110
#1 0x7f25c7d05235 in std::_Vector_base<kudu::Mutex*,
std::allocator<kudu::Mutex*> >::~_Vector_base()
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:160:9
#2 0x7f25c2b30539 in __cxa_finalize
/build/eglibc-oGUzwX/eglibc-2.19/stdlib/cxa_finalize.c:56
previously allocated by thread T0 here:
#0 0x5c1870 in operator new(unsigned long)
/home/jenkins-slave/workspace/kudu-1/thirdparty/src/llvm-3.9.1.src/projects/compiler-rt/lib/asan/asan_new_delete.cc:78
#1 0x7f25c7d03cfc in kudu::Mutex** std::vector<kudu::Mutex*,
std::allocator<kudu::Mutex*>
>::_M_allocate_and_copy<std::move_iterator<kudu::Mutex**> >(unsigned long,
std::move_iterator<kudu::Mutex**>, std::move_iterator<kudu::Mutex**>)
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../
#2 0x7f25c7d038e6 in std::vector<kudu::Mutex*, std::allocator<kudu::Mutex*>
>::reserve(unsigned long)
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/vector.tcc:73:20
#3 0x7f25c7d02420 in kudu::security::(anonymous
namespace)::DoInitializeOpenSSL()
/home/jenkins-slave/workspace/kudu-1/src/kudu/security/openssl_util.cc:78:16
...
Change-Id: Id6fdd1162eb39114c67f3c46073345829530434f
---
M src/kudu/security/openssl_util.cc
1 file changed, 7 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/5853/1
--
To view, visit http://gerrit.cloudera.org:8080/5853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id6fdd1162eb39114c67f3c46073345829530434f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
