DNS-OARC / flamethrower

a DNS performance and functional testing utility supporting UDP, TCP, DoT and DoH
Apache License 2.0
318 stars 37 forks source link

DoH test ended with SIGSEGV #76

Closed pemensik closed 2 years ago

pemensik commented 3 years ago

Hi, I just tested 0.11.0 build with backported http_parser usage to ensure it is no broken.

However, it received few crashes. Now always and reliable. I did just cherry pick current latest commit, it might be fixed by few missing commits already.

Core was generated by `./flame -P doh https://cloudflare-dns.com/dns-query'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007f13b3e9fd36 in free_streams (entry=0x55a6acf31990, ptr=ptr@entry=0x55a6acfe28f0)
    at /usr/src/debug/nghttp2-1.43.0-2.fc34.x86_64/lib/nghttp2_session.c:680
680   if (item && !item->queued && item != session->aob.item) {
Missing separate debuginfos, use: dnf debuginfo-install cyrus-sasl-lib-2.1.27-8.fc34.x86_64 docopt-cpp-0.6.3-1.fc35.x86_64 glibc-2.33-20.fc34.x86_64 gmp-6.2.0-6.fc34.x86_64 gnutls-3.7.2-1.fc34.x86_64 http-parser-2.9.4-4.fc34.x86_64 keyutils-libs-1.6.1-2.fc34.x86_64 krb5-libs-1.19.1-14.fc34.x86_64 ldns-1.7.1-4.fc34.x86_64 libcom_err-1.45.6-5.fc34.x86_64 libffi-3.1-28.fc34.x86_64 libgcc-11.2.1-1.fc34.x86_64 libstdc++-11.2.1-1.fc34.x86_64 libtasn1-4.16.0-4.fc34.x86_64 libunistring-0.9.10-10.fc34.x86_64 libuv-1.41.0-1.fc34.x86_64 nettle-3.7.3-1.fc34.x86_64 opencryptoki-libs-3.16.0-1.fc34.x86_64 openldap-2.4.57-5.fc34.x86_64 opensc-0.21.0-4.fc34.x86_64 openssl-libs-1.1.1k-1.fc34.x86_64 p11-kit-trust-0.23.22-3.fc34.x86_64 pcre2-10.36-4.fc34.x86_64 systemd-libs-248.6-1.fc34.x86_64 zlib-1.2.11-26.fc34.x86_64
(gdb) bt
#0  0x00007f13b3e9fd36 in free_streams (entry=0x55a6acf31990, ptr=ptr@entry=0x55a6acfe28f0)
    at /usr/src/debug/nghttp2-1.43.0-2.fc34.x86_64/lib/nghttp2_session.c:680
#1  0x00007f13b3ea26c8 in nghttp2_map_each_free (func=<optimized out>, ptr=0x55a6acfe28f0, map=0x55a6acfe28f0)
    at /usr/src/debug/nghttp2-1.43.0-2.fc34.x86_64/lib/nghttp2_map.c:79
#2  nghttp2_session_del (session=0x55a6acfe28f0)
    at /usr/src/debug/nghttp2-1.43.0-2.fc34.x86_64/lib/nghttp2_session.c:755
#3  0x00007f13b4651d8a in HTTPSSession::~HTTPSSession (this=<optimized out>, this=<optimized out>)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/flame/httpssession.cpp:42
#4  0x00007f13b461bc4a in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x55a6acf5d7f0)
    at /usr/include/c++/11/bits/shared_ptr_base.h:168
#5  0x00007f13b46469dd in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, 
    this=<optimized out>) at /usr/include/c++/11/bits/shared_ptr_base.h:705
#6  std::__shared_ptr<TCPSession, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, 
    this=<optimized out>) at /usr/include/c++/11/bits/shared_ptr_base.h:1154
#7  std::__shared_ptr<TCPSession, (__gnu_cxx::_Lock_policy)2>::reset (this=<optimized out>)
    at /usr/include/c++/11/bits/shared_ptr_base.h:1272
#8  operator() (event=..., h=..., __closure=0x55a6ad20a508)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/flame/trafgen.cpp:194
#9  std::__invoke_impl<void, TrafGen::start_tcp_session()::<lambda(uvw::CloseEvent&, uvw::TcpHandle&)>&, uvw::CloseEvent&, uvw::TcpHandle&> (__f=...) at /usr/include/c++/11/bits/invoke.h:61
#10 std::__invoke_r<void, TrafGen::start_tcp_session()::<lambda(uvw::CloseEvent&, uvw::TcpHandle&)>&, uvw::CloseEvent&, uvw::TcpHandle&> (__fn=...) at /usr/include/c++/11/bits/invoke.h:111
#11 std::_Function_handler<void(uvw::CloseEvent&, uvw::TcpHandle&), TrafGen::start_tcp_session()::<lambda(uvw::CloseEvent&, uvw::TcpHandle&)> >::_M_invoke(const std::_Any_data &, uvw::CloseEvent &, uvw::TcpHandle &) (__functor=..., 
    __args#0=..., __args#1=...) at /usr/include/c++/11/bits/std_function.h:291
#12 0x00007f13b464024b in std::function<void (uvw::TimerEvent&, uvw::TimerHandle&)>::operator()(uvw::TimerEvent&, uvw::TimerHandle&) const (__args#1=..., __args#0=..., this=<optimized out>) at /usr/include/c++/11/bits/std_function.h:560
#13 uvw::Emitter<uvw::TcpHandle>::Handler<uvw::CloseEvent>::publish(uvw::CloseEvent, uvw::TcpHandle&)::{lambda(auto:1&&)#1}::operator()<std::pair<bool, std::function<void (uvw::CloseEvent&, uvw::TcpHandle&)> >&>(std::pair<bool, std::function<void (uvw::CloseEvent&, uvw::TcpHandle&)> >&) const (__closure=<optimized out>, __closure=<optimized out>, element=...)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/3rd/uvw/uvw/emitter.hpp:142
#14 uvw::Emitter<uvw::TcpHandle>::Handler<uvw::CloseEvent>::publish(uvw::CloseEvent, uvw::TcpHandle&)::{lambda(auto:1&&)#1}::operator()<std::pair<bool, std::function<void (uvw::CloseEvent&, uvw::TcpHandle&)> >&>(std::pair<bool, std::function<void (uvw::CloseEvent&, uvw::TcpHandle&)> >&) const (element=..., __closure=<synthetic pointer>)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/3rd/uvw/uvw/emitter.hpp:141
#15 std::for_each<std::reverse_iterator<std::_List_iterator<std::pair<bool, std::function<void (uvw::CloseEvent&, uvw::TcpHandle&)> > > >, uvw::Emitter<uvw::TcpHandle>::Handler<uvw::CloseEvent>::publish(uvw::CloseEvent, uvw::TcpHandle&)::{lambda(auto:1&&)#1}>(std::reverse_iterator<std::_List_iterator<std::pair<bool, std::function<void (uvw::CloseEvent&, uvw::TcpHandle&)> > > >, std::reverse_iterator<std::_List_iterator<std::pair<bool, std::function<void (uvw::CloseEvent&, uvw::Tc--Type <RET> for more, q to quit, c to continue without paging--
pHandle&)> > > >, uvw::Emitter<uvw::TcpHandle>::Handler<uvw::CloseEvent>::publish(uvw::CloseEvent, uvw::TcpHandle&)::{lambda(auto:1&&)#1}) (__f=..., __last=..., __first=...) at /usr/include/c++/11/bits/stl_algo.h:3820
#16 uvw::Emitter<uvw::TcpHandle>::Handler<uvw::CloseEvent>::publish (ref=..., event=..., this=0x55a6ad0c3840)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/3rd/uvw/uvw/emitter.hpp:147
#17 uvw::Emitter<uvw::TcpHandle>::publish<uvw::CloseEvent> (event=..., this=0x55a6acf59c58)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/3rd/uvw/uvw/emitter.hpp:190
#18 uvw::Handle<uvw::TcpHandle, uv_tcp_s>::closeCallback (handle=<optimized out>)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/3rd/uvw/uvw/handle.hpp:36
#19 0x00007f13b45af9b5 in uv_run () from /lib64/libuv.so.1
#20 0x000055a6abff6452 in uvw::Loop::run<(uvw::details::UVRunMode)0> (this=<optimized out>)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/3rd/uvw/uvw/loop.hpp:307
#21 main (argc=<optimized out>, argv=<optimized out>)
    at /home/pemensik/fedora/flamethrower/flamethrower-0.11.0/flame/main.cpp:509

Is this one of issues already fixed?

weyrick commented 3 years ago

@pemensik unfortunately, I'm not sure. This problem does not show up on master branch?

pemensik commented 3 years ago

No, I were unable to reproduce it on commit c155af61c524df5d04060f5a692cdb1da7351070 on master branch.

jroyalty commented 2 years ago

I came across this bug today and found the cause of a DoH-related crash, which I think is the same.

The crash in HTTPSSession::~HTTPSSession() is due the _current_session pointer (which is passed into nghttp2_session_del) being uninitialized. That's because a) it's not nullified by the ctor, and b) init_nghttp2() has not been called by the time the object is destroyed.

In my testing, this condition arises when a connection to the DoH endpoint cannot be established. This small patch fixes the uninitialized pointer which fixes the crash (because nghttp2_session_del can deal with NULL) but it's likely worth looking at the logic flow of HTTP handling as well. I haven't had the time to do so, but figured I'd drop these finding in the meantime.

diff --git a/flame/httpssession.cpp b/flame/httpssession.cpp
index 241751e..22605b2 100644
--- a/flame/httpssession.cpp
+++ b/flame/httpssession.cpp
@@ -32,6 +32,7 @@ HTTPSSession::HTTPSSession(std::shared_ptr<uvw::TcpHandle> handle,
     , _handshake_error{handshake_error_handler}
     , _target{target}
     , _method{method}
+    , _current_session{nullptr}
 {
 }