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.

Sign up for GitHub

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

Jump to bottom

server: reorder SecretManagerImpl field to avoid heap-after-use in te… #4940

Merged
merged 1 commit into from
Nov 1, 2018
Merged

server: reorder SecretManagerImpl field to avoid heap-after-use in te… #4940

merged 1 commit into from
Nov 1, 2018

Conversation

htuch
Copy link
Member

@htuch htuch commented Nov 1, 2018 •
edited
Loading

…ardown.

server_fuzz_test indicated the below crash, where the DispatcherImpl teardown
releases some upstream client SSL related objects that then needs
SecretManagerImpl to unregister. Previously, this was already destructed by
time we were in ~DispatcherImpl(), this PR reorders.

#0 0xc1e826 in size /usr/local/include/c++/v1/__hash_table:809:55
#1 0xc1e826 in bucket_count /usr/local/include/c++/v1/__hash_table:1197
#2 0xc1e826 in std::__1::__hash_iterator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, void*>*> std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> > > >::find<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /usr/local/include/c++/v1/__hash_table:2334
#3 0xc1e278 in unsigned long std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> > > >::__erase_unique<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /usr/local/include/c++/v1/__hash_table:2510:20
#4 0xc1def6 in erase /usr/local/include/c++/v1/unordered_map:1156:59
#5 0xc1def6 in Envoy::Secret::SecretManagerImpl::DynamicSecretProviders<Envoy::Secret::TlsCertificateSdsApi>::removeDynamicSecretProvider(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /bazel-out/k8-fastbuild/bin/source/common/secret/_virtual_includes/secret_manager_impl_lib/common/secret/secret_manager_impl.h:75
#6 0x724aa9 in Envoy::Cleanup::~Cleanup() /bazel-out/k8-fastbuild/bin/source/common/common/_virtual_includes/cleanup_lib/common/common/cleanup.h:11:16
#7 0xc1ff33 in Envoy::Secret::SdsApi::~SdsApi() /bazel-out/k8-fastbuild/bin/source/common/secret/_virtual_includes/sds_api_lib/common/secret/sds_api.h:29:7
#8 0xc188a0 in __release_shared /usr/local/include/c++/v1/memory:3530:9
#9 0xc188a0 in __release_shared /usr/local/include/c++/v1/memory:3572
#10 0xc188a0 in std::__1::shared_ptr<Envoy::Secret::SecretProvider<Envoy::Ssl::TlsCertificateConfig> >::~shared_ptr() /usr/local/include/c++/v1/memory:4508
#11 0x149c922 in Envoy::Ssl::ContextConfigImpl::~ContextConfigImpl() /source/common/ssl/context_config_impl.cc:117:1
#12 0x14a0d8f in Envoy::Ssl::ClientContextConfigImpl::~ClientContextConfigImpl() /bazel-out/k8-fastbuild/bin/source/common/ssl/_virtual_includes/context_config_lib/common/ssl/context_config_impl.h:91:7
#13 0x14a0dc8 in Envoy::Ssl::ClientContextConfigImpl::~ClientContextConfigImpl() /bazel-out/k8-fastbuild/bin/source/common/ssl/_virtual_includes/context_config_lib/common/ssl/context_config_impl.h:91:7
#14 0x149815b in operator() /usr/local/include/c++/v1/memory:2325:5
#15 0x149815b in reset /usr/local/include/c++/v1/memory:2638
#16 0x149815b in ~unique_ptr /usr/local/include/c++/v1/memory:2592
#17 0x149815b in Envoy::Ssl::ClientSslSocketFactory::~ClientSslSocketFactory() /bazel-out/k8-fastbuild/bin/source/common/ssl/_virtual_includes/ssl_socket_lib/common/ssl/ssl_socket.h:83
#18 0x14981c8 in Envoy::Ssl::ClientSslSocketFactory::~ClientSslSocketFactory() /bazel-out/k8-fastbuild/bin/source/common/ssl/_virtual_includes/ssl_socket_lib/common/ssl/ssl_socket.h:83:7
#19 0x1362caf in operator() /usr/local/include/c++/v1/memory:2325:5
#20 0x1362caf in reset /usr/local/include/c++/v1/memory:2638
#21 0x1362caf in ~unique_ptr /usr/local/include/c++/v1/memory:2592
#22 0x1362caf in Envoy::Upstream::ClusterInfoImpl::~ClusterInfoImpl() /bazel-out/k8-fastbuild/bin/source/common/upstream/_virtual_includes/upstream_includes/common/upstream/upstream_impl.h:362
#23 0x1362d28 in Envoy::Upstream::ClusterInfoImpl::~ClusterInfoImpl() /bazel-out/k8-fastbuild/bin/source/common/upstream/_virtual_includes/upstream_includes/common/upstream/upstream_impl.h:362:7
#24 0x66e560 in __release_shared /usr/local/include/c++/v1/memory:3530:9
#25 0x66e560 in __release_shared /usr/local/include/c++/v1/memory:3572
#26 0x66e560 in std::__1::shared_ptr<Envoy::Upstream::ClusterInfo const>::~shared_ptr() /usr/local/include/c++/v1/memory:4508
#27 0x13621bf in Envoy::Upstream::HostImpl::~HostImpl() /bazel-out/k8-fastbuild/bin/source/common/upstream/_virtual_includes/upstream_includes/common/upstream/upstream_impl.h:156:7
#28 0x13621f8 in Envoy::Upstream::HostImpl::~HostImpl() /bazel-out/k8-fastbuild/bin/source/common/upstream/_virtual_includes/upstream_includes/common/upstream/upstream_impl.h:156:7
#29 0x66e650 in __release_shared /usr/local/include/c++/v1/memory:3530:9
#30 0x66e650 in __release_shared /usr/local/include/c++/v1/memory:3572
#31 0x66e650 in std::__1::shared_ptr<Envoy::Upstream::HostDescription const>::~shared_ptr() /usr/local/include/c++/v1/memory:4508
#32 0x13b20c3 in Envoy::Http::CodecClient::~CodecClient() /source/common/http/codec_client.cc:38:30
#33 0x13b2258 in Envoy::Http::CodecClientProd::~CodecClientProd() /bazel-out/k8-fastbuild/bin/source/common/http/_virtual_includes/codec_client_lib/common/http/codec_client.h:229:7
#34 0x751de6 in operator() /usr/local/include/c++/v1/memory:2325:5
#35 0x751de6 in reset /usr/local/include/c++/v1/memory:2638
#36 0x751de6 in ~unique_ptr /usr/local/include/c++/v1/memory:2592
#37 0x751de6 in destroy /usr/local/include/c++/v1/memory:1867
#38 0x751de6 in __destroy<std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> > > /usr/local/include/c++/v1/memory:1729
#39 0x751de6 in destroy<std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> > > /usr/local/include/c++/v1/memory:1597
#40 0x751de6 in __destruct_at_end /usr/local/include/c++/v1/vector:422
#41 0x751de6 in clear /usr/local/include/c++/v1/vector:365
#42 0x751de6 in std::__1::__vector_base<std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> >, std::__1::allocator<std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> > > >::~__vector_base() /usr/local/include/c++/v1/vector:459
#43 0x74d1aa in ~vector /usr/local/include/c++/v1/vector:551:5
#44 0x74d1aa in Envoy::Event::DispatcherImpl::~DispatcherImpl() /source/common/event/dispatcher_impl.cc:41
#45 0x74d658 in Envoy::Event::DispatcherImpl::~DispatcherImpl() /source/common/event/dispatcher_impl.cc:41:35
#46 0x697b76 in operator() /usr/local/include/c++/v1/memory:2325:5
#47 0x697b76 in reset /usr/local/include/c++/v1/memory:2638
#48 0x697b76 in ~unique_ptr /usr/local/include/c++/v1/memory:2592
#49 0x697b76 in Envoy::Server::InstanceImpl::InstanceImpl(Envoy::Server::Options&, Envoy::Event::TimeSystem&, std::__1::shared_ptr<Envoy::Network::Address::Instance const>, Envoy::TestHooks&, Envoy::Server::HotRestart&, Envoy::Stats::StoreRoot&, Envoy::Thread::BasicLockable&, Envoy::Server::ComponentFactory&, std::__1::unique_ptr<Envoy::Runtime::RandomGenerator, std::__1::default_delete<Envoy::Runtime::RandomGenerator> >&&, Envoy::ThreadLocal::Instance&) /source/server/server.cc:92
#50 0x586026 in make_unique<Envoy::Server::InstanceImpl, testing::NiceMock<Envoy::Server::MockOptions> &, Envoy::Event::TestTimeSystem &, std::__1::shared_ptr<Envoy::Network::Address::Ipv4Instance>, Envoy::DefaultTestHooks &, testing::NiceMock<Envoy::Server::MockHotRestart> &, Envoy::Stats::TestIsolatedStoreImpl &, Envoy::Thread::MutexBasicLockable &, Envoy::Server::TestComponentFactory &, std::__1::unique_ptr<Envoy::Runtime::RandomGeneratorImpl, std::__1::default_delete<Envoy::Runtime::RandomGeneratorImpl> >, Envoy::ThreadLocal::InstanceImpl &> /usr/local/include/c++/v1/memory:3118:32
#51 0x586026 in Envoy::Server::TestOneProtoInput(envoy::config::bootstrap::v2::Bootstrap const&) /test/server/server_fuzz_test.cc:78

