Bug#989562: apache2: CVE-2021-31618: NULL pointer dereference on specially crafted HTTP/2 request
Le 07/06/2021 à 17:34, Salvatore Bonaccorso a écrit :
> Source: apache2
> Version: 2.4.47-1
> Severity: grave
> Tags: security upstream
> Justification: user security hole
> X-Debbugs-Cc: carnil@debian.org, Debian Security Team <team@security.debian.org>
>
> Hi,
>
> The following vulnerability was published for apache2.
>
> CVE-2021-31618[0]:
> | httpd: NULL pointer dereference on specially crafted HTTP/2 request
>
> If you fix the vulnerability please also make sure to include the
> CVE (Common Vulnerabilities & Exposures) id in your changelog entry.
>
> For further information see:
>
> [0] https://security-tracker.debian.org/tracker/CVE-2021-31618
> https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-31618
> [1] https://github.com/apache/httpd/commit/a4fba223668c554e06bc78d6e3a88f33d4238ae4
> [2] https://httpd.apache.org/security/vulnerabilities_24.html#CVE-2021-31618
>
> Please adjust the affected versions in the BTS as needed.
>
> Regards,
> Salvatore
Hi all,
I can't import the whole patch for Bullseye since it is written for
2.4.47. I think the best solution is to import the whole http2 module in
Bullseye. This gives the attached patch
Cheers,
Yadd
Description: import the whole HTTP/2 module from 2.4.47 to fix CVE-2021-31618
Author: Xavier Guimard <yadd@debian.org>
Origin: upstream
Bug: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-31618
Bug-Debian: https://bugs.debian.org/989562
Forwarded: not-needed
Reviewed-By: Yadd <yadd@debian.org>
Last-Update: 2021-06-08
--- a/modules/http2/h2.h
+++ b/modules/http2/h2.h
@@ -141,8 +141,19 @@
unsigned int chunked : 1; /* iff request body needs to be forwarded as chunked */
unsigned int serialize : 1; /* iff this request is written in HTTP/1.1 serialization */
apr_off_t raw_bytes; /* RAW network bytes that generated this
request - if known. */
+ int http_status; /* Store a possible HTTP status code that gets
+ * defined before creating the dummy HTTP/1.1
+ * request e.g. due to an error already
+ * detected.
+ */
};
+/*
+ * A possible HTTP status code is not defined yet. See the http_status field
+ * in struct h2_request above for further explanation.
+ */
+#define H2_HTTP_STATUS_UNSET (0)
+
typedef struct h2_headers h2_headers;
struct h2_headers {
--- a/modules/http2/h2_bucket_beam.c
+++ b/modules/http2/h2_bucket_beam.c
@@ -945,7 +945,8 @@
apr_status_t h2_beam_receive(h2_bucket_beam *beam,
apr_bucket_brigade *bb,
apr_read_type_e block,
- apr_off_t readbytes)
+ apr_off_t readbytes,
+ int *pclosed)
{
h2_beam_lock bl;
apr_bucket *bsender, *brecv, *ng;
@@ -953,7 +954,7 @@
apr_status_t status = APR_SUCCESS;
apr_off_t remain;
int transferred_buckets = 0;
-
+
/* Called from the receiver thread to take buckets from the beam */
if (enter_yellow(beam, &bl) == APR_SUCCESS) {
if (readbytes <= 0) {
@@ -1039,6 +1040,7 @@
H2_BLIST_INSERT_TAIL(&beam->hold_list, bsender);
remain -= bsender->length;
+ beam->received_bytes += bsender->length;
++transferred;
++transferred_buckets;
continue;
@@ -1126,7 +1128,8 @@
}
goto transfer;
}
-leave:
+leave:
+ if (pclosed) *pclosed = beam->closed? 1 : 0;
leave_yellow(beam, &bl);
}
return status;
--- a/modules/http2/h2_bucket_beam.h
+++ b/modules/http2/h2_bucket_beam.h
@@ -258,11 +258,15 @@
* if no data is available.
*
* Call from the receiver side only.
+ * @param pclosed on return != 0 iff the beam has been closed by the sender. It
+ * may still hold untransfered data. Maybe NULL if the caller is
+ * not interested in this.
*/
apr_status_t h2_beam_receive(h2_bucket_beam *beam,
apr_bucket_brigade *green_buckets,
apr_read_type_e block,
- apr_off_t readbytes);
+ apr_off_t readbytes,
+ int *pclosed);
/**
* Determine if beam is empty.
--- a/modules/http2/h2_config.c
+++ b/modules/http2/h2_config.c
@@ -78,6 +78,7 @@
int early_hints; /* support status code 103 */
int padding_bits;
int padding_always;
+ int output_buffered;
} h2_config;
typedef struct h2_dir_config {
@@ -115,6 +116,7 @@
0, /* early hints, http status 103 */
0, /* padding bits */
1, /* padding always */
+ 1, /* strean output buffered */
};
static h2_dir_config defdconf = {
@@ -159,6 +161,7 @@
conf->early_hints = DEF_VAL;
conf->padding_bits = DEF_VAL;
conf->padding_always = DEF_VAL;
+ conf->output_buffered = DEF_VAL;
return conf;
}
@@ -193,6 +196,7 @@
}
n->push_diary_size = H2_CONFIG_GET(add, base, push_diary_size);
n->copy_files = H2_CONFIG_GET(add, base, copy_files);
+ n->output_buffered = H2_CONFIG_GET(add, base, output_buffered);
if (add->push_list && base->push_list) {
n->push_list = apr_array_append(pool, base->push_list, add->push_list);
}
@@ -286,6 +290,8 @@
return H2_CONFIG_GET(conf, &defconf, padding_bits);
case H2_CONF_PADDING_ALWAYS:
return H2_CONFIG_GET(conf, &defconf, padding_always);
+ case H2_CONF_OUTPUT_BUFFER:
+ return H2_CONFIG_GET(conf, &defconf, output_buffered);
default:
return DEF_VAL;
}
@@ -351,6 +357,9 @@
case H2_CONF_PADDING_ALWAYS:
H2_CONFIG_SET(conf, padding_always, val);
break;
+ case H2_CONF_OUTPUT_BUFFER:
+ H2_CONFIG_SET(conf, output_buffered, val);
+ break;
default:
break;
}
@@ -721,7 +730,7 @@
else if (!strcasecmp("BEFORE", sdependency)) {
dependency = H2_DEPENDANT_BEFORE;
if (sweight) {
- return "dependency 'Before' does not allow a weight";
+ return "dependecy 'Before' does not allow a weight";
}
}
else if (!strcasecmp("INTERLEAVED", sdependency)) {
@@ -904,6 +913,19 @@
return NULL;
}
+static const char *h2_conf_set_output_buffer(cmd_parms *cmd,
+ void *dirconf, const char *value)
+{
+ if (!strcasecmp(value, "On")) {
+ CONFIG_CMD_SET(cmd, dirconf, H2_CONF_OUTPUT_BUFFER, 1);
+ return NULL;
+ }
+ else if (!strcasecmp(value, "Off")) {
+ CONFIG_CMD_SET(cmd, dirconf, H2_CONF_OUTPUT_BUFFER, 0);
+ return NULL;
+ }
+ return "value must be On or Off";
+}
void h2_get_num_workers(server_rec *s, int *minw, int *maxw)
{
@@ -975,6 +997,8 @@
RSRC_CONF, "on to enable interim status 103 responses"),
AP_INIT_TAKE1("H2Padding", h2_conf_set_padding, NULL,
RSRC_CONF, "set payload padding"),
+ AP_INIT_TAKE1("H2OutputBuffering", h2_conf_set_output_buffer, NULL,
+ RSRC_CONF, "set stream output buffer on/off"),
AP_END_CMD
};
--- a/modules/http2/h2_config.h
+++ b/modules/http2/h2_config.h
@@ -44,6 +44,7 @@
H2_CONF_EARLY_HINTS,
H2_CONF_PADDING_BITS,
H2_CONF_PADDING_ALWAYS,
+ H2_CONF_OUTPUT_BUFFER,
} h2_config_var_t;
struct apr_hash_t;
--- a/modules/http2/h2_h2.c
+++ b/modules/http2/h2_h2.c
@@ -749,6 +749,7 @@
if (task) {
/* check if we copy vs. setaside files in this location */
task->output.copy_files = h2_config_rgeti(r, H2_CONF_COPY_FILES);
+ task->output.buffered = h2_config_rgeti(r, H2_CONF_OUTPUT_BUFFER);
if (task->output.copy_files) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, task->c,
"h2_secondary_out(%s): copy_files on", task->id);
--- a/modules/http2/h2_headers.c
+++ b/modules/http2/h2_headers.c
@@ -64,6 +64,7 @@
b = apr_bucket_shared_make(b, br, 0, 0);
b->type = &h2_bucket_type_headers;
+ b->length = h2_headers_length(r);
return b;
}
@@ -125,6 +126,20 @@
return headers;
}
+static int add_header_lengths(void *ctx, const char *name, const char *value)
+{
+ apr_size_t *plen = ctx;
+ *plen += strlen(name) + strlen(value);
+ return 1;
+}
+
+apr_size_t h2_headers_length(h2_headers *headers)
+{
+ apr_size_t len = 0;
+ apr_table_do(add_header_lengths, &len, headers->headers, NULL);
+ return len;
+}
+
h2_headers *h2_headers_rcreate(request_rec *r, int status,
apr_table_t *header, apr_pool_t *pool)
{
--- a/modules/http2/h2_headers.h
+++ b/modules/http2/h2_headers.h
@@ -82,4 +82,9 @@
int h2_headers_are_response(h2_headers *headers);
+/**
+ * Give the number of bytes of all contained header strings.
+ */
+apr_size_t h2_headers_length(h2_headers *headers);
+
#endif /* defined(__mod_h2__h2_headers__) */
--- a/modules/http2/h2_mplx.c
+++ b/modules/http2/h2_mplx.c
@@ -91,10 +91,6 @@
static void mst_check_data_for(h2_mplx *m, h2_stream *stream, int mplx_is_locked);
-static void mst_stream_output_consumed(void *ctx, h2_bucket_beam *beam, apr_off_t length)
-{
-}
-
static void mst_stream_input_ev(void *ctx, h2_bucket_beam *beam)
{
h2_stream *stream = ctx;
@@ -299,18 +295,6 @@
stream->task = NULL;
secondary = task->c;
if (secondary) {
- /* On non-serialized requests, the IO logging has not accounted for any
- * meta data send over the network: response headers and h2 frame headers. we
- * counted this on the stream and need to add this now.
- * This is supposed to happen before the EOR bucket triggers
the
- * logging of the transaction. *fingers crossed* */
- if (task->request && !task->request->serialize && h2_task_logio_add_bytes_out) {
- apr_off_t unaccounted = stream->out_frame_octets - stream->out_data_octets;
- if (unaccounted > 0) {
- h2_task_logio_add_bytes_out(secondary, unaccounted);
- }
- }
-
if (m->s->keep_alive_max == 0 || secondary->keepalives <
m->s->keep_alive_max) {
reuse_secondary = ((m->spare_secondary->nelts < (m->limit_active * 3 / 2))
&& !task->rst_error);
@@ -540,7 +524,6 @@
"h2_mplx(%s): out open", stream->task->id);
}
- h2_beam_on_consumed(stream->output, NULL, mst_stream_output_consumed, stream);
h2_beam_on_produced(stream->output, mst_output_produced, stream);
if (stream->task->output.copy_files) {
h2_beam_on_file_beam(stream->output, h2_beam_no_files, NULL);
--- a/modules/http2/h2_request.c
+++ b/modules/http2/h2_request.c
@@ -79,11 +79,12 @@
}
req = apr_pcalloc(pool, sizeof(*req));
- req->method = apr_pstrdup(pool, r->method);
- req->scheme = scheme;
- req->authority = authority;
- req->path = path;
- req->headers = apr_table_make(pool, 10);
+ req->method = apr_pstrdup(pool, r->method);
+ req->scheme = scheme;
+ req->authority = authority;
+ req->path = path;
+ req->headers = apr_table_make(pool, 10);
+ req->http_status = H2_HTTP_STATUS_UNSET;
if (r->server) {
req->serialize = h2_config_rgeti(r, H2_CONF_SER_HEADERS);
}
@@ -269,9 +270,7 @@
request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c)
{
- int access_status = HTTP_OK;
- const char *rpath;
- const char *s;
+ int access_status;
#if AP_MODULE_MAGIC_AT_LEAST(20150222, 13)
request_rec *r = ap_create_request(c);
@@ -279,52 +278,88 @@
request_rec *r = my_ap_create_request(c);
#endif
- r->headers_in = apr_table_clone(r->pool, req->headers);
-
+#if AP_MODULE_MAGIC_AT_LEAST(20200331, 3)
ap_run_pre_read_request(r, c);
-
+
/* Time to populate r with the data we have. */
r->request_time = req->request_time;
- r->method = apr_pstrdup(r->pool, req->method);
- /* Provide quick information about the request method as soon as known */
- r->method_number = ap_method_number_of(r->method);
- if (r->method_number == M_GET && r->method[0] == 'H') {
- r->header_only = 1;
- }
- r->the_request = apr_psprintf(r->pool, "%s %s HTTP/2.0",
+ r->the_request = apr_psprintf(r->pool, "%s %s HTTP/2.0",
req->method, req->path ? req->path : "");
r->headers_in = apr_table_clone(r->pool, req->headers);
- rpath = (req->path ? req->path : "");
- ap_parse_uri(r, rpath);
- r->protocol = (char*)"HTTP/2.0";
- r->proto_num = HTTP_VERSION(2, 0);
-
- r->the_request = apr_psprintf(r->pool, "%s %s %s",
- r->method, rpath, r->protocol);
-
- /* update what we think the virtual host is based on the headers we've
- * now read. may update status.
- * Leave r->hostname empty, vhost will parse if form our Host: header,
- * otherwise we get complains about port numbers.
+ /* Start with r->hostname = NULL, ap_check_request_header() will get it
+ * form Host: header, otherwise we get complains about port numbers.
*/
r->hostname = NULL;
- ap_update_vhost_from_headers(r);
- r->protocol = "HTTP/2.0";
- r->proto_num = HTTP_VERSION(2, 0);
- /* we may have switched to another server */
- r->per_dir_config = r->server->lookup_defaults;
-
- s = apr_table_get(r->headers_in, "Expect");
- if (s && s[0]) {
- if (ap_cstr_casecmp(s, "100-continue") == 0) {
- r->expecting_100 = 1;
+ /* Validate HTTP/1 request and select vhost. */
+ if (!ap_parse_request_line(r) || !ap_check_request_header(r)) {
+ /* we may have switched to another server still */
+ r->per_dir_config = r->server->lookup_defaults;
+ if (req->http_status != H2_HTTP_STATUS_UNSET) {
+ access_status = req->http_status;
+ /* Be safe and close the connection */
+ c->keepalive = AP_CONN_CLOSE;
}
else {
- r->status = HTTP_EXPECTATION_FAILED;
- ap_send_error_response(r, 0);
+ access_status = r->status;
}
+ r->status = HTTP_OK;
+ goto die;
+ }
+#else
+ {
+ const char *s;
+
+ r->headers_in = apr_table_clone(r->pool, req->headers);
+ ap_run_pre_read_request(r, c);
+
+ /* Time to populate r with the data we have. */
+ r->request_time = req->request_time;
+ r->method = apr_pstrdup(r->pool, req->method);
+ /* Provide quick information about the request method as soon as
known */
+ r->method_number = ap_method_number_of(r->method);
+ if (r->method_number == M_GET && r->method[0] == 'H') {
+ r->header_only = 1;
+ }
+ ap_parse_uri(r, req->path ? req->path : "");
+ r->protocol = (char*)"HTTP/2.0";
+ r->proto_num = HTTP_VERSION(2, 0);
+ r->the_request = apr_psprintf(r->pool, "%s %s HTTP/2.0",
+ r->method, req->path ? req->path :
"");
+
+ /* Start with r->hostname = NULL, ap_check_request_header() will get it
+ * form Host: header, otherwise we get complains about port numbers.
+ */
+ r->hostname = NULL;
+ ap_update_vhost_from_headers(r);
+
+ /* we may have switched to another server */
+ r->per_dir_config = r->server->lookup_defaults;
+
+ s = apr_table_get(r->headers_in, "Expect");
+ if (s && s[0]) {
+ if (ap_cstr_casecmp(s, "100-continue") == 0) {
+ r->expecting_100 = 1;
+ }
+ else {
+ r->status = HTTP_EXPECTATION_FAILED;
+ access_status = r->status;
+ goto die;
+ }
+ }
+ }
+#endif
+
+ /* we may have switched to another server */
+ r->per_dir_config = r->server->lookup_defaults;
+
+ if (req->http_status != H2_HTTP_STATUS_UNSET) {
+ access_status = req->http_status;
+ r->status = HTTP_OK;
+ /* Be safe and close the connection */
+ c->keepalive = AP_CONN_CLOSE;
+ goto die;
}
/*
@@ -336,28 +371,47 @@
ap_add_input_filter_handle(ap_http_input_filter_handle,
NULL, r, r->connection);
- if (access_status != HTTP_OK
- || (access_status = ap_run_post_read_request(r))) {
+ if ((access_status = ap_run_post_read_request(r))) {
/* Request check post hooks failed. An example of this would be a
* request for a vhost where h2 is disabled --> 421.
*/
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(03367)
"h2_request: access_status=%d, request_create failed",
access_status);
- ap_die(access_status, r);
- ap_update_child_status(c->sbh, SERVER_BUSY_LOG, r);
- ap_run_log_transaction(r);
- r = NULL;
- goto traceout;
+ goto die;
}
AP_READ_REQUEST_SUCCESS((uintptr_t)r, (char *)r->method,
(char *)r->uri, (char *)r->server->defn_name,
r->status);
return r;
-traceout:
+
+die:
+ ap_die(access_status, r);
+
+ /* ap_die() sent the response through the output filters, we must now
+ * end the request with an EOR bucket for stream/pipeline accounting.
+ */
+ {
+ apr_bucket_brigade *eor_bb;
+#if AP_MODULE_MAGIC_AT_LEAST(20180905, 1)
+ eor_bb = ap_acquire_brigade(c);
+ APR_BRIGADE_INSERT_TAIL(eor_bb,
+ ap_bucket_eor_create(c->bucket_alloc, r));
+ ap_pass_brigade(c->output_filters, eor_bb);
+ ap_release_brigade(c, eor_bb);
+#else
+ eor_bb = apr_brigade_create(c->pool, c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(eor_bb,
+ ap_bucket_eor_create(c->bucket_alloc, r));
+ ap_pass_brigade(c->output_filters, eor_bb);
+ apr_brigade_destroy(eor_bb);
+#endif
+ }
+
+ r = NULL;
AP_READ_REQUEST_FAILURE((uintptr_t)r);
- return r;
+ return NULL;
}
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -311,7 +311,9 @@
status = h2_stream_add_header(stream, (const char *)name, namelen,
(const char *)value, valuelen);
- if (status != APR_SUCCESS && !h2_stream_is_ready(stream)) {
+ if (status != APR_SUCCESS
+ && (!stream->rtmp
+ || stream->rtmp->http_status == H2_HTTP_STATUS_UNSET)) {
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
return 0;
--- a/modules/http2/h2_stream.c
+++ b/modules/http2/h2_stream.c
@@ -639,16 +639,7 @@
static void set_error_response(h2_stream *stream, int http_status)
{
if (!h2_stream_is_ready(stream)) {
- conn_rec *c = stream->session->c;
- apr_bucket *b;
- h2_headers *response;
-
- response = h2_headers_die(http_status, stream->request, stream->pool);
- prep_output(stream);
- b = apr_bucket_eos_create(c->bucket_alloc);
- APR_BRIGADE_INSERT_HEAD(stream->out_buffer, b);
- b = h2_bucket_headers_create(c->bucket_alloc, response);
- APR_BRIGADE_INSERT_HEAD(stream->out_buffer, b);
+ stream->rtmp->http_status = http_status;
}
}
@@ -910,7 +901,7 @@
apr_status_t status = APR_SUCCESS;
apr_off_t requested, missing, max_chunk = H2_DATA_CHUNK_SIZE;
conn_rec *c;
- int complete;
+ int complete, was_closed = 0;
ap_assert(stream);
@@ -959,9 +950,11 @@
if (stream->output) {
H2_STREAM_OUT_LOG(APLOG_TRACE2, stream, "pre");
- rv = h2_beam_receive(stream->output, stream->out_buffer,
- APR_NONBLOCK_READ, stream->max_mem - *plen);
+ h2_beam_log(stream->output, c, APLOG_TRACE2, "pre read output");
+ rv = h2_beam_receive(stream->output, stream->out_buffer,
+ APR_NONBLOCK_READ, stream->max_mem - *plen, &was_closed);
H2_STREAM_OUT_LOG(APLOG_TRACE2, stream, "post");
+ h2_beam_log(stream->output, c, APLOG_TRACE2, "post read output");
}
if (rv == APR_SUCCESS) {
@@ -991,7 +984,7 @@
(long)*plen, *peos);
}
else {
- status = (stream->output && h2_beam_is_closed(stream->output))? APR_EOF : APR_EAGAIN;
+ status = was_closed? APR_EOF : APR_EAGAIN;
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
H2_STRM_MSG(stream, "prepare, no data"));
}
--- a/modules/http2/h2_stream.h
+++ b/modules/http2/h2_stream.h
@@ -92,7 +92,8 @@
unsigned int input_eof : 1; /* no more request data coming */
unsigned int out_checked : 1; /* output eof was double checked */
unsigned int push_policy; /* which push policy to use for this request */
-
+ unsigned int input_buffering : 1; /* buffer request bodies for efficiency */
+
struct h2_task *task; /* assigned task to fullfill request */
const h2_priority *pref_priority; /* preferred priority for this stream */
--- a/modules/http2/h2_task.c
+++ b/modules/http2/h2_task.c
@@ -89,6 +89,14 @@
return h2_mplx_t_out_open(task->mplx, task->stream_id, task->output.beam);
}
+static void output_consumed(void *ctx, h2_bucket_beam *beam, apr_off_t length)
+{
+ h2_task *task = ctx;
+ if (task && h2_task_logio_add_bytes_out) {
+ h2_task_logio_add_bytes_out(task->c, length);
+ }
+}
+
static apr_status_t send_out(h2_task *task, apr_bucket_brigade* bb, int block)
{
apr_off_t written, left;
@@ -108,9 +116,6 @@
status = APR_SUCCESS;
}
if (status == APR_SUCCESS) {
- if (h2_task_logio_add_bytes_out) {
- h2_task_logio_add_bytes_out(task->c, written);
- }
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, task->c,
"h2_task(%s): send_out done", task->id);
}
@@ -183,7 +188,9 @@
}
}
- if (APR_SUCCESS == rv && !task->output.opened && flush) {
+ ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, task->c,
+ "h2_secondary_out(%s): buffered=%d", task->id, task->output.buffered);
+ if (APR_SUCCESS == rv && !task->output.opened && (flush || !task->output.buffered)) {
/* got a flush or could not write all, time to tell someone to read */
rv = open_output(task);
}
@@ -259,7 +266,7 @@
}
if (task->input.beam) {
status = h2_beam_receive(task->input.beam, task->input.bb,
block,
- 128*1024);
+ 128*1024, NULL);
}
else {
status = APR_EOF;
@@ -597,7 +604,8 @@
h2_beam_buffer_size_set(task->output.beam, task->output.max_buffer);
h2_beam_send_from(task->output.beam, task->pool);
-
+ h2_beam_on_consumed(task->output.beam, NULL, output_consumed, task);
+
h2_ctx_create_for(c, task);
apr_table_setn(c->notes, H2_TASK_ID_NOTE, task->id);
--- a/modules/http2/h2_task.h
+++ b/modules/http2/h2_task.h
@@ -71,6 +71,7 @@
unsigned int opened : 1;
unsigned int sent_response : 1;
unsigned int copy_files : 1;
+ unsigned int buffered : 1;
struct h2_response_parser *rparser;
apr_bucket_brigade *bb;
apr_size_t max_buffer;
--- a/modules/http2/h2_version.h
+++ b/modules/http2/h2_version.h
@@ -27,7 +27,7 @@
* @macro
* Version number of the http2 module as c string
*/
-#define MOD_HTTP2_VERSION "1.15.14"
+#define MOD_HTTP2_VERSION "1.15.17"
/**
* @macro
@@ -35,6 +35,7 @@
* 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 0x010f0e
+#define MOD_HTTP2_VERSION_NUM 0x010f11
+
#endif /* mod_h2_h2_version_h */
--- a/modules/http2/h2_workers.c
+++ b/modules/http2/h2_workers.c
@@ -34,17 +34,16 @@
typedef struct h2_slot h2_slot;
struct h2_slot {
int id;
+ int sticks;
h2_slot *next;
h2_workers *workers;
- int aborted;
- int sticks;
h2_task *task;
apr_thread_t *thread;
apr_thread_mutex_t *lock;
apr_thread_cond_t *not_idle;
};
-static h2_slot *pop_slot(h2_slot **phead)
+static h2_slot *pop_slot(h2_slot *volatile *phead)
{
/* Atomically pop a slot from the list */
for (;;) {
@@ -59,7 +58,7 @@
}
}
-static void push_slot(h2_slot **phead, h2_slot *slot)
+static void push_slot(h2_slot *volatile *phead, h2_slot *slot)
{
/* Atomically push a slot to the list */
ap_assert(!slot->next);
@@ -78,7 +77,6 @@
apr_status_t status;
slot->workers = workers;
- slot->aborted = 0;
slot->task = NULL;
if (!slot->lock) {
@@ -101,16 +99,18 @@
ap_log_error(APLOG_MARK, APLOG_TRACE2, 0, workers->s,
"h2_workers: new thread for slot %d", slot->id);
+
/* thread will either immediately start work or add itself
* to the idle queue */
- apr_thread_create(&slot->thread, workers->thread_attr, slot_run, slot,
- workers->pool);
- if (!slot->thread) {
+ apr_atomic_inc32(&workers->worker_count);
+ status = apr_thread_create(&slot->thread, workers->thread_attr,
+ slot_run, slot, workers->pool);
+ if (status != APR_SUCCESS) {
+ apr_atomic_dec32(&workers->worker_count);
push_slot(&workers->free, slot);
- return APR_ENOMEM;
+ return status;
}
- apr_atomic_inc32(&workers->worker_count);
return APR_SUCCESS;
}
@@ -136,17 +136,15 @@
}
}
-static void cleanup_zombies(h2_workers *workers)
+static void join_zombies(h2_workers *workers)
{
h2_slot *slot;
while ((slot = pop_slot(&workers->zombies))) {
- if (slot->thread) {
- apr_status_t status;
- apr_thread_join(&status, slot->thread);
- slot->thread = NULL;
- }
- apr_atomic_dec32(&workers->worker_count);
- slot->next = NULL;
+ apr_status_t status;
+ ap_assert(slot->thread != NULL);
+ apr_thread_join(&status, slot->thread);
+ slot->thread = NULL;
+
push_slot(&workers->free, slot);
}
}
@@ -184,37 +182,49 @@
* Get the next task for the given worker. Will block until a task arrives
* or the max_wait timer expires and more than min workers exist.
*/
-static apr_status_t get_next(h2_slot *slot)
+static int get_next(h2_slot *slot)
{
h2_workers *workers = slot->workers;
- apr_status_t status;
-
- slot->task = NULL;
- while (!slot->aborted) {
- if (!slot->task) {
- status = h2_fifo_try_peek(workers->mplxs, mplx_peek, slot);
- if (status == APR_EOF) {
- return status;
- }
+
+ while (!workers->aborted) {
+ ap_assert(slot->task == NULL);
+ if (h2_fifo_try_peek(workers->mplxs, mplx_peek, slot) == APR_EOF) {
+ /* The queue is terminated with the MPM child being cleaned up,
+ * just leave.
+ */
+ break;
}
-
if (slot->task) {
- return APR_SUCCESS;
+ return 1;
}
- cleanup_zombies(workers);
+ join_zombies(workers);
apr_thread_mutex_lock(slot->lock);
- push_slot(&workers->idle, slot);
- apr_thread_cond_wait(slot->not_idle, slot->lock);
+ if (!workers->aborted) {
+ push_slot(&workers->idle, slot);
+ apr_thread_cond_wait(slot->not_idle, slot->lock);
+ }
apr_thread_mutex_unlock(slot->lock);
}
- return APR_EOF;
+
+ return 0;
}
static void slot_done(h2_slot *slot)
{
- push_slot(&(slot->workers->zombies), slot);
+ h2_workers *workers = slot->workers;
+
+ push_slot(&workers->zombies, slot);
+
+ /* If this worker is the last one exiting and the MPM child is stopping,
+ * unblock workers_pool_cleanup().
+ */
+ if (!apr_atomic_dec32(&workers->worker_count) && workers->aborted) {
+ apr_thread_mutex_lock(workers->lock);
+ apr_thread_cond_signal(workers->all_done);
+ apr_thread_mutex_unlock(workers->lock);
+ }
}
@@ -222,28 +232,28 @@
{
h2_slot *slot = wctx;
- while (!slot->aborted) {
-
- /* Get a h2_task from the mplxs queue. */
- get_next(slot);
- while (slot->task) {
-
+ /* Get the h2_task(s) from the ->mplxs queue. */
+ while (get_next(slot)) {
+ ap_assert(slot->task != NULL);
+ do {
h2_task_do(slot->task, thread, slot->id);
/* Report the task as done. If stickyness is left, offer the
* mplx the opportunity to give us back a new task right away.
*/
- if (!slot->aborted && (--slot->sticks > 0)) {
+ if (!slot->workers->aborted && --slot->sticks > 0) {
h2_mplx_s_task_done(slot->task->mplx, slot->task, &slot->task);
}
else {
h2_mplx_s_task_done(slot->task->mplx, slot->task, NULL);
slot->task = NULL;
}
- }
+ } while (slot->task);
}
slot_done(slot);
+
+ apr_thread_exit(thread, APR_SUCCESS);
return NULL;
}
@@ -252,30 +262,28 @@
h2_workers *workers = data;
h2_slot *slot;
- if (!workers->aborted) {
- workers->aborted = 1;
- /* abort all idle slots */
- for (;;) {
- slot = pop_slot(&workers->idle);
- if (slot) {
- apr_thread_mutex_lock(slot->lock);
- slot->aborted = 1;
- apr_thread_cond_signal(slot->not_idle);
- apr_thread_mutex_unlock(slot->lock);
- }
- else {
- break;
- }
- }
+ workers->aborted = 1;
+ h2_fifo_term(workers->mplxs);
- h2_fifo_term(workers->mplxs);
+ /* abort all idle slots */
+ while ((slot = pop_slot(&workers->idle))) {
+ apr_thread_mutex_lock(slot->lock);
+ apr_thread_cond_signal(slot->not_idle);
+ apr_thread_mutex_unlock(slot->lock);
+ }
- cleanup_zombies(workers);
+ /* wait for all the workers to become zombies and join them */
+ apr_thread_mutex_lock(workers->lock);
+ if (apr_atomic_read32(&workers->worker_count)) {
+ apr_thread_cond_wait(workers->all_done, workers->lock);
}
+ apr_thread_mutex_unlock(workers->lock);
+ join_zombies(workers);
+
return APR_SUCCESS;
}
-h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool,
+h2_workers *h2_workers_create(server_rec *s, apr_pool_t *pchild,
int min_workers, int max_workers,
int idle_secs)
{
@@ -285,14 +293,14 @@
int i, n;
ap_assert(s);
- ap_assert(server_pool);
+ ap_assert(pchild);
/* let's have our own pool that will be parent to all h2_worker
* instances we create. This happens in various threads, but always
* guarded by our lock. Without this pool, all subpool creations would
* happen on the pool handed to us, which we do not guard.
*/
- apr_pool_create(&pool, server_pool);
+ apr_pool_create(&pool, pchild);
apr_pool_tag(pool, "h2_workers");
workers = apr_pcalloc(pool, sizeof(h2_workers));
if (!workers) {
@@ -338,6 +346,9 @@
APR_THREAD_MUTEX_DEFAULT,
workers->pool);
if (status == APR_SUCCESS) {
+ status = apr_thread_cond_create(&workers->all_done, workers->pool);
+ }
+ if (status == APR_SUCCESS) {
n = workers->nslots = workers->max_workers;
workers->slots = apr_pcalloc(workers->pool, n * sizeof(h2_slot));
if (workers->slots == NULL) {
@@ -363,7 +374,12 @@
workers->dynamic = (workers->worker_count < workers->max_workers);
}
if (status == APR_SUCCESS) {
- apr_pool_pre_cleanup_register(pool, workers, workers_pool_cleanup);
+ /* Stop/join the workers threads when the MPM child exits (pchild is
+ * destroyed), and as a pre_cleanup of pchild thus before the threads
+ * pools (children of workers->pool) so that they are not destroyed
+ * before/under us.
+ */
+ apr_pool_pre_cleanup_register(pchild, workers, workers_pool_cleanup);
return workers;
}
return NULL;
--- a/modules/http2/h2_workers.h
+++ b/modules/http2/h2_workers.h
@@ -42,7 +42,7 @@
int max_workers;
int max_idle_secs;
- int aborted;
+ volatile int aborted;
int dynamic;
apr_threadattr_t *thread_attr;
@@ -58,6 +58,7 @@
struct h2_fifo *mplxs;
struct apr_thread_mutex_t *lock;
+ struct apr_thread_cond_t *all_done;
};
--- a/modules/http2/mod_http2.c
+++ b/modules/http2/mod_http2.c
@@ -180,15 +180,33 @@
/* Runs once per created child process. Perform any process
* related initionalization here.
*/
-static void h2_child_init(apr_pool_t *pool, server_rec *s)
+static void h2_child_init(apr_pool_t *pchild, server_rec *s)
{
+ apr_allocator_t *allocator;
+ apr_thread_mutex_t *mutex;
+ apr_status_t status;
+
+ /* The allocator of pchild has no mutex with MPM prefork, but we need one
+ * for h2 workers threads synchronization. Even though mod_http2 shouldn't
+ * be used with prefork, better be safe than sorry, so forcibly set the
+ * mutex here. For MPM event/worker, pchild has no allocator so pconf's
+ * is used, with its mutex.
+ */
+ allocator = apr_pool_allocator_get(pchild);
+ if (allocator) {
+ mutex = apr_allocator_mutex_get(allocator);
+ if (!mutex) {
+ apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, pchild);
+ apr_allocator_mutex_set(allocator, mutex);
+ }
+ }
+
/* Set up our connection processing */
- apr_status_t status = h2_conn_child_init(pool, s);
+ status = h2_conn_child_init(pchild, s);
if (status != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_ERR, status, s,
APLOGNO(02949) "initializing connection handling");
}
-
}
/* Install this module into the apache2 infrastructure.
--- a/modules/http2/mod_proxy_http2.c
+++ b/modules/http2/mod_proxy_http2.c
@@ -425,7 +425,7 @@
ctx->p_conn = NULL;
}
++reconnects;
- if (reconnects < 5) {
+ if (reconnects < 2) {
goto run_connect;
}
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(10023)
Reply to: