Stiffstream / restinio

Cross-platform, efficient, customizable, and robust asynchronous HTTP(S)/WebSocket server C++ library with the right balance between performance and ease of use
Other
1.16k stars 93 forks source link

Possible corruption in response_builder_t #155

Closed i-am-justin-dick closed 2 years ago

i-am-justin-dick commented 2 years ago

I came across a weird segfault in my application today; fortunately I was running in gdb at the time so I can get some info. Here's where the error happened (v.0.6.14) :

#0  restinio::response_builder_t<restinio::restinio_controlled_output_t>::done(std::function<void (std::error_code const&)>) (
    this=0xabbf2e28, wscb=...) at ../restinio/dev/restinio/message_builders.hpp:333

For reference, this is the code:

            auto conn = std::move( m_connection );

            conn->write_response_parts(
               m_request_id,
               response_output_flags,
               std::move( wg ) );

As you can see from the gdb output below, the issue seems to be that conn or m_connection were corrupted somehow. The value for conn is suspicious in that (besides being outside the application's segments), the value 0x32203a69 seems to be part of an ASCII string ("2 :i"), possibly indicating a buffer overrun somewhere. To me, the values of *this look fine with expected data. The values in wg seem to be mainly garbage, but that may be expected given that there is no body for this response? Anyway, any thoughts or ideas of how to debug this would be greatly appreciated.

(gdb) p wg
$7 = {m_items = std::vector of length 1, capacity 2 = {{m_write_type = restinio::writable_item_type_t::trivial_write_operation, 
      m_storage = {__data = "\350\212#\000\230\b\274\253\337\000\000\000\371\000\000\000channel '24.", __align = {<No data fields>}}}}, 
  m_status_line_size = 22, 
  m_after_write_notificator = {<std::_Maybe_unary_or_binary_function<void, std::error_code const&>> = {<std::unary_function<std::error_code const&, void>> = {<No data fields>}, <No data fields>}, <std::_Function_base> = {static _M_max_size = 8, static _M_max_align = 4, 
      _M_functor = {_M_unused = {_M_object = 0xabbc07c0, _M_const_object = 0xabbc07c0, _M_function_pointer = 0xabbc07c0, 
          _M_member_pointer = (void (std::_Undefined_class::*)(std::_Undefined_class * const)) 0xabbc07c0, this adjustment -689452006}, 
        _M_pod_data = "\300\a\274\253\064\230ϭ"}, _M_manager = 0x0}, _M_invoker = 0xadcf98a8}}
(gdb) p conn
$14 = <error reading variable: Cannot access memory at address 0x32203a69>
(gdb) p *this
$15 = {<restinio::base_response_builder_t<restinio::response_builder_t<restinio::restinio_controlled_output_t> >> = {
    _vptr.base_response_builder_t = 0x238ac8 <vtable for restinio::response_builder_t<restinio::restinio_controlled_output_t>+8>, 
    m_header = {<restinio::http_header_common_t> = {<restinio::http_header_fields_t> = {
          _vptr.http_header_fields_t = 0x238b34 <vtable for restinio::http_response_header_t+8>, 
          m_fields = std::vector of length 5, capacity 8 = {{m_name = "Server", m_value = "Aerial", 
              m_field_id = restinio::http_field_t::server}, {m_name = "Date", m_value = "Tue, 15 Mar 2022 16:02:06 GMT", 
              m_field_id = restinio::http_field_t::date}, {m_name = "Access-Control-Allow-Origin", m_value = "*", m_field_id = -102}, {
              m_name = "Content-Type", m_value = "application/json; charset=utf-8", m_field_id = restinio::http_field_t::content_type}, {
              m_name = "Location", m_value = "/24.1/24.1.m3u8", m_field_id = restinio::http_field_t::location}}}, m_http_major = 1, 
        m_http_minor = 1, m_content_length = 0, m_http_connection_header_field_value = restinio::http_connection_header_t::close}, 
      m_status_line = {m_status_code = {m_status_code = 303}, m_reason_phrase = "See Other"}}, 
    m_connection = std::shared_ptr<restinio::impl::connection_base_t> (empty) = {get() = 0x0}, m_request_id = 1852403041}, 
  m_body_size = 0, m_response_parts = std::vector of length 0, capacity 0}
eao197 commented 2 years ago

Hi!

Thanks for reporting the issue.

Could you provide a stack trace from gdb?

i-am-justin-dick commented 2 years ago

Sure, although it's a bit hairy ...

(gdb) bt
#0  restinio::response_builder_t<restinio::restinio_controlled_output_t>::done(std::function<void (std::error_code const&)>) (
    this=0xabbf2e28, wscb=...) at ../restinio/dev/restinio/message_builders.hpp:333
#1  0x000b278c in Endpoints::initializeGetStreamEndpoint<std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > > >(std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > >&)::{lambda(auto:1, auto:2 const&)#1}::operator()<std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t>(std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t const&) const::{lambda(ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)#1}::operator()(restinio::router::route_params_t, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) const (this=0xb2dbaab0, responseCode=ResponseCode::SeeOther, response=...)
    at src/endpoints.cpp:169
#2  0x000d056c in std::__invoke_impl<void, Endpoints::initializeGetStreamEndpoint<std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > > >(std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > >&)::{lambda(auto:1, auto:2 const&)#1}::operator()<std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t>(std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t const&) const::{lambda(ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)#1}&, restinio::router::route_params_t, std::allocator<char> >(std::__invoke_other, Endpoints::initializeGetStreamEndpoint<std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > > >(std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > >&)::{lambda(auto:1, auto:2 const&)#1}::operator()<std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t>(std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t const&) const::{lambda(ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)#1}&, restinio::router::route_params_t&&, std::allocator<char>&&) (__f=...)
    at /usr/local/include/c++/10.1.0/bits/invoke.h:60
