v1.5.7 from Apache svn

icing · Jun 7, 2016 · 60ce3f1 · 60ce3f1
1 parent cb78bb4
commit 60ce3f1
Show file tree

Hide file tree

Showing 12 changed files with 65 additions and 65 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,15 @@
+v1.5.7
+--------------------------------------------------------------------------------
+ * fixed read after free on certain conditions when clients abort connections
+ * fixed connection shutdown when stream tasks block on reads
+
+v1.5.6
+--------------------------------------------------------------------------------
+ * fixed possible null pointer reference in new bucket beams during read on an 
+   aborted beam
+ * improved event handling on master connection (idle/submit/wait) to increase 
+   performance
+
 v1.5.5
 -------------------------------------------------------------------------------- 
  * Fix async write issue that sometimes led to selection of wrong timeout

diff --git a/configure.ac b/configure.ac
@@ -14,7 +14,7 @@
 #
 
 AC_PREREQ([2.69])
-AC_INIT([mod_http2], [1.5.6], [[email protected]])
+AC_INIT([mod_http2], [1.5.7], [[email protected]])
 
 LT_PREREQ([2.2.6])
 LT_INIT()

diff --git a/mod_http2/h2_bucket_eos.c b/mod_http2/h2_bucket_eos.c
@@ -38,10 +38,8 @@ static apr_status_t bucket_cleanup(void *data)
     h2_stream **pstream = data;
 
     if (*pstream) {
-        /*
-         * If bucket_destroy is called after us, this prevents
-         * bucket_destroy from trying to destroy the pool again.
-         */
+        /* If bucket_destroy is called after us, this prevents
+         * bucket_destroy from trying to destroy the stream again. */
         *pstream = NULL;
     }
     return APR_SUCCESS;
@@ -92,10 +90,13 @@ static void bucket_destroy(void *data)
 
     if (apr_bucket_shared_destroy(h)) {
         h2_stream *stream = h->stream;
+        if (stream && stream->pool) {
+            apr_pool_cleanup_kill(stream->pool, &h->stream, bucket_cleanup);
+        }
+        apr_bucket_free(h);
         if (stream) {
             h2_stream_eos_destroy(stream);
         }
-        apr_bucket_free(h);
     }
 }
 

diff --git a/mod_http2/h2_mplx.c b/mod_http2/h2_mplx.c
@@ -200,7 +200,7 @@ static int purge_stream(void *ctx, void *val)
     h2_ihash_remove(m->spurge, stream->id);
     h2_stream_destroy(stream);
     if (task) {
-        task_destroy(m, task, 1);
+        task_destroy(m, task, 0);
     }
     return 0;
 }
@@ -349,10 +349,12 @@ static void task_destroy(h2_mplx *m, h2_task *task, int called_from_master)
     ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, m->c, 
                   "h2_task(%s): destroy", task->id);
     /* cleanup any buffered input */
-    status = h2_task_shutdown(task, 0);
-    if (status != APR_SUCCESS){
-        ap_log_cerror(APLOG_MARK, APLOG_WARNING, status, m->c, APLOGNO(03385) 
-                      "h2_task(%s): shutdown", task->id);
+    if (task->input.beam) {
+        status = h2_beam_shutdown(task->input.beam, APR_NONBLOCK_READ);
+        if (status != APR_SUCCESS){
+            ap_log_cerror(APLOG_MARK, APLOG_WARNING, status, m->c, APLOGNO(03385) 
+                          "h2_task(%s): input shutdown", task->id);
+        }
     }
 
     if (called_from_master) {
@@ -446,10 +448,8 @@ static void stream_done(h2_mplx *m, h2_stream *stream, int rst_error)
             if (rst_error) {
                 h2_task_rst(task, rst_error);
             }
-            /* FIXME: this should work, but does not 
             h2_ihash_add(m->shold, stream);
-            return;*/
-            task->input.beam = NULL;
+            return;
         }
         else {
             /* already finished */
@@ -500,6 +500,12 @@ static int task_abort_connection(void *ctx, void *val)
     if (task->c) {
         task->c->aborted = 1;
     }
+    if (task->input.beam) {
+        h2_beam_abort(task->input.beam);
+    }
+    if (task->output.beam) {
+        h2_beam_abort(task->output.beam);
+    }
     return 1;
 }
 
@@ -603,7 +609,7 @@ apr_status_t h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)
         AP_DEBUG_ASSERT(h2_ihash_empty(m->shold));
         if (!h2_ihash_empty(m->spurge)) {
             ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c,
-                          "h2_mplx(%ld): release_join %d streams to purge", 
+                          "h2_mplx(%ld): 3. release_join %d streams to purge", 
                           m->id, (int)h2_ihash_count(m->spurge));
             purge_streams(m);
         }
