ASAN heap-use-after-free error in Envoy::Tcp::ConnPoolImpl::onPoolReady

[roelfdutoit]

Description:
When we test our Envoy-based application with ASAN enabled we hit a heap-use-after-free error in Envoy::Tcp::ConnPoolImpl::onPoolReady. The callbacks_ pointer seems to be dangling. Our assessment of the issue is that the upstream connection still has pending network events after the TcpConnPool dtor is called.

Adding the following code to Envoy::Extensions::Upstreams::Http::Tcp::TcpConnPool seems to solve the problem:

~TcpConnPool() override { cancelAnyPendingStream(); }

Repro steps:
The test setup is a performance test with lots of HTTP CONNECT requests per second and many concurrent connections. The issue seems to occur when the test is stopped midway, which results in connection closure patterns that differ from connection closures of a stable-state running test.

Call Stack:

==86300==ERROR: AddressSanitizer: heap-use-after-free on address 0x607003856478 at pc 0x5609f63c491c bp 0x7f0967155070 sp 0x7f0967155068
READ of size 8 at 0x607003856478 thread T18
    #0 0x5609f63c491b in Envoy::Tcp::ConnPoolImpl::onPoolReady(Envoy::ConnectionPool::ActiveClient&, Envoy::ConnectionPool::AttachContext&) /proc/self/cwd/external/envoy/source/common/tcp/conn_pool.cc:188:14
    #1 0x5609f640ecf0 in Envoy::ConnectionPool::ConnPoolImplBase::attachStreamToClient(Envoy::ConnectionPool::ActiveClient&, Envoy::ConnectionPool::AttachContext&) /proc/self/cwd/external/envoy/source/common/conn_pool/conn_pool_base.cc:210:3
    #2 0x5609f64122bb in Envoy::ConnectionPool::ConnPoolImplBase::onUpstreamReady() /proc/self/cwd/external/envoy/source/common/conn_pool/conn_pool_base.cc:335:5
    #3 0x5609f641c652 in Envoy::ConnectionPool::ConnPoolImplBase::onConnectionEvent(Envoy::ConnectionPool::ActiveClient&, std::basic_string_view<char, std::char_traits<char>>, Envoy::Network::ConnectionEvent) /proc/self/cwd/external/envoy/source/common/conn_pool/conn_pool_base.cc:608:7
    #4 0x5609f63c1a87 in Envoy::Tcp::ActiveTcpClient::onEvent(Envoy::Network::ConnectionEvent) /proc/self/cwd/external/envoy/source/common/tcp/conn_pool.cc:94:11
    #5 0x5609f7421835 in Envoy::Network::ConnectionImplBase::raiseConnectionEvent(Envoy::Network::ConnectionEvent) /proc/self/cwd/external/envoy/source/common/network/connection_impl_base.cc:62:17
    #6 0x5609f7324c9e in Envoy::Network::ConnectionImpl::raiseEvent(Envoy::Network::ConnectionEvent) /proc/self/cwd/external/envoy/source/common/network/connection_impl.cc:470:23
    #7 0x5609f73270ac in non-virtual thunk to Envoy::Network::ConnectionImpl::raiseEvent(Envoy::Network::ConnectionEvent) /proc/self/cwd/external/envoy/source/common/network/connection_impl.cc
    #8 0x5609f734a187 in Envoy::Network::RawBufferSocket::onConnected() /proc/self/cwd/external/envoy/source/common/network/raw_buffer_socket.cc:91:51
    #9 0x5609f5306677 in Envoy::Extensions::TransportSockets::PassthroughSocket::onConnected() /proc/self/cwd/external/envoy/source/extensions/transport_sockets/common/passthrough.cc:43:60
    #10 0x5609f7325037 in Envoy::Network::ConnectionImpl::onConnected() /proc/self/cwd/external/envoy/source/common/network/connection_impl.cc:312:22
    #11 0x5609f733e961 in Envoy::Network::ClientConnectionImpl::onConnected() /proc/self/cwd/external/envoy/source/common/network/connection_impl.cc:1051:19
    #12 0x5609f733045d in Envoy::Network::ConnectionImpl::onWriteReady() /proc/self/cwd/external/envoy/source/common/network/connection_impl.cc:748:7
    #13 0x5609f732b8f7 in Envoy::Network::ConnectionImpl::onFileEvent(unsigned int) /proc/self/cwd/external/envoy/source/common/network/connection_impl.cc:642:5
   :

