Skip to content

Commit

Permalink
TCPIOHandler: Fix a race when creating the first TLS connections
Browse files Browse the repository at this point in the history 
We used to get a index from OpenSSL when the first incoming or
outgoing TLS connection was created. That index is later used
to store application-related data along the TLS connection, to
be able to access it in callbacks called from inside the OpenSSL
library.
Unfortunately the atomic flag construction used was racy: if a second
connection is created while the first one is still getting the index,
it could be reading the initial value (-1) instead of the actual index,
which might prevent the callback from working properly because they
cannot retrieve the necessary data. The good news is that this should
not have a serious impact: a TLS session might not be properly resumed
while it should have been, leading to a full TLS session negotiation.
This commit fixes the race by using a full mutex and only releasing
it once the value has been computed. In order to avoid a performance
penalty, the index is now computed when a TLS connection _context_
is created, instead of a TLS connection. TLS contexts should be
reused for a large number of connections, and mostly created during
startup or in the first few seconds of the application.

The race was reported by Thread Sanitizer during the
`test_TLS.py::TestTLSFrontendLimits::testTCPConnsPerTLSFrontend`
regression test as:
```
WARNING: ThreadSanitizer: data race (pid=120466)
  Read of size 4 at 0x55a12bf3d758 by thread T4:
    #0 OpenSSLTLSConnection::OpenSSLTLSConnection(int, timeval const&, std::shared_ptr<OpenSSLFrontendContext>) /work/pdns/pdns/dnsdistdist/tcpiohandler.cc:106 (dnsdist+0x97ece8) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #1 std::__detail::_MakeUniq<OpenSSLTLSConnection>::__single_object std::make_unique<OpenSSLTLSConnection, int&, timeval const&, std::shared_ptr<OpenSSLFrontendContext>&>(int&, timeval const&, std::shared_ptr<OpenSSLFrontendContext>&) /usr/include/c++/13.2.1/bits/unique_ptr.h:1070 (dnsdist+0x97eff6) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #2 OpenSSLTLSIOCtx::getConnection(int, timeval const&, long) /work/pdns/pdns/dnsdistdist/tcpiohandler.cc:797 (dnsdist+0x97eff6)
    #3 TCPIOHandler::TCPIOHandler(int, timeval const&, std::shared_ptr<TLSCtx>, long) /work/pdns/pdns/dnsdistdist/tcpiohandler.hh:246 (dnsdist+0x88c24f) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #4 IncomingTCPConnectionState::IncomingTCPConnectionState(ConnectionInfo&&, TCPClientThreadData&, timeval const&) /work/pdns/pdns/dnsdistdist/dnsdist-tcp-upstream.hh:29 (dnsdist+0x88c24f)
    #5 void std::_Construct<IncomingTCPConnectionState, ConnectionInfo, TCPClientThreadData&, timeval&>(IncomingTCPConnectionState*, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/stl_construct.h:119 (dnsdist+0x878b1e) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #6 void std::allocator_traits<std::allocator<void> >::construct<IncomingTCPConnectionState, ConnectionInfo, TCPClientThreadData&, timeval&>(std::allocator<void>&, IncomingTCPConnectionState*, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/alloc_traits.h:660 (dnsdist+0x878b1e)
    #7 std::_Sp_counted_ptr_inplace<IncomingTCPConnectionState, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<ConnectionInfo, TCPClientThreadData&, timeval&>(std::allocator<void>, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr_base.h:604 (dnsdist+0x878b1e)
    #8 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<IncomingTCPConnectionState, std::allocator<void>, ConnectionInfo, TCPClientThreadData&, timeval&>(IncomingTCPConnectionState*&, std::_Sp_alloc_shared_tag<std::allocator<void> >, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr_base.h:971 (dnsdist+0x878b1e)
    #9 std::__shared_ptr<IncomingTCPConnectionState, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<void>, ConnectionInfo, TCPClientThreadData&, timeval&>(std::_Sp_alloc_shared_tag<std::allocator<void> >, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr_base.h:1712 (dnsdist+0x878b1e)
    #10 std::shared_ptr<IncomingTCPConnectionState>::shared_ptr<std::allocator<void>, ConnectionInfo, TCPClientThreadData&, timeval&>(std::_Sp_alloc_shared_tag<std::allocator<void> >, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr.h:464 (dnsdist+0x878b1e)
    #11 std::shared_ptr<std::enable_if<!std::is_array<IncomingTCPConnectionState>::value, IncomingTCPConnectionState>::type> std::make_shared<IncomingTCPConnectionState, ConnectionInfo, TCPClientThreadData&, timeval&>(ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr.h:1010 (dnsdist+0x878b1e)
    #12 handleIncomingTCPQuery /work/pdns/pdns/dnsdistdist/dnsdist-tcp.cc:1118 (dnsdist+0x878b1e)
    #13 void std::__invoke_impl<void, void (*&)(int, boost::any&), int, boost::any&>(std::__invoke_other, void (*&)(int, boost::any&), int&&, boost::any&) /usr/include/c++/13.2.1/bits/invoke.h:61 (dnsdist+0x32d951) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #14 std::enable_if<is_invocable_r_v<void, void (*&)(int, boost::any&), int, boost::any&>, void>::type std::__invoke_r<void, void (*&)(int, boost::any&), int, boost::any&>(void (*&)(int, boost::any&), int&&, boost::any&) /usr/include/c++/13.2.1/bits/invoke.h:111 (dnsdist+0x32d951)
    #15 std::_Function_handler<void (int, boost::any&), void (*)(int, boost::any&)>::_M_invoke(std::_Any_data const&, int&&, boost::any&) /usr/include/c++/13.2.1/bits/std_function.h:290 (dnsdist+0x32d951)
    #16 std::function<void (int, boost::any&)>::operator()(int, boost::any&) const /usr/include/c++/13.2.1/bits/std_function.h:591 (dnsdist+0x98fc0f) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #17 EpollFDMultiplexer::run(timeval*, int) /work/pdns/pdns/dnsdistdist/epollmplexer.cc:190 (dnsdist+0x98fc0f)
    #18 tcpClientThread /work/pdns/pdns/dnsdistdist/dnsdist-tcp.cc:1251 (dnsdist+0x86cb7f) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #19 void std::__invoke_impl<void, void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > >(std::__invoke_other, void (*&&)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >&&) /usr/include/c++/13.2.1/bits/invoke.h:61 (dnsdist+0x87aab1) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #20 std::__invoke_result<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > >::type std::__invoke<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > >(void (*&&)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >&&) /usr/include/c++/13.2.1/bits/invoke.h:96 (dnsdist+0x87aab1)
    #21 void std::thread::_Invoker<std::tuple<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > > >::_M_invoke<0ul, 1ul, 2ul, 3ul, 4ul, 5ul>(std::_Index_tuple<0ul, 1ul, 2ul, 3ul, 4ul, 5ul>) /usr/include/c++/13.2.1/bits/std_thread.h:292 (dnsdist+0x87aab1)
    #22 std::thread::_Invoker<std::tuple<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > > >::operator()() /usr/include/c++/13.2.1/bits/std_thread.h:299 (dnsdist+0x87aab1)
    #23 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > > > >::_M_run() /usr/include/c++/13.2.1/bits/std_thread.h:244 (dnsdist+0x87aab1)
    #24 execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:104 (libstdc++.so.6+0xe1942) (BuildId: 207eb738c5976dd9aac1ae0640fc4de5946b547e)

  Previous write of size 4 at 0x55a12bf3d758 by thread T3:
    #0 OpenSSLTLSConnection::OpenSSLTLSConnection(int, timeval const&, std::shared_ptr<OpenSSLFrontendContext>) /work/pdns/pdns/dnsdistdist/tcpiohandler.cc:88 (dnsdist+0x97ed98) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #1 std::__detail::_MakeUniq<OpenSSLTLSConnection>::__single_object std::make_unique<OpenSSLTLSConnection, int&, timeval const&, std::shared_ptr<OpenSSLFrontendContext>&>(int&, timeval const&, std::shared_ptr<OpenSSLFrontendContext>&) /usr/include/c++/13.2.1/bits/unique_ptr.h:1070 (dnsdist+0x97eff6) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #2 OpenSSLTLSIOCtx::getConnection(int, timeval const&, long) /work/pdns/pdns/dnsdistdist/tcpiohandler.cc:797 (dnsdist+0x97eff6)
    #3 TCPIOHandler::TCPIOHandler(int, timeval const&, std::shared_ptr<TLSCtx>, long) /work/pdns/pdns/dnsdistdist/tcpiohandler.hh:246 (dnsdist+0x88c24f) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #4 IncomingTCPConnectionState::IncomingTCPConnectionState(ConnectionInfo&&, TCPClientThreadData&, timeval const&) /work/pdns/pdns/dnsdistdist/dnsdist-tcp-upstream.hh:29 (dnsdist+0x88c24f)
    #5 void std::_Construct<IncomingTCPConnectionState, ConnectionInfo, TCPClientThreadData&, timeval&>(IncomingTCPConnectionState*, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/stl_construct.h:119 (dnsdist+0x878b1e) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #6 void std::allocator_traits<std::allocator<void> >::construct<IncomingTCPConnectionState, ConnectionInfo, TCPClientThreadData&, timeval&>(std::allocator<void>&, IncomingTCPConnectionState*, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/alloc_traits.h:660 (dnsdist+0x878b1e)
    #7 std::_Sp_counted_ptr_inplace<IncomingTCPConnectionState, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<ConnectionInfo, TCPClientThreadData&, timeval&>(std::allocator<void>, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr_base.h:604 (dnsdist+0x878b1e)
    #8 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<IncomingTCPConnectionState, std::allocator<void>, ConnectionInfo, TCPClientThreadData&, timeval&>(IncomingTCPConnectionState*&, std::_Sp_alloc_shared_tag<std::allocator<void> >, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr_base.h:971 (dnsdist+0x878b1e)
    #9 std::__shared_ptr<IncomingTCPConnectionState, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<void>, ConnectionInfo, TCPClientThreadData&, timeval&>(std::_Sp_alloc_shared_tag<std::allocator<void> >, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr_base.h:1712 (dnsdist+0x878b1e)
    #10 std::shared_ptr<IncomingTCPConnectionState>::shared_ptr<std::allocator<void>, ConnectionInfo, TCPClientThreadData&, timeval&>(std::_Sp_alloc_shared_tag<std::allocator<void> >, ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr.h:464 (dnsdist+0x878b1e)
    #11 std::shared_ptr<std::enable_if<!std::is_array<IncomingTCPConnectionState>::value, IncomingTCPConnectionState>::type> std::make_shared<IncomingTCPConnectionState, ConnectionInfo, TCPClientThreadData&, timeval&>(ConnectionInfo&&, TCPClientThreadData&, timeval&) /usr/include/c++/13.2.1/bits/shared_ptr.h:1010 (dnsdist+0x878b1e)
    #12 handleIncomingTCPQuery /work/pdns/pdns/dnsdistdist/dnsdist-tcp.cc:1118 (dnsdist+0x878b1e)
    #13 void std::__invoke_impl<void, void (*&)(int, boost::any&), int, boost::any&>(std::__invoke_other, void (*&)(int, boost::any&), int&&, boost::any&) /usr/include/c++/13.2.1/bits/invoke.h:61 (dnsdist+0x32d951) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #14 std::enable_if<is_invocable_r_v<void, void (*&)(int, boost::any&), int, boost::any&>, void>::type std::__invoke_r<void, void (*&)(int, boost::any&), int, boost::any&>(void (*&)(int, boost::any&), int&&, boost::any&) /usr/include/c++/13.2.1/bits/invoke.h:111 (dnsdist+0x32d951)
    #15 std::_Function_handler<void (int, boost::any&), void (*)(int, boost::any&)>::_M_invoke(std::_Any_data const&, int&&, boost::any&) /usr/include/c++/13.2.1/bits/std_function.h:290 (dnsdist+0x32d951)
    #16 std::function<void (int, boost::any&)>::operator()(int, boost::any&) const /usr/include/c++/13.2.1/bits/std_function.h:591 (dnsdist+0x98fc0f) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #17 EpollFDMultiplexer::run(timeval*, int) /work/pdns/pdns/dnsdistdist/epollmplexer.cc:190 (dnsdist+0x98fc0f)
    #18 tcpClientThread /work/pdns/pdns/dnsdistdist/dnsdist-tcp.cc:1251 (dnsdist+0x86cb7f) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #19 void std::__invoke_impl<void, void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > >(std::__invoke_other, void (*&&)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >&&) /usr/include/c++/13.2.1/bits/invoke.h:61 (dnsdist+0x87aab1) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #20 std::__invoke_result<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > >::type std::__invoke<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > >(void (*&&)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >&&) /usr/include/c++/13.2.1/bits/invoke.h:96 (dnsdist+0x87aab1)
    #21 void std::thread::_Invoker<std::tuple<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > > >::_M_invoke<0ul, 1ul, 2ul, 3ul, 4ul, 5ul>(std::_Index_tuple<0ul, 1ul, 2ul, 3ul, 4ul, 5ul>) /usr/include/c++/13.2.1/bits/std_thread.h:292 (dnsdist+0x87aab1)
    #22 std::thread::_Invoker<std::tuple<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > > >::operator()() /usr/include/c++/13.2.1/bits/std_thread.h:299 (dnsdist+0x87aab1)
    #23 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >&&, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >&&, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >&&, std::vector<ClientState*, std::allocator<ClientState*> >), pdns::channel::Receiver<ConnectionInfo, std::default_delete<ConnectionInfo> >, pdns::channel::Receiver<CrossProtocolQuery, std::default_delete<CrossProtocolQuery> >, pdns::channel::Receiver<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, pdns::channel::Sender<TCPCrossProtocolResponse, std::default_delete<TCPCrossProtocolResponse> >, std::vector<ClientState*, std::allocator<ClientState*> > > > >::_M_run() /usr/include/c++/13.2.1/bits/std_thread.h:244 (dnsdist+0x87aab1)
    #24 execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:104 (libstdc++.so.6+0xe1942) (BuildId: 207eb738c5976dd9aac1ae0640fc4de5946b547e)

  Location is global 'OpenSSLTLSConnection::s_tlsConnIndex' of size 4 at 0x55a12bf3d758 (dnsdist+0xc49758)

  Thread T4 'dnsdist/tcpClie' (tid=120471, running) created by main thread at:
    #0 pthread_create /usr/src/debug/gcc/gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1036 (libtsan.so.2+0x44219) (BuildId: 7e8fcb9ed0a63b98f2293e37c92ac955413efd9e)
    #1 __gthread_create /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663 (libstdc++.so.6+0xe1a29) (BuildId: 207eb738c5976dd9aac1ae0640fc4de5946b547e)
    #2 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:172 (libstdc++.so.6+0xe1a29)
    #3 TCPClientCollection::addTCPClientThread(std::vector<ClientState*, std::allocator<ClientState*> >&) /work/pdns/pdns/dnsdistdist/dnsdist-tcp.cc:149 (dnsdist+0x8685a1) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #4 TCPClientCollection::TCPClientCollection(unsigned long, std::vector<ClientState*, std::allocator<ClientState*> >) /work/pdns/pdns/dnsdistdist/dnsdist-tcp.cc:126 (dnsdist+0x868912) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #5 std::__detail::_MakeUniq<TCPClientCollection>::__single_object std::make_unique<TCPClientCollection, unsigned long&, std::vector<ClientState*, std::allocator<ClientState*> > >(unsigned long&, std::vector<ClientState*, std::allocator<ClientState*> >&&) /usr/include/c++/13.2.1/bits/unique_ptr.h:1070 (dnsdist+0x20adef) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #6 main /work/pdns/pdns/dnsdistdist/dnsdist.cc:2865 (dnsdist+0x20adef)

  Thread T3 'dnsdist/tcpClie' (tid=120470, running) created by main thread at:
    #0 pthread_create /usr/src/debug/gcc/gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1036 (libtsan.so.2+0x44219) (BuildId: 7e8fcb9ed0a63b98f2293e37c92ac955413efd9e)
    #1 __gthread_create /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663 (libstdc++.so.6+0xe1a29) (BuildId: 207eb738c5976dd9aac1ae0640fc4de5946b547e)
    #2 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:172 (libstdc++.so.6+0xe1a29)
    #3 TCPClientCollection::addTCPClientThread(std::vector<ClientState*, std::allocator<ClientState*> >&) /work/pdns/pdns/dnsdistdist/dnsdist-tcp.cc:149 (dnsdist+0x8685a1) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #4 TCPClientCollection::TCPClientCollection(unsigned long, std::vector<ClientState*, std::allocator<ClientState*> >) /work/pdns/pdns/dnsdistdist/dnsdist-tcp.cc:126 (dnsdist+0x868912) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #5 std::__detail::_MakeUniq<TCPClientCollection>::__single_object std::make_unique<TCPClientCollection, unsigned long&, std::vector<ClientState*, std::allocator<ClientState*> > >(unsigned long&, std::vector<ClientState*, std::allocator<ClientState*> >&&) /usr/include/c++/13.2.1/bits/unique_ptr.h:1070 (dnsdist+0x20adef) (BuildId: ad82581368352777fda41d4b4145ba8ec738044c)
    #6 main /work/pdns/pdns/dnsdistdist/dnsdist.cc:2865 (dnsdist+0x20adef)

SUMMARY: ThreadSanitizer: data race /work/pdns/pdns/dnsdistdist/tcpiohandler.cc:106 in OpenSSLTLSConnection::OpenSSLTLSConnection(int, timeval const&, std::shared_ptr<OpenSSLFrontendContext>)
```
  • Loading branch information