Fixes oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11231

Risk Level: Low
Testing: Corpus entry added. It's pretty hard to build regressions for this
kind of destruction ordering, so relying on server_fuzz_test + corpus.

Signed-off-by: Harvey Tuch [email protected]

@htuch
server: reorder SecretManagerImpl field to avoid heap-after-use in te…
f9063ad 
…ardown.

server_fuzz_test indicated the below crash, where the DispatcherImpl teardown
releases some upstream client SSL related objects that then needs
SecretManagerImpl to unregister. Previously, this was already destructed by
time we were in ~DispatcherImpl(), this PR reorders.

    #0 0xc1e826 in size /usr/local/include/c++/v1/__hash_table:809:55
    envoyproxy#1 0xc1e826 in bucket_count /usr/local/include/c++/v1/__hash_table:1197
    envoyproxy#2 0xc1e826 in std::__1::__hash_iterator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, void*>*> std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> > > >::find<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /usr/local/include/c++/v1/__hash_table:2334
    envoyproxy#3 0xc1e278 in unsigned long std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::weak_ptr<Envoy::Secret::TlsCertificateSdsApi> > > >::__erase_unique<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /usr/local/include/c++/v1/__hash_table:2510:20
    envoyproxy#4 0xc1def6 in erase /usr/local/include/c++/v1/unordered_map:1156:59
    envoyproxy#5 0xc1def6 in Envoy::Secret::SecretManagerImpl::DynamicSecretProviders<Envoy::Secret::TlsCertificateSdsApi>::removeDynamicSecretProvider(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /bazel-out/k8-fastbuild/bin/source/common/secret/_virtual_includes/secret_manager_impl_lib/common/secret/secret_manager_impl.h:75
    envoyproxy#6 0x724aa9 in Envoy::Cleanup::~Cleanup() /bazel-out/k8-fastbuild/bin/source/common/common/_virtual_includes/cleanup_lib/common/common/cleanup.h:11:16
    envoyproxy#7 0xc1ff33 in Envoy::Secret::SdsApi::~SdsApi() /bazel-out/k8-fastbuild/bin/source/common/secret/_virtual_includes/sds_api_lib/common/secret/sds_api.h:29:7
    envoyproxy#8 0xc188a0 in __release_shared /usr/local/include/c++/v1/memory:3530:9
    envoyproxy#9 0xc188a0 in __release_shared /usr/local/include/c++/v1/memory:3572
    envoyproxy#10 0xc188a0 in std::__1::shared_ptr<Envoy::Secret::SecretProvider<Envoy::Ssl::TlsCertificateConfig> >::~shared_ptr() /usr/local/include/c++/v1/memory:4508
    envoyproxy#11 0x149c922 in Envoy::Ssl::ContextConfigImpl::~ContextConfigImpl() /source/common/ssl/context_config_impl.cc:117:1
    envoyproxy#12 0x14a0d8f in Envoy::Ssl::ClientContextConfigImpl::~ClientContextConfigImpl() /bazel-out/k8-fastbuild/bin/source/common/ssl/_virtual_includes/context_config_lib/common/ssl/context_config_impl.h:91:7
    envoyproxy#13 0x14a0dc8 in Envoy::Ssl::ClientContextConfigImpl::~ClientContextConfigImpl() /bazel-out/k8-fastbuild/bin/source/common/ssl/_virtual_includes/context_config_lib/common/ssl/context_config_impl.h:91:7
    envoyproxy#14 0x149815b in operator() /usr/local/include/c++/v1/memory:2325:5
    envoyproxy#15 0x149815b in reset /usr/local/include/c++/v1/memory:2638
    envoyproxy#16 0x149815b in ~unique_ptr /usr/local/include/c++/v1/memory:2592
    envoyproxy#17 0x149815b in Envoy::Ssl::ClientSslSocketFactory::~ClientSslSocketFactory() /bazel-out/k8-fastbuild/bin/source/common/ssl/_virtual_includes/ssl_socket_lib/common/ssl/ssl_socket.h:83
    envoyproxy#18 0x14981c8 in Envoy::Ssl::ClientSslSocketFactory::~ClientSslSocketFactory() /bazel-out/k8-fastbuild/bin/source/common/ssl/_virtual_includes/ssl_socket_lib/common/ssl/ssl_socket.h:83:7
    envoyproxy#19 0x1362caf in operator() /usr/local/include/c++/v1/memory:2325:5
    envoyproxy#20 0x1362caf in reset /usr/local/include/c++/v1/memory:2638
    envoyproxy#21 0x1362caf in ~unique_ptr /usr/local/include/c++/v1/memory:2592
    envoyproxy#22 0x1362caf in Envoy::Upstream::ClusterInfoImpl::~ClusterInfoImpl() /bazel-out/k8-fastbuild/bin/source/common/upstream/_virtual_includes/upstream_includes/common/upstream/upstream_impl.h:362
    envoyproxy#23 0x1362d28 in Envoy::Upstream::ClusterInfoImpl::~ClusterInfoImpl() /bazel-out/k8-fastbuild/bin/source/common/upstream/_virtual_includes/upstream_includes/common/upstream/upstream_impl.h:362:7
    envoyproxy#24 0x66e560 in __release_shared /usr/local/include/c++/v1/memory:3530:9
    envoyproxy#25 0x66e560 in __release_shared /usr/local/include/c++/v1/memory:3572
    envoyproxy#26 0x66e560 in std::__1::shared_ptr<Envoy::Upstream::ClusterInfo const>::~shared_ptr() /usr/local/include/c++/v1/memory:4508
    envoyproxy#27 0x13621bf in Envoy::Upstream::HostImpl::~HostImpl() /bazel-out/k8-fastbuild/bin/source/common/upstream/_virtual_includes/upstream_includes/common/upstream/upstream_impl.h:156:7
    envoyproxy#28 0x13621f8 in Envoy::Upstream::HostImpl::~HostImpl() /bazel-out/k8-fastbuild/bin/source/common/upstream/_virtual_includes/upstream_includes/common/upstream/upstream_impl.h:156:7
    envoyproxy#29 0x66e650 in __release_shared /usr/local/include/c++/v1/memory:3530:9
    envoyproxy#30 0x66e650 in __release_shared /usr/local/include/c++/v1/memory:3572
    envoyproxy#31 0x66e650 in std::__1::shared_ptr<Envoy::Upstream::HostDescription const>::~shared_ptr() /usr/local/include/c++/v1/memory:4508
    envoyproxy#32 0x13b20c3 in Envoy::Http::CodecClient::~CodecClient() /source/common/http/codec_client.cc:38:30
    envoyproxy#33 0x13b2258 in Envoy::Http::CodecClientProd::~CodecClientProd() /bazel-out/k8-fastbuild/bin/source/common/http/_virtual_includes/codec_client_lib/common/http/codec_client.h:229:7
    envoyproxy#34 0x751de6 in operator() /usr/local/include/c++/v1/memory:2325:5
    envoyproxy#35 0x751de6 in reset /usr/local/include/c++/v1/memory:2638
    envoyproxy#36 0x751de6 in ~unique_ptr /usr/local/include/c++/v1/memory:2592
    envoyproxy#37 0x751de6 in destroy /usr/local/include/c++/v1/memory:1867
    envoyproxy#38 0x751de6 in __destroy<std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> > > /usr/local/include/c++/v1/memory:1729
    envoyproxy#39 0x751de6 in destroy<std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> > > /usr/local/include/c++/v1/memory:1597
    envoyproxy#40 0x751de6 in __destruct_at_end /usr/local/include/c++/v1/vector:422
    envoyproxy#41 0x751de6 in clear /usr/local/include/c++/v1/vector:365
    envoyproxy#42 0x751de6 in std::__1::__vector_base<std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> >, std::__1::allocator<std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> > > >::~__vector_base() /usr/local/include/c++/v1/vector:459
    envoyproxy#43 0x74d1aa in ~vector /usr/local/include/c++/v1/vector:551:5
    envoyproxy#44 0x74d1aa in Envoy::Event::DispatcherImpl::~DispatcherImpl() /source/common/event/dispatcher_impl.cc:41
    envoyproxy#45 0x74d658 in Envoy::Event::DispatcherImpl::~DispatcherImpl() /source/common/event/dispatcher_impl.cc:41:35
    envoyproxy#46 0x697b76 in operator() /usr/local/include/c++/v1/memory:2325:5
    envoyproxy#47 0x697b76 in reset /usr/local/include/c++/v1/memory:2638
    envoyproxy#48 0x697b76 in ~unique_ptr /usr/local/include/c++/v1/memory:2592
    envoyproxy#49 0x697b76 in Envoy::Server::InstanceImpl::InstanceImpl(Envoy::Server::Options&, Envoy::Event::TimeSystem&, std::__1::shared_ptr<Envoy::Network::Address::Instance const>, Envoy::TestHooks&, Envoy::Server::HotRestart&, Envoy::Stats::StoreRoot&, Envoy::Thread::BasicLockable&, Envoy::Server::ComponentFactory&, std::__1::unique_ptr<Envoy::Runtime::RandomGenerator, std::__1::default_delete<Envoy::Runtime::RandomGenerator> >&&, Envoy::ThreadLocal::Instance&) /source/server/server.cc:92
    envoyproxy#50 0x586026 in make_unique<Envoy::Server::InstanceImpl, testing::NiceMock<Envoy::Server::MockOptions> &, Envoy::Event::TestTimeSystem &, std::__1::shared_ptr<Envoy::Network::Address::Ipv4Instance>, Envoy::DefaultTestHooks &, testing::NiceMock<Envoy::Server::MockHotRestart> &, Envoy::Stats::TestIsolatedStoreImpl &, Envoy::Thread::MutexBasicLockable &, Envoy::Server::TestComponentFactory &, std::__1::unique_ptr<Envoy::Runtime::RandomGeneratorImpl, std::__1::default_delete<Envoy::Runtime::RandomGeneratorImpl> >, Envoy::ThreadLocal::InstanceImpl &> /usr/local/include/c++/v1/memory:3118:32
    envoyproxy#51 0x586026 in Envoy::Server::TestOneProtoInput(envoy::config::bootstrap::v2::Bootstrap const&) /test/server/server_fuzz_test.cc:78

