*) mod_http2: Avoid segfaults in case of handling certain responses for

    already aborted connections.
    Trunk version of patch:
      http://svn.apache.org/r1879544
      http://svn.apache.org/r1879546
      http://svn.apache.org/r1879547
    Backport version for 2.4.x of patch:
      https://github.com/apache/httpd/pull/132.diff
    +1: rpluem, icing, minfrin

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1879891 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 31ad67417151a9f56630f9b5c2a91827d90ce47f..d69cfa81ceab6506f3a0b86df2935e1a3242d5f9 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.44
 
+  *) mod_http2: Avoid segfaults in case of handling certain responses for
+     already aborted connections.  [Stefan Eissing, Ruediger Pluem]
+
   *) mod_http2: The module now handles master/secondary connections and has marked
      methods according to use. [Stefan Eissing]
 

diff --git a/STATUS b/STATUS
index 9e8ca3fe3f7e153c34049f2532b8248515040b63..582eb067adab239b8bf87b5d2f7d5eca656d3d68 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -135,15 +135,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
- *) mod_http2: Avoid segfaults in case of handling certain responses for
-    already aborted connections.
-    Trunk version of patch:
-      http://svn.apache.org/r1879544
-      http://svn.apache.org/r1879546
-      http://svn.apache.org/r1879547
-    Backport version for 2.4.x of patch:
-      https://github.com/apache/httpd/pull/132.diff
-    +1: rpluem, icing, minfrin
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:

diff --git a/modules/http2/h2_from_h1.c b/modules/http2/h2_from_h1.c
index 47b572e8b45f969a8f51d51828e1d3fc9bcfaa72..073076b73f699b5e3ecc97cfaa70ab71003b4599 100644 (file)
--- a/modules/http2/h2_from_h1.c
+++ b/modules/http2/h2_from_h1.c
@@ -315,6 +315,7 @@ typedef struct h2_response_parser {
     int http_status;
     apr_array_header_t *hlines;
     apr_bucket_brigade *tmp;
+    apr_bucket_brigade *saveto;
 } h2_response_parser;
 
 static apr_status_t parse_header(h2_response_parser *parser, char *line) {
@@ -351,13 +352,17 @@ static apr_status_t get_line(h2_response_parser *parser, apr_bucket_brigade *bb,
         parser->tmp = apr_brigade_create(task->pool, task->c->bucket_alloc);
     }
     status = apr_brigade_split_line(parser->tmp, bb, APR_BLOCK_READ, 
-                                    HUGE_STRING_LEN);
+                                    len);
     if (status == APR_SUCCESS) {
         --len;
         status = apr_brigade_flatten(parser->tmp, line, &len);
         if (status == APR_SUCCESS) {
             /* we assume a non-0 containing line and remove trailing crlf. */
             line[len] = '\0';
+            /*
+             * XXX: What to do if there is an LF but no CRLF?
+             *      Should we error out?
+             */
             if (len >= 2 && !strcmp(H2_CRLF, line + len - 2)) {
                 len -= 2;
                 line[len] = '\0';
@@ -367,10 +372,47 @@ static apr_status_t get_line(h2_response_parser *parser, apr_bucket_brigade *bb,
                               task->id, line);
             }
             else {
+                apr_off_t brigade_length;
+
+                /*
+                 * If the brigade parser->tmp becomes longer than our buffer
+                 * for flattening we never have a chance to get a complete
+                 * line. This can happen if we are called multiple times after
+                 * previous calls did not find a H2_CRLF and we returned
+                 * APR_EAGAIN. In this case parser->tmp (correctly) grows
+                 * with each call to apr_brigade_split_line.
+                 *
+                 * XXX: Currently a stack based buffer of HUGE_STRING_LEN is
+                 * used. This means we cannot cope with lines larger than
+                 * HUGE_STRING_LEN which might be an issue.
+                 */
+                status = apr_brigade_length(parser->tmp, 0, &brigade_length);
+                if ((status != APR_SUCCESS) || (brigade_length > len)) {
+                    ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, task->c, APLOGNO(10257)
+                                  "h2_task(%s): read response, line too long",
+                                  task->id);
+                    return APR_ENOSPC;
+                }
                 /* this does not look like a complete line yet */
                 ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, task->c,
                               "h2_task(%s): read response, incomplete line: %s", 
                               task->id, line);
+                if (!parser->saveto) {
+                    parser->saveto = apr_brigade_create(task->pool,
+                                                        task->c->bucket_alloc);
+                }
+                /*
+                 * Be on the save side and save the parser->tmp brigade
+                 * as it could contain transient buckets which could be
+                 * invalid next time we are here.
+                 *
+                 * NULL for the filter parameter is ok since we
+                 * provide our own brigade as second parameter
+                 * and ap_save_brigade does not need to create one.
+                 */
+                ap_save_brigade(NULL, &(parser->saveto), &(parser->tmp),
+                                parser->tmp->p);
+                APR_BRIGADE_CONCAT(parser->tmp, parser->saveto);
                 return APR_EAGAIN;
             }
         }

diff --git a/modules/http2/h2_task.c b/modules/http2/h2_task.c
index 606fecc75d2aa0e4eb16d8f6848cd099d2d784a9..dac3dfe49990ada6bda8ae7fe741f948d37a32c6 100644 (file)
--- a/modules/http2/h2_task.c
+++ b/modules/http2/h2_task.c
@@ -380,7 +380,7 @@ static apr_status_t h2_filter_parse_h1(ap_filter_t* f, apr_bucket_brigade* bb)
     /* There are cases where we need to parse a serialized http/1.1 
      * response. One example is a 100-continue answer in serialized mode
      * or via a mod_proxy setup */
-    while (bb && !task->output.sent_response) {
+    while (bb && !task->c->aborted && !task->output.sent_response) {
         status = h2_from_h1_parse_response(task, f, bb);
         ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, f->c,
                       "h2_task(%s): parsed response", task->id);