@rgacogne
rgacogne committed Aug 23, 2023
1 parent af4d79c commit b56f05b
Showing 1 changed file with 32 additions and 25 deletions.
57 changes: 32 additions & 25 deletions pdns/tcpiohandler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,6 @@ class OpenSSLTLSConnection: public TLSConnection
{
d_socket = socket;

if (!s_initTLSConnIndex.test_and_set()) {
/* not initialized yet */
s_tlsConnIndex = SSL_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr);
if (s_tlsConnIndex == -1) {
throw std::runtime_error("Error getting an index for TLS connection data");
}
}

if (!d_conn) {
vinfolog("Error creating TLS object");
if (g_verbose) {
Expand All @@ -103,22 +95,14 @@ class OpenSSLTLSConnection: public TLSConnection
throw std::runtime_error("Error assigning socket");
}

SSL_set_ex_data(d_conn.get(), s_tlsConnIndex, this);
SSL_set_ex_data(d_conn.get(), getConnectionIndex(), this);
}

/* client-side connection */
OpenSSLTLSConnection(const std::string& hostname, bool hostIsAddr, int socket, const struct timeval& timeout, std::shared_ptr<SSL_CTX>& tlsCtx): d_tlsCtx(tlsCtx), d_conn(std::unique_ptr<SSL, void(*)(SSL*)>(SSL_new(tlsCtx.get()), SSL_free)), d_hostname(hostname), d_timeout(timeout)
{
d_socket = socket;

if (!s_initTLSConnIndex.test_and_set()) {
/* not initialized yet */
s_tlsConnIndex = SSL_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr);
if (s_tlsConnIndex == -1) {
throw std::runtime_error("Error getting an index for TLS connection data");
}
}