Risk Level: Low
Testing: Corpus entry added. It's pretty hard to build regressions for this
  kind of destruction ordering, so relying on server_fuzz_test + corpus.

Signed-off-by: Harvey Tuch <[email protected]>
@htuch htuch merged commit 871155f into envoyproxy:master Nov 1, 2018
@htuch htuch deleted the sds-use-after-free branch November 1, 2018 21:22
htuch added a commit to htuch/envoy that referenced this pull request Feb 5, 2019
@htuch
config_validation: fix heap-use-after-free.
2b12a78 
Same issue as in envoyproxy#4940, but on the config_validation side.

Risk level: Low
Testing: corpus entry added.

Signed-off-by: Harvey Tuch <[email protected]>
@htuch htuch mentioned this pull request Feb 5, 2019
Merged
htuch added a commit that referenced this pull request Feb 8, 2019
@htuch
config_validation: fix heap-use-after-free. (#5847)
f679d8a 
Same issue as in #4940, but on the config_validation side.

Risk level: Low
Testing: corpus entry added.

Signed-off-by: Harvey Tuch <[email protected]>
htuch added a commit to htuch/envoy that referenced this pull request Feb 21, 2019
@htuch
config_validation/server: fix initialization order redux.
028da2f 
Another heap-user-after-free, this time we were missing a fix that had been applied to server.h but
not config_validation/server.h (envoyproxy#4940). While working on this, attempted to make fully consistent and as
simple/clear as possible the constraints on member ordering.

This PR is in the tradition (!) of envoyproxy#5847, envoyproxy#4940, envoyproxy#4937. I think long-term we might want to think of
more dynamic and explicit declaration of ordering constraints, it's evidently pretty fragile. Also,
the lack of single-source-of-truth and duplication across prod and config server code bites again.

Fixes oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=13228.

Risk level: Low
Testing: Corpus entry added.

Signed-off-by: Harvey Tuch <[email protected]>
@htuch htuch mentioned this pull request Feb 21, 2019
Merged
htuch added a commit that referenced this pull request Feb 21, 2019
@htuch
config_validation/server: fix initialization order redux. (#6023)
6ff8603 
Another heap-user-after-free, this time we were missing a fix that had been applied to server.h but
not config_validation/server.h (#4940). While working on this, attempted to make fully consistent and as
simple/clear as possible the constraints on member ordering.

This PR is in the tradition (!) of #5847, #4940, #4937. I think long-term we might want to think of
more dynamic and explicit declaration of ordering constraints, it's evidently pretty fragile. Also,
the lack of single-source-of-truth and duplication across prod and config server code bites again.

Fixes oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=13228.

Risk level: Low
Testing: Corpus entry added.

Signed-off-by: Harvey Tuch <[email protected]>
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 5, 2019
@htuch @fredlas
config_validation: fix heap-use-after-free. (envoyproxy#5847)
b767534 
Same issue as in envoyproxy#4940, but on the config_validation side.

Risk level: Low
Testing: corpus entry added.

Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Fred Douglas <[email protected]>
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 5, 2019
@htuch @fredlas
config_validation/server: fix initialization order redux. (envoyproxy…
c23489c 
…#6023)

Another heap-user-after-free, this time we were missing a fix that had been applied to server.h but
not config_validation/server.h (envoyproxy#4940). While working on this, attempted to make fully consistent and as
simple/clear as possible the constraints on member ordering.

This PR is in the tradition (!) of envoyproxy#5847, envoyproxy#4940, envoyproxy#4937. I think long-term we might want to think of
more dynamic and explicit declaration of ordering constraints, it's evidently pretty fragile. Also,
the lack of single-source-of-truth and duplication across prod and config server code bites again.

Fixes oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=13228.

Risk level: Low
Testing: Corpus entry added.

Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Fred Douglas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@JimmyCYJ JimmyCYJ Awaiting requested review from JimmyCYJ

Assignees

@mattklein123 mattklein123

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@htuch @mattklein123