#3  0x000cb398 in std::__invoke_r<void, Endpoints::initializeGetStreamEndpoint<std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > > >(std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > >&)::{lambda(auto:1, auto:2 const&)#1}::operator()<std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t>(std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t const&) const::{lambda(ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)#1}&, restinio::router::route_params_t, std::allocator<char> >(Endpoints::initializeGetStreamEndpoint<std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > > >(std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > >&)::{lambda(auto:1, auto:2 const&)#1}::operator()<std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t>(std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t const&) const::{lambda(ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)#1}&, (Endpoints::initializeGetStreamEndpoint<std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > > >(std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > >&)::{lambda(auto:1, auto:2 const&)#1}::operator()<std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t>(std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t const&) const::{lambda(ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)#1}&)...) (__fn=...)
    at /usr/local/include/c++/10.1.0/bits/invoke.h:110
#4  0x000c4fc4 in std::_Function_handler<void (ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&), Endpoints::initializeGetStreamEndpoint<std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > > >(std::unique_ptr<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t>, std::default_delete<restinio::router::generic_express_router_t<restinio::router::std_regex_engine_t, restinio::no_extra_data_factory_t> > >&)::{lambda(auto:1, auto:2 const&)#1}::operator()<std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t>(std::shared_ptr<restinio::generic_request_t<restinio::no_extra_data_factory_t::data_t> >, restinio::router::route_params_t const&) const::{lambda(ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)#1}>::_M_invoke(std::_Any_data const&, ResponseCode&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&) (__functor=..., __args#0=@0xadcf9a48: ResponseCode::SeeOther, __args#1=...)
    at /usr/local/include/c++/10.1.0/bits/std_function.h:291
#5  0x00069cf4 in std::function<void (ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)>::operator()(ResponseCode, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&) const (this=0xabbb66c0, 
    __args#0=ResponseCode::SeeOther, __args#1=...) at /usr/local/include/c++/10.1.0/bits/std_function.h:622
#6  0x0018783c in operator()<std::any> (__closure=0xb3701508, wrapped_m3u8=std::any containing std::string = {...})
    at src/stream_services.cpp:45
#7  0x0018843c in std::__invoke_impl<bool, SendStreamDataService::run()::<lambda(auto:20)>&, std::any>(std::__invoke_other, struct {...} &) (__f=...) at /usr/local/include/c++/10.1.0/bits/invoke.h:60
#8  0x001882bc in std::__invoke_r<bool, SendStreamDataService::run()::<lambda(auto:20)>&, std::any>(struct {...} &) (__fn=...)
    at /usr/local/include/c++/10.1.0/bits/invoke.h:113
#9  0x00188154 in std::_Function_handler<bool(std::any), SendStreamDataService::run()::<lambda(auto:20)> >::_M_invoke(const std::_Any_data &, std::any &&) (__functor=..., __args#0=...) at /usr/local/include/c++/10.1.0/bits/std_function.h:291
#10 0x0013c170 in std::function<bool (std::any)>::operator()(std::any) const (this=0xb3701860, __args#0=std::any [no contained value])
    at /usr/local/include/c++/10.1.0/bits/std_function.h:622
#11 0x0013b640 in InternalEvents::publish (this=0x23bad8 <InternalEvents::instance()::i>, topic=..., 
    value=std::any containing std::string = {...}) at src/internal_events.cpp:71
#12 0x0013e4fc in LiveStream::handleData (this=0x572780, data=...) at src/live_stream.cpp:82
...
eao197 commented 2 years ago

There are two things that looks very suspicious:

The first one is the content of wg:

{m_items = std::vector of length 1, capacity 2 = {{m_write_type = restinio::writable_item_type_t::trivial_write_operation, 
      m_storage = {__data = "\350\212#\000\230\b\274\253\337\000\000\000\371\000\000\000channel '24.", __align = {<No data fields>}}}},

especially the content of __data. It should contain an instance of std::string, so the first several bytes are something like std::string::size, std::string::capacity or std::string::data. But "channel '24." is not expected here. I think it should be "HTTP/1.1 303" or something like that.

The another is the inability to get the content of conn variable:

(gdb) p conn
$14 = <error reading variable: Cannot access memory at address 0x32203a69>

conn is just a std::shared_ptr on the stack. So the pointer value inside that variable has to be available for gdb (and that pointer can hold invalid value). But gdb can't show the content of conn that should be on the stack.

It looks like you're in the situation of stack overflow and conn is out of the valid stack address.

But I can't say more at the moment because don't know anything about your app and your usage of RESTinio.

i-am-justin-dick commented 2 years ago

OK, this was 100% on my end. I had just switched to using std::any as the parameter type that drove the lambda that created the response, and given that the "channel '24" is part of a log line that happened just before the call to the response_builder, I tried switching to a different data type to see if std::any was somehow causing the stack corruption. I got lucky, and the stack remained unchanged enough to get the following:

terminate called after throwing an instance of 'restinio::exception_t'
  what():  connection already moved

This was enough to put me down a path that resulted in the realization that there was a way the lambda that created this response could be called after the thread that owned the lambda had exited.

So, sorry to waste your time - I kind of doubted that the issue was in restinio, but your familiarity with the restinio code base to eliminate that as the culprit and your analysis of the problem were extremely helpful.

Please let me know if there's ever a way to return the favor.

eao197 commented 2 years ago

Hi! I glad that you found the error.

Please let me know if there's ever a way to return the favor.

No need to worry, it's a pleasure to know that our product is used and found helpful.