if (!d_conn) {
vinfolog("Error creating TLS object");
if (g_verbose) {
Expand Down Expand Up @@ -166,7 +150,7 @@ class OpenSSLTLSConnection: public TLSConnection
#endif
}

SSL_set_ex_data(d_conn.get(), s_tlsConnIndex, this);
SSL_set_ex_data(d_conn.get(), getConnectionIndex(), this);
}

std::vector<int> getAsyncFDs() override
Expand Down Expand Up @@ -579,11 +563,30 @@ class OpenSSLTLSConnection: public TLSConnection
d_ktls = true;
}

static int s_tlsConnIndex;
static void generateConnectionIndexIfNeeded()
{
auto init = s_initTLSConnIndex.lock();
if (*init == true) {
return;
}

private:
static std::atomic_flag s_initTLSConnIndex;
/* not initialized yet */
s_tlsConnIndex = SSL_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr);
if (s_tlsConnIndex == -1) {
throw std::runtime_error("Error getting an index for TLS connection data");
}

*init = true;
}

static int getConnectionIndex()
{
return s_tlsConnIndex;
}

private:
static LockGuarded<bool> s_initTLSConnIndex;
static int s_tlsConnIndex;
std::vector<std::unique_ptr<TLSSession>> d_tlsSessions;
/* server context */
std::shared_ptr<OpenSSLFrontendContext> d_feContext;
Expand All @@ -596,15 +599,17 @@ class OpenSSLTLSConnection: public TLSConnection
bool d_ktls{false};
};