0x607003856478 is located 8 bytes inside of 80-byte region [0x607003856470,0x6070038564c0)
freed by thread T18 here:
    #0 0x5609f39a0622 in free /local/mnt/workspace/bcain_clang_hu-bcain-lv_22036/final/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3
    #1 0x5609f6d1f2c3 in Envoy::Extensions::Upstreams::Http::Tcp::TcpConnPool::~TcpConnPool() /proc/self/cwd/external/envoy/source/extensions/upstreams/http/tcp/upstream_request.h:23:7
    #2 0x5609f6d19465 in std::unique_ptr<Envoy::Router::GenericConnPool, std::default_delete<Envoy::Router::GenericConnPool>>::~unique_ptr() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/unique_ptr.h:85:2
    #3 0x5609f6d7cf43 in Envoy::Router::UpstreamRequest::~UpstreamRequest() /proc/self/cwd/external/envoy/source/common/router/upstream_request.cc:157:50
    #4 0x5609f6d7ea84 in Envoy::Router::UpstreamRequest::~UpstreamRequest() /proc/self/cwd/external/envoy/source/common/router/upstream_request.cc:157:37
  : 

previously allocated by thread T18 here:
    #0 0x5609f39a08ce in malloc /local/mnt/workspace/bcain_clang_hu-bcain-lv_22036/final/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
    #1 0x7f097a0ae98b in operator new(unsigned long) (/lib/x86_64-linux-gnu/libstdc++.so.6+0xae98b) (BuildId: e37fe1a879783838de78cbc8c80621fa685d58a2)
    #2 0x5609f6d18bf9 in Envoy::Extensions::Upstreams::Http::Generic::GenericGenericConnPoolFactory::createGenericConnPool(Envoy::Upstream::ThreadLocalCluster&, Envoy::Router::GenericConnPoolFactory::UpstreamProtocol, Envoy::Upstream::ResourcePriority, std::optional<Envoy::Http::Protocol>, Envoy::Upstream::LoadBalancerContext*) const /proc/self/cwd/external/envoy/source/extensions/upstreams/http/generic/config.cc:27:9
    #3 0x5609f6d3e29e in Envoy::Router::Filter::createConnPool(Envoy::Upstream::ThreadLocalCluster&) /proc/self/cwd/external/envoy/source/common/router/router.cc:837:19
    #4 0x5609f6d34093 in Envoy::Router::Filter::decodeHeaders(Envoy::Http::RequestHeaderMap&, bool) /proc/self/cwd/external/envoy/source/common/router/router.cc:642:56
    #5 0x5609f6f589cf in Envoy::Http::ActiveStreamDecoderFilter::decodeHeaders(Envoy::Http::RequestHeaderMap&, bool) /proc/self/cwd/external/envoy/source/common/http/filter_manager.h:276:43
    #6 0x5609f6f3edfd in Envoy::Http::FilterManager::decodeHeaders(Envoy::Http::ActiveStreamDecoderFilter*, Envoy::Http::RequestHeaderMap&, bool) /proc/self/cwd/external/envoy/source/common/http/filter_manager.cc:583:44
    #7 0x5609f6f3e1c3 in Envoy::Http::ActiveStreamDecoderFilter::doHeaders(bool) /proc/self/cwd/external/envoy/source/common/http/filter_manager.cc:421:11
    #8 0x5609f6f38cad in Envoy::Http::ActiveStreamFilterBase::commonContinue() /proc/self/cwd/external/envoy/source/common/http/filter_manager.cc:94:5
    #9 0x5609f6f4b5ac in non-virtual thunk to Envoy::Http::ActiveStreamDecoderFilter::continueDecoding() /proc/self/cwd/external/envoy/source/common/http/filter_manager.cc:487:54
  : 

[phlax]

@roelfdutoit would you be able to create a PR with your suggested change ?

cc @alyssawilk @ggreenway @mattklein123

[alyssawilk]

AFIK We shouldn't be deleting the pool with open streams so worth debugging, but definitely a reasonable interim fix. I'd lean towards ENVOY_BUG on open streams as well in the hopes we track down the larger problem less urgently.

[roelfdutoit]

@roelfdutoit would you be able to create a PR with your suggested change ?

I can certainly do that, yes. I am not sure I'll be able to add a gtest for that though.