From e9660c35580c5953d65cad11fbc3633fb40085a1 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Wed, 8 Apr 2015 13:43:57 +0900
Subject: [PATCH] nghttpx: Fix heap-use-after-free

The bug was introduced by 8c3b379b66cfa31df1a4ccb29225e5aa44b70d54.
---
 src/shrpx_client_handler.cc |  2 +-
 src/shrpx_worker.cc         | 25 ++++++++++++++++++++++++-
 src/shrpx_worker.h          |  2 ++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc
index 0a6457a4..48037a54 100644
--- a/src/shrpx_client_handler.cc
+++ b/src/shrpx_client_handler.cc
@@ -405,7 +405,7 @@ ClientHandler::~ClientHandler() {
   --worker_stat->num_connections;
 
   if (worker_stat->num_connections == 0) {
-    worker_->get_mcpool()->clear();
+    worker_->schedule_clear_mcpool();
   }
 
   ev_timer_stop(conn_.loop, &reneg_shutdown_timer_);
diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc
index e84d456f..bbae73b1 100644
--- a/src/shrpx_worker.cc
+++ b/src/shrpx_worker.cc
@@ -46,6 +46,16 @@ void eventcb(struct ev_loop *loop, ev_async *w, int revents) {
 }
 } // namespace
 
+namespace {
+void mcpool_clear_cb(struct ev_loop *loop, ev_timer *w, int revents) {
+  auto worker = static_cast<Worker *>(w->data);
+  if (worker->get_worker_stat()->num_connections != 0) {
+    return;
+  }
+  worker->get_mcpool()->clear();
+}
+} // namespace
+
 Worker::Worker(struct ev_loop *loop, SSL_CTX *sv_ssl_ctx, SSL_CTX *cl_ssl_ctx,
                ssl::CertLookupTree *cert_tree,
                const std::shared_ptr<TicketKeys> &ticket_keys)
@@ -57,6 +67,9 @@ Worker::Worker(struct ev_loop *loop, SSL_CTX *sv_ssl_ctx, SSL_CTX *cl_ssl_ctx,
   w_.data = this;
   ev_async_start(loop_, &w_);
 
+  ev_timer_init(&mcpool_clear_timer_, mcpool_clear_cb, 0., 0.);
+  mcpool_clear_timer_.data = this;
+
   if (get_config()->downstream_proto == PROTO_HTTP2) {
     auto n = get_config()->http2_downstream_connections_per_worker;
     for (; n > 0; --n) {
@@ -66,7 +79,17 @@ Worker::Worker(struct ev_loop *loop, SSL_CTX *sv_ssl_ctx, SSL_CTX *cl_ssl_ctx,
   }
 }
 
-Worker::~Worker() { ev_async_stop(loop_, &w_); }
+Worker::~Worker() {
+  ev_async_stop(loop_, &w_);
+  ev_timer_stop(loop_, &mcpool_clear_timer_);
+}
+
+void Worker::schedule_clear_mcpool() {
+  // libev manual says: "If the watcher is already active nothing will
+  // happen."  Since we don't change any timeout here, we don't have
+  // to worry about querying ev_is_active.
+  ev_timer_start(loop_, &mcpool_clear_timer_);
+}
 
 void Worker::wait() {
 #ifndef NOTHREADS
diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h
index 43bb729b..47213710 100644
--- a/src/shrpx_worker.h
+++ b/src/shrpx_worker.h
@@ -108,6 +108,7 @@ public:
   bool get_graceful_shutdown() const;
 
   MemchunkPool *get_mcpool();
+  void schedule_clear_mcpool();
 
 private:
   std::vector<std::unique_ptr<Http2Session>> http2sessions_;
@@ -118,6 +119,7 @@ private:
   std::mutex m_;
   std::deque<WorkerEvent> q_;
   ev_async w_;
+  ev_timer mcpool_clear_timer_;
   MemchunkPool mcpool_;
   DownstreamConnectionPool dconn_pool_;
   WorkerStat worker_stat_;