std::atomic_flag OpenSSLTLSConnection::s_initTLSConnIndex = ATOMIC_FLAG_INIT;
int OpenSSLTLSConnection::s_tlsConnIndex = -1;
LockGuarded<bool> OpenSSLTLSConnection::s_initTLSConnIndex{false};
int OpenSSLTLSConnection::s_tlsConnIndex{-1};

class OpenSSLTLSIOCtx: public TLSCtx
{
public:
/* server side context */
OpenSSLTLSIOCtx(TLSFrontend& fe): d_feContext(std::make_shared<OpenSSLFrontendContext>(fe.d_addr, fe.d_tlsConfig))
{
OpenSSLTLSConnection::generateConnectionIndexIfNeeded();

d_ticketsKeyRotationDelay = fe.d_tlsConfig.d_ticketsKeyRotationDelay;

if (fe.d_tlsConfig.d_enableTickets && fe.d_tlsConfig.d_numberOfTicketsKeys > 0) {
Expand Down Expand Up @@ -674,6 +679,8 @@ class OpenSSLTLSIOCtx: public TLSCtx

registerOpenSSLUser();

OpenSSLTLSConnection::generateConnectionIndexIfNeeded();

#ifdef HAVE_TLS_CLIENT_METHOD
d_tlsCtx = std::shared_ptr<SSL_CTX>(SSL_CTX_new(TLS_client_method()), SSL_CTX_free);
#else
Expand Down Expand Up @@ -753,7 +760,7 @@ class OpenSSLTLSIOCtx: public TLSCtx
int ret = libssl_ticket_key_callback(s, ctx->d_ticketKeys, keyName, iv, ectx, hctx, enc);
if (enc == 0) {
if (ret == 0 || ret == 2) {
auto* conn = reinterpret_cast<OpenSSLTLSConnection*>(SSL_get_ex_data(s, OpenSSLTLSConnection::s_tlsConnIndex));
auto* conn = reinterpret_cast<OpenSSLTLSConnection*>(SSL_get_ex_data(s, OpenSSLTLSConnection::getConnectionIndex()));
if (conn != nullptr) {
if (ret == 0) {
conn->setUnknownTicketKey();
Expand Down Expand Up @@ -781,7 +788,7 @@ class OpenSSLTLSIOCtx: public TLSCtx

static int newTicketFromServerCb(SSL* ssl, SSL_SESSION* session)
{
OpenSSLTLSConnection* conn = reinterpret_cast<OpenSSLTLSConnection*>(SSL_get_ex_data(ssl, OpenSSLTLSConnection::s_tlsConnIndex));
OpenSSLTLSConnection* conn = reinterpret_cast<OpenSSLTLSConnection*>(SSL_get_ex_data(ssl, OpenSSLTLSConnection::getConnectionIndex()));
if (session == nullptr || conn == nullptr) {
return 0;
}
Expand Down

0 comments on commit b56f05b

Please sign in to comment.