From b38b233aa60e0d992a9457dde3abc4fef4d075ec Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 20 Dec 2013 23:28:54 +0900 Subject: [PATCH] nghttpx: Check failure of evbuffer_* and bufferevent_* functions --- src/shrpx_client_handler.cc | 6 +++++- src/shrpx_downstream.cc | 2 +- src/shrpx_http2_downstream_connection.cc | 4 ++++ src/shrpx_http2_session.cc | 21 ++++++++++++++++----- src/shrpx_http2_upstream.cc | 7 +++++-- src/shrpx_http_downstream_connection.cc | 15 ++++++++++++--- src/shrpx_https_upstream.cc | 10 ++++++++-- src/shrpx_spdy_upstream.cc | 7 +++++-- src/shrpx_thread_event_receiver.cc | 4 ++++ 9 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index bd640f99..d6a1ba5b 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -221,7 +221,11 @@ ClientHandler::ClientHandler(bufferevent *bev, int fd, SSL *ssl, fd_(fd), should_close_after_write_(false) { - bufferevent_set_rate_limit(bev_, get_config()->rate_limit_cfg); + int rv; + rv = bufferevent_set_rate_limit(bev_, get_config()->rate_limit_cfg); + if(rv == -1) { + CLOG(FATAL, this) << "bufferevent_set_rate_limit() failed"; + } bufferevent_enable(bev_, EV_READ | EV_WRITE); bufferevent_setwatermark(bev_, EV_READ, 0, SHRPX_READ_WARTER_MARK); set_upstream_timeouts(&get_config()->upstream_read_timeout, diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index b7d3bb00..1cb81ee0 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -593,7 +593,7 @@ int Downstream::init_response_body_buf() { if(!response_body_buf_) { response_body_buf_ = evbuffer_new(); - if(response_body_buf_ == 0) { + if(response_body_buf_ == nullptr) { DIE(); } evbuffer_setcb(response_body_buf_, body_buf_cb, this); diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index cb0f2699..9dfc90fa 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -175,6 +175,10 @@ ssize_t http2_data_read_callback(nghttp2_session *session, int nread = 0; for(;;) { nread = evbuffer_remove(body, buf, length); + if(nread == -1) { + DCLOG(FATAL, dconn) << "evbuffer_remove() failed"; + return NGHTTP2_ERR_CALLBACK_FAILURE; + } if(nread == 0) { if(downstream->get_request_state() == Downstream::MSG_COMPLETE) { if(!downstream->get_upgrade_request() || diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 0b17132a..10cebe27 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -538,7 +538,10 @@ int Http2Session::on_read_proxy() reinterpret_cast(mem), evbuffer_get_length(input)); - evbuffer_drain(input, nread); + if(evbuffer_drain(input, nread) != 0) { + SSLOG(FATAL, this) << "evbuffer_drain() failed"; + return -1; + } auto htperr = HTTP_PARSER_ERRNO(proxy_htp_.get()); if(htperr == HPE_OK) { return 0; @@ -1182,8 +1185,12 @@ int Http2Session::on_connect() } } - bufferevent_write(bev_, NGHTTP2_CLIENT_CONNECTION_HEADER, - NGHTTP2_CLIENT_CONNECTION_HEADER_LEN); + rv = bufferevent_write(bev_, NGHTTP2_CLIENT_CONNECTION_HEADER, + NGHTTP2_CLIENT_CONNECTION_HEADER_LEN); + if(rv != 0) { + SSLOG(FATAL, this) << "bufferevent_write() failed"; + return -1; + } rv = send(); if(rv != 0) { @@ -1252,14 +1259,18 @@ int Http2Session::send() void Http2Session::clear_notify() { auto input = bufferevent_get_output(rdbev_); - evbuffer_drain(input, evbuffer_get_length(input)); + if(evbuffer_drain(input, evbuffer_get_length(input)) != 0) { + SSLOG(FATAL, this) << "evbuffer_drain() failed"; + } notified_ = false; } void Http2Session::notify() { if(!notified_) { - bufferevent_write(wrbev_, "1", 1); + if(bufferevent_write(wrbev_, "1", 1) != 0) { + SSLOG(FATAL, this) << "bufferevent_write failed"; + } notified_ = true; } } diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 6c9d8105..28706c55 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -832,17 +832,20 @@ ssize_t downstream_data_read_callback(nghttp2_session *session, void *user_data) { auto downstream = reinterpret_cast(source->ptr); + auto upstream = reinterpret_cast(downstream->get_upstream()); auto body = downstream->get_response_body_buf(); assert(body); int nread = evbuffer_remove(body, buf, length); + if(nread == -1) { + ULOG(FATAL, upstream) << "evbuffer_remove() failed"; + return NGHTTP2_ERR_CALLBACK_FAILURE; + } if(nread == 0 && downstream->get_response_state() == Downstream::MSG_COMPLETE) { if(!downstream->get_upgraded()) { *eof = 1; } else { // For tunneling, issue RST_STREAM to finish the stream. - auto upstream = reinterpret_cast - (downstream->get_upstream()); if(LOG_ENABLED(INFO)) { ULOG(INFO, upstream) << "RST_STREAM to tunneled stream stream_id=" << stream_id; diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index f2641f52..446d9640 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -484,14 +484,23 @@ int HttpDownstreamConnection::on_read() int rv; rv = downstream_->get_upstream()->on_downstream_body (downstream_, reinterpret_cast(mem), inputlen); - evbuffer_drain(input, inputlen); - return rv; + if(rv != 0) { + return rv; + } + if(evbuffer_drain(input, inputlen) != 0) { + DCLOG(FATAL, this) << "evbuffer_drain() failed"; + return -1; + } + return 0; } size_t nread = http_parser_execute(&response_htp_, &htp_hooks, reinterpret_cast(mem), inputlen); - evbuffer_drain(input, nread); + if(evbuffer_drain(input, nread) != 0) { + DCLOG(FATAL, this) << "evbuffer_drain() failed"; + return -1; + } auto htperr = HTTP_PARSER_ERRNO(&response_htp_); if(htperr == HPE_OK) { return 0; diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 209abc5c..19006b54 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -262,10 +262,13 @@ int HttpsUpstream::on_read() if(downstream && downstream->get_upgraded()) { int rv = downstream->push_upload_data_chunk (reinterpret_cast(mem), inputlen); - evbuffer_drain(input, inputlen); if(rv != 0) { return -1; } + if(evbuffer_drain(input, inputlen) != 0) { + ULOG(FATAL, this) << "evbuffer_drain() failed"; + return -1; + } if(downstream->get_output_buffer_full()) { if(LOG_ENABLED(INFO)) { ULOG(INFO, this) << "Downstream output buffer is full"; @@ -278,7 +281,10 @@ int HttpsUpstream::on_read() size_t nread = http_parser_execute(&htp_, &htp_hooks, reinterpret_cast(mem), inputlen); - evbuffer_drain(input, nread); + if(evbuffer_drain(input, nread) != 0) { + ULOG(FATAL, this) << "evbuffer_drain() failed"; + return -1; + } // Well, actually header length + some body bytes current_header_length_ += nread; // Get downstream again because it may be initialized in http parser diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index aca23332..86fbb990 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -730,17 +730,20 @@ ssize_t spdy_data_read_callback(spdylay_session *session, void *user_data) { Downstream *downstream = reinterpret_cast(source->ptr); + auto upstream = reinterpret_cast(downstream->get_upstream()); evbuffer *body = downstream->get_response_body_buf(); assert(body); int nread = evbuffer_remove(body, buf, length); + if(nread == -1) { + ULOG(FATAL, upstream) << "evbuffer_remove() failed"; + return SPDYLAY_ERR_CALLBACK_FAILURE; + } if(nread == 0 && downstream->get_response_state() == Downstream::MSG_COMPLETE) { if(!downstream->get_upgraded()) { *eof = 1; } else { // For tunneling, issue RST_STREAM to finish the stream. - SpdyUpstream *upstream; - upstream = reinterpret_cast(downstream->get_upstream()); if(LOG_ENABLED(INFO)) { ULOG(INFO, upstream) << "RST_STREAM to tunneled stream stream_id=" << stream_id; diff --git a/src/shrpx_thread_event_receiver.cc b/src/shrpx_thread_event_receiver.cc index 90ef22db..516413fb 100644 --- a/src/shrpx_thread_event_receiver.cc +++ b/src/shrpx_thread_event_receiver.cc @@ -48,6 +48,10 @@ void ThreadEventReceiver::on_read(bufferevent *bev) while(evbuffer_get_length(input) >= sizeof(WorkerEvent)) { WorkerEvent wev; int nread = evbuffer_remove(input, &wev, sizeof(wev)); + if(nread == -1) { + TLOG(FATAL, this) << "evbuffer_remove() failed"; + continue; + } if(nread != sizeof(wev)) { TLOG(FATAL, this) << "evbuffer_remove() removed fewer bytes. Expected:" << sizeof(wev) << " Actual:" << nread;