@@ -1028,6 +1034,7 @@ static void task_done(h2_mplx *m, h2_task *task, h2_req_engine *ngn)
                  * parent pool / allocator) */
                 h2_ihash_remove(m->shold, stream->id);
                 h2_ihash_add(m->spurge, stream);
+                task_destroy(m, task, 0);
             }
             else {
                 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c,

diff --git a/mod_http2/h2_mplx.h b/mod_http2/h2_mplx.h
@@ -167,7 +167,7 @@ apr_uint32_t h2_mplx_shutdown(h2_mplx *m);
  * Notifies mplx that a stream has finished processing.
  * 
  * @param m the mplx itself
- * @param stream_id the id of the stream being done
+ * @param stream the id of the stream being done
  * @param rst_error if != 0, the stream was reset with the error given
  *
  */
@@ -188,7 +188,7 @@ apr_status_t h2_mplx_out_trywait(h2_mplx *m, apr_interval_time_t timeout,
  * Process a stream request.
  * 
  * @param m the multiplexer
- * @param stream_id the identifier of the stream
+ * @param stream the identifier of the stream
  * @param r the request to be processed
  * @param cmp the stream priority compare function
  * @param ctx context data for the compare function

diff --git a/mod_http2/h2_push.h b/mod_http2/h2_push.h
@@ -63,7 +63,7 @@ apr_array_header_t *h2_push_collect(apr_pool_t *p,
 /**
  * Create a new push diary for the given maximum number of entries.
  * 
- * @oaram p the pool to use
+ * @param p the pool to use
  * @param N the max number of entries, rounded up to 2^x
  * @return the created diary, might be NULL of max_entries is 0
  */

diff --git a/mod_http2/h2_stream.c b/mod_http2/h2_stream.c
@@ -240,7 +240,7 @@ void h2_stream_destroy(h2_stream *stream)
 void h2_stream_eos_destroy(h2_stream *stream)
 {
     h2_session_stream_done(stream->session, stream);
-    /* stream is gone */
+    /* stream possibly destroyed */
 }
 
 apr_pool_t *h2_stream_detach_pool(h2_stream *stream)

diff --git a/mod_http2/h2_stream.h b/mod_http2/h2_stream.h
@@ -103,7 +103,7 @@ void h2_stream_cleanup(h2_stream *stream);
  * destruction to take the pool with it.
  *
  * @param stream the stream to detach the pool from
- * @param the detached memmory pool or NULL if stream no longer has one
+ * @result the detached memory pool or NULL if stream no longer has one
  */
 apr_pool_t *h2_stream_detach_pool(h2_stream *stream);
 
@@ -153,7 +153,7 @@ apr_status_t h2_stream_write_data(h2_stream *stream,
  * @param stream the stream to reset
  * @param error_code the HTTP/2 error code
  */
-void h2_stream_rst(h2_stream *streamm, int error_code);
+void h2_stream_rst(h2_stream *stream, int error_code);
 
 /**
  * Schedule the stream for execution. All header information must be
@@ -182,7 +182,7 @@ struct h2_response *h2_stream_get_response(h2_stream *stream);
  * the stream response has been collected.
  * 
  * @param stream the stream to set the response for
- * @param resonse the response data for the stream
+ * @param response the response data for the stream
  * @param bb bucket brigade with output data for the stream. Optional,
  *        may be incomplete.
  */

diff --git a/mod_http2/h2_task.c b/mod_http2/h2_task.c
@@ -88,7 +88,7 @@ static apr_status_t input_handle_eos(h2_task *task, request_rec *r,
 {
     apr_status_t status = APR_SUCCESS;
     apr_bucket_brigade *bb = task->input.bb;
-    apr_table_t *t = task->request->trailers;
+    apr_table_t *t = task->request? task->request->trailers : NULL;
 
     if (task->input.chunked) {
         task->input.tmp = apr_brigade_split_ex(bb, b, task->input.tmp);
@@ -114,7 +114,7 @@ static apr_status_t input_append_eos(h2_task *task, request_rec *r)
 {
     apr_status_t status = APR_SUCCESS;
     apr_bucket_brigade *bb = task->input.bb;
-    apr_table_t *t = task->request->trailers;
+    apr_table_t *t = task->request? task->request->trailers : NULL;
 
     if (task->input.chunked) {
         if (t && !apr_is_empty_table(t)) {
@@ -151,13 +151,14 @@ static apr_status_t input_read(h2_task *task, ap_filter_t* f,
         return ap_get_brigade(f->c->input_filters, bb, mode, block, readbytes);
     }
 
-    if (f->c->aborted) {
+    if (f->c->aborted || !task->request) {
         return APR_ECONNABORTED;
     }
 
     if (!task->input.bb) {
         if (!task->input.eos_written) {
             input_append_eos(task, f->r);
+            return APR_SUCCESS;
         }
         return APR_EOF;
     }
@@ -172,11 +173,7 @@ static apr_status_t input_read(h2_task *task, ap_filter_t* f,
         } 
     }
 
-    while (APR_BRIGADE_EMPTY(task->input.bb)) {
-        if (task->input.eos_written) {
-            return APR_EOF;
-        }
-
+    while (APR_BRIGADE_EMPTY(task->input.bb) && !task->input.eos) {
         /* Get more input data for our request. */
         ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, f->c,
                       "h2_task(%s): get more data from mplx, block=%d, "
@@ -193,7 +190,7 @@ static apr_status_t input_read(h2_task *task, ap_filter_t* f,
             status = APR_EOF;
         }
 
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, f->c,
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, f->c,
                       "h2_task(%s): read returned", task->id);
         if (APR_STATUS_IS_EAGAIN(status) 
             && (mode == AP_MODE_GETLINE || block == APR_BLOCK_READ)) {
@@ -202,7 +199,7 @@ static apr_status_t input_read(h2_task *task, ap_filter_t* f,
              * upload 100k test on test-ser.example.org hangs */
             status = APR_SUCCESS;
         }
-        else if (APR_STATUS_IS_EOF(status) && !task->input.eos_written) {
+        else if (APR_STATUS_IS_EOF(status)) {
             task->input.eos = 1;
         }
         else if (status != APR_SUCCESS) {
@@ -251,8 +248,13 @@ static apr_status_t input_read(h2_task *task, ap_filter_t* f,
         }
     }
 
-    if (!task->input.eos_written && task->input.eos) {
-        input_append_eos(task, f->r);
+    if (task->input.eos) {
+        if (!task->input.eos_written) {
+            input_append_eos(task, f->r);
+        }
+        if (APR_BRIGADE_EMPTY(task->input.bb)) {
+            return APR_EOF;
+        }
     }
 
     h2_util_bb_log(f->c, task->stream_id, APLOG_TRACE2, 
@@ -556,23 +558,6 @@ void h2_task_rst(h2_task *task, int error)
     }
 }
 
-apr_status_t h2_task_shutdown(h2_task *task, int block)
-{
-    if (task->output.beam) {
-        apr_status_t status;
-        status = h2_beam_shutdown(task->output.beam, APR_NONBLOCK_READ);
-        if (block && status == APR_EAGAIN) {
-            ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, task->c,
-                          "h2_task(%s): output shutdown waiting", task->id);
-            status = h2_beam_shutdown(task->output.beam, APR_BLOCK_READ);
-            ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, task->c,
-                          "h2_task(%s): output shutdown done", task->id);
-        }
-        return status;
-    }
-    return APR_SUCCESS;
-}
-
 /*******************************************************************************
  * Register various hooks
  */

diff --git a/mod_http2/h2_task.h b/mod_http2/h2_task.h
@@ -114,11 +114,6 @@ int h2_task_can_redo(h2_task *task);
  */
 void h2_task_rst(h2_task *task, int error);
 
-/**
- * Shuts all input/output down. Clears any buckets buffered and closes.
- */
-apr_status_t h2_task_shutdown(h2_task *task, int block);
-
 void h2_task_register_hooks(void);
 /*
  * One time, post config intialization.

diff --git a/mod_http2/h2_util.h b/mod_http2/h2_util.h
@@ -58,7 +58,7 @@ void *h2_ihash_get(h2_ihash_t *ih, int id);
  * @param ih the hash to iterate over
  * @param fn the function to invoke on each member
  * @param ctx user supplied data passed into each iteration call
- * @param 0 if one iteration returned 0, otherwise != 0
+ * @return 0 if one iteration returned 0, otherwise != 0
  */
 int h2_ihash_iter(h2_ihash_t *ih, h2_ihash_iter_t *fn, void *ctx);
 
@@ -141,14 +141,14 @@ int h2_iq_empty(h2_iqueue *q);
 int h2_iq_count(h2_iqueue *q);
 
 /**
- * Add a stream idto the queue. 
+ * Add a stream id to the queue. 
  *
  * @param q the queue to append the task to
  * @param sid the stream id to add
  * @param cmp the comparator for sorting
  * @param ctx user data for comparator 
  */
-void h2_iq_add(h2_iqueue *q, int i, h2_iq_cmp *cmp, void *ctx);
+void h2_iq_add(h2_iqueue *q, int sid, h2_iq_cmp *cmp, void *ctx);
 
 /**
  * Remove the stream id from the queue. Return != 0 iff task
@@ -157,7 +157,7 @@ void h2_iq_add(h2_iqueue *q, int i, h2_iq_cmp *cmp, void *ctx);
  * @param sid the stream id to remove
  * @return != 0 iff task was found in queue
  */
-int h2_iq_remove(h2_iqueue *q, int i);
+int h2_iq_remove(h2_iqueue *q, int sid);
 
 /**
  * Remove all entries in the queue.
@@ -374,19 +374,19 @@ apr_size_t h2_util_bb_print(char *buffer, apr_size_t bmax,
  * Logs the bucket brigade (which bucket types with what length)
  * to the log at the given level.
  * @param c the connection to log for
- * @param stream_id the stream identifier this brigade belongs to
+ * @param sid the stream identifier this brigade belongs to
  * @param level the log level (as in APLOG_*)
  * @param tag a short message text about the context
  * @param bb the brigade to log
  */
-#define h2_util_bb_log(c, i, level, tag, bb) \
+#define h2_util_bb_log(c, sid, level, tag, bb) \
 do { \
     char buffer[4 * 1024]; \
     const char *line = "(null)"; \
     apr_size_t len, bmax = sizeof(buffer)/sizeof(buffer[0]); \
     len = h2_util_bb_print(buffer, bmax, (tag), "", (bb)); \
     ap_log_cerror(APLOG_MARK, level, 0, (c), "bb_dump(%ld-%d): %s", \
-        (c)->id, (int)(i), (len? buffer : line)); \
+        (c)->id, (int)(sid), (len? buffer : line)); \
 } while(0)
 
 

diff --git a/mod_http2/h2_version.h b/mod_http2/h2_version.h
@@ -26,15 +26,15 @@
  * @macro
  * Version number of the http2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.5.6"
+#define MOD_HTTP2_VERSION "1.5.7"
 
 /**
  * @macro
  * Numerical representation of the version number of the http2 module
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_HTTP2_VERSION_NUM 0x010506
+#define MOD_HTTP2_VERSION_NUM 0x010507
 
 
 #endif /* mod_h2_h2_version_h */