From: Graham Leggett Date: Wed, 15 Jul 2020 14:17:17 +0000 (+0000) Subject: *) mod_http2: Avoid segfaults in case of handling certain responses for X-Git-Tag: 2.4.44~18 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ca90af8ee725381c9819e3f41e38f06a27253acd;p=thirdparty%2Fapache%2Fhttpd.git *) 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 31ad6741715..d69cfa81cea 100644 --- 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 9e8ca3fe3f7..582eb067ada 100644 --- 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 47b572e8b45..073076b73f6 100644 --- 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 606fecc75d2..dac3dfe4999 100644 --- 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);