You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ic...@apache.org on 2023/06/13 14:36:43 UTC
svn commit: r1910386 - in /httpd/httpd/trunk: changes-entries/h2_pr66646.txt modules/http2/h2_c1_io.c modules/http2/h2_c1_io.h modules/http2/h2_session.c modules/http2/h2_stream.c modules/http2/h2_stream.h modules/http2/h2_version.h
Author: icing
Date: Tue Jun 13 14:36:43 2023
New Revision: 1910386
URL: http://svn.apache.org/viewvc?rev=1910386&view=rev
Log:
*) mod_http2: fixed a bug that could lead to a crash in main connection
output handling. This occured only when the last request on a HTTP/2
connection had been processed and the session decided to shut down.
This could lead to an attempt to send a final GOAWAY while the previous
write was still in progress. See PR 66646.
Added:
httpd/httpd/trunk/changes-entries/h2_pr66646.txt
Modified:
httpd/httpd/trunk/modules/http2/h2_c1_io.c
httpd/httpd/trunk/modules/http2/h2_c1_io.h
httpd/httpd/trunk/modules/http2/h2_session.c
httpd/httpd/trunk/modules/http2/h2_stream.c
httpd/httpd/trunk/modules/http2/h2_stream.h
httpd/httpd/trunk/modules/http2/h2_version.h
Added: httpd/httpd/trunk/changes-entries/h2_pr66646.txt
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/h2_pr66646.txt?rev=1910386&view=auto
==============================================================================
--- httpd/httpd/trunk/changes-entries/h2_pr66646.txt (added)
+++ httpd/httpd/trunk/changes-entries/h2_pr66646.txt Tue Jun 13 14:36:43 2023
@@ -0,0 +1,6 @@
+ *) mod_http2: fixed a bug that could lead to a crash in main connection
+ output handling. This occured only when the last request on a HTTP/2
+ connection had been processed and the session decided to shut down.
+ This could lead to an attempt to send a final GOAWAY while the previous
+ write was still in progress. See PR 66646.
+ [Stefan Eissing]
Modified: httpd/httpd/trunk/modules/http2/h2_c1_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_c1_io.c?rev=1910386&r1=1910385&r2=1910386&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_c1_io.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_c1_io.c Tue Jun 13 14:36:43 2023
@@ -260,9 +260,22 @@ static apr_status_t read_to_scratch(h2_c
static apr_status_t pass_output(h2_c1_io *io, int flush)
{
conn_rec *c = io->session->c1;
- apr_off_t bblen;
+ apr_off_t bblen = 0;
apr_status_t rv;
-
+
+ if (io->is_passing) {
+ /* recursive call, may be triggered by an H2EOS bucket
+ * being destroyed and triggering sending more data? */
+ AP_DEBUG_ASSERT(0);
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO()
+ "h2_c1_io(%ld): recursive call of h2_c1_io_pass. "
+ "Denied to prevent output corruption. This "
+ "points to a bug in the HTTP/2 implementation.",
+ c->id);
+ return APR_EGENERAL;
+ }
+ io->is_passing = 1;
+
append_scratch(io);
if (flush) {
if (!APR_BUCKET_IS_FLUSH(APR_BRIGADE_LAST(io->output))) {
@@ -271,13 +284,14 @@ static apr_status_t pass_output(h2_c1_io
}
}
if (APR_BRIGADE_EMPTY(io->output)) {
- return APR_SUCCESS;
+ rv = APR_SUCCESS;
+ goto cleanup;
}
io->unflushed = !APR_BUCKET_IS_FLUSH(APR_BRIGADE_LAST(io->output));
apr_brigade_length(io->output, 0, &bblen);
C1_IO_BB_LOG(c, 0, APLOG_TRACE2, "out", io->output);
-
+
rv = ap_pass_brigade(c->output_filters, io->output);
if (APR_SUCCESS != rv) goto cleanup;
@@ -309,6 +323,7 @@ cleanup:
c->id, (long)bblen);
}
apr_brigade_cleanup(io->output);
+ io->is_passing = 0;
return rv;
}
Modified: httpd/httpd/trunk/modules/http2/h2_c1_io.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_c1_io.h?rev=1910386&r1=1910385&r2=1910386&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_c1_io.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_c1_io.h Tue Jun 13 14:36:43 2023
@@ -44,7 +44,8 @@ typedef struct {
apr_off_t buffered_len;
apr_off_t flush_threshold;
unsigned int is_flushed : 1;
-
+ unsigned int is_passing : 1;
+
char *scratch;
apr_size_t ssize;
apr_size_t slen;
Modified: httpd/httpd/trunk/modules/http2/h2_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1910386&r1=1910385&r2=1910386&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.c Tue Jun 13 14:36:43 2023
@@ -1642,10 +1642,6 @@ static void on_stream_state_enter(void *
h2_mplx_c1_stream_cleanup(session->mplx, stream, &session->open_streams);
++session->streams_done;
update_child_status(session, SERVER_BUSY_WRITE, "done", stream);
- if (session->open_streams == 0) {
- h2_session_dispatch_event(session, H2_SESSION_EV_NO_MORE_STREAMS,
- 0, "stream done");
- }
break;
default:
break;
Modified: httpd/httpd/trunk/modules/http2/h2_stream.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_stream.c?rev=1910386&r1=1910385&r2=1910386&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_stream.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_stream.c Tue Jun 13 14:36:43 2023
@@ -166,6 +166,7 @@ static int on_frame_recv(h2_stream_state
static int on_event(h2_stream* stream, h2_stream_event_t ev)
{
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (stream->monitor && stream->monitor->on_event) {
stream->monitor->on_event(stream->monitor->ctx, stream, ev);
}
@@ -392,6 +393,7 @@ void h2_stream_dispatch(h2_stream *strea
{
int new_state;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, stream->session->c1,
H2_STRM_MSG(stream, "dispatch event %d"), ev);
new_state = on_event(stream, ev);
@@ -425,6 +427,7 @@ apr_status_t h2_stream_send_frame(h2_str
apr_status_t status = APR_SUCCESS;
int new_state, eos = 0;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
new_state = on_frame_send(stream->state, ftype);
if (new_state < 0) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, stream->session->c1,
@@ -474,6 +477,7 @@ apr_status_t h2_stream_recv_frame(h2_str
apr_status_t status = APR_SUCCESS;
int new_state, eos = 0;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
new_state = on_frame_recv(stream->state, ftype);
if (new_state < 0) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, stream->session->c1,
@@ -528,6 +532,7 @@ apr_status_t h2_stream_recv_DATA(h2_stre
h2_session *session = stream->session;
apr_status_t status = APR_SUCCESS;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
stream->in_data_frames++;
if (len > 0) {
if (APLOGctrace3(session->c1)) {
@@ -548,11 +553,38 @@ apr_status_t h2_stream_recv_DATA(h2_stre
return status;
}
+#ifdef AP_DEBUG
+static apr_status_t stream_pool_destroy(void *data)
+{
+ h2_stream *stream = data;
+ switch (stream->magic) {
+ case H2_STRM_MAGIC_OK:
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, stream->session->c1,
+ H2_STRM_MSG(stream, "was not destroyed explicitly"));
+ AP_DEBUG_ASSERT(0);
+ break;
+ case H2_STRM_MAGIC_SDEL:
+ /* stream has been explicitly destroyed, as it should */
+ H2_STRM_ASSIGN_MAGIC(stream, H2_STRM_MAGIC_PDEL);
+ break;
+ case H2_STRM_MAGIC_PDEL:
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, stream->session->c1,
+ H2_STRM_MSG(stream, "already pool destroyed"));
+ AP_DEBUG_ASSERT(0);
+ break;
+ default:
+ AP_DEBUG_ASSERT(0);
+ }
+ return APR_SUCCESS;
+}
+#endif
+
h2_stream *h2_stream_create(int id, apr_pool_t *pool, h2_session *session,
h2_stream_monitor *monitor, int initiated_on)
{
h2_stream *stream = apr_pcalloc(pool, sizeof(h2_stream));
-
+
+ H2_STRM_ASSIGN_MAGIC(stream, H2_STRM_MAGIC_OK);
stream->id = id;
stream->initiated_on = initiated_on;
stream->created = apr_time_now();
@@ -560,6 +592,12 @@ h2_stream *h2_stream_create(int id, apr_
stream->pool = pool;
stream->session = session;
stream->monitor = monitor;
+#ifdef AP_DEBUG
+ if (id) { /* stream 0 has special lifetime */
+ apr_pool_cleanup_register(pool, stream, stream_pool_destroy,
+ apr_pool_cleanup_null);
+ }
+#endif
#ifdef H2_NG2_LOCAL_WIN_SIZE
if (id) {
@@ -581,6 +619,7 @@ void h2_stream_cleanup(h2_stream *stream
* end of the in/out notifications get closed.
*/
ap_assert(stream);
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (stream->out_buffer) {
apr_brigade_cleanup(stream->out_buffer);
}
@@ -589,13 +628,16 @@ void h2_stream_cleanup(h2_stream *stream
void h2_stream_destroy(h2_stream *stream)
{
ap_assert(stream);
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, stream->session->c1,
H2_STRM_MSG(stream, "destroy"));
+ H2_STRM_ASSIGN_MAGIC(stream, H2_STRM_MAGIC_SDEL);
apr_pool_destroy(stream->pool);
}
void h2_stream_rst(h2_stream *stream, int error_code)
{
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
stream->rst_error = error_code;
if (stream->c2) {
h2_c2_abort(stream->c2, stream->session->c1);
@@ -611,6 +653,7 @@ apr_status_t h2_stream_set_request_rec(h
h2_request *req;
apr_status_t status;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
ap_assert(stream->request == NULL);
ap_assert(stream->rtmp == NULL);
if (stream->rst_error) {
@@ -632,6 +675,7 @@ apr_status_t h2_stream_set_request_rec(h
void h2_stream_set_request(h2_stream *stream, const h2_request *r)
{
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
ap_assert(stream->request == NULL);
ap_assert(stream->rtmp == NULL);
stream->rtmp = h2_request_clone(stream->pool, r);
@@ -691,6 +735,7 @@ apr_status_t h2_stream_add_header(h2_str
int error = 0, was_added = 0;
apr_status_t status = APR_SUCCESS;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (stream->response) {
return APR_EINVAL;
}
@@ -797,6 +842,7 @@ apr_status_t h2_stream_end_headers(h2_st
int is_http_or_https;
h2_request *req = stream->rtmp;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
status = h2_request_end_headers(req, stream->pool, raw_bytes);
if (APR_SUCCESS != status || req->http_status != H2_HTTP_STATUS_UNSET) {
goto cleanup;
@@ -1045,6 +1091,7 @@ apr_status_t h2_stream_read_to(h2_stream
{
apr_status_t rv = APR_SUCCESS;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (stream->rst_error) {
return APR_ECONNRESET;
}
@@ -1139,6 +1186,7 @@ apr_status_t h2_stream_submit_pushes(h2_
apr_array_header_t *pushes;
int i;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
pushes = h2_push_collect_update(stream, stream->request, response);
if (pushes && !apr_is_empty_array(pushes)) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, stream->session->c1,
@@ -1158,6 +1206,7 @@ apr_status_t h2_stream_submit_pushes(h2_
apr_table_t *h2_stream_get_trailers(h2_stream *stream)
{
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
return NULL;
}
@@ -1169,6 +1218,7 @@ const h2_priority *h2_stream_get_priorit
h2_headers *response)
#endif
{
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (response && stream->initiated_on) {
const char *ctype = apr_table_get(response->headers, "content-type");
if (ctype) {
@@ -1182,6 +1232,7 @@ const h2_priority *h2_stream_get_priorit
int h2_stream_is_ready(h2_stream *stream)
{
/* Have we sent a response or do we have the response in our buffer? */
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (stream->response) {
return 1;
}
@@ -1193,11 +1244,13 @@ int h2_stream_is_ready(h2_stream *stream
int h2_stream_is_at(const h2_stream *stream, h2_stream_state_t state)
{
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
return stream->state == state;
}
int h2_stream_is_at_or_past(const h2_stream *stream, h2_stream_state_t state)
{
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
switch (state) {
case H2_SS_IDLE:
return 1; /* by definition */
@@ -1220,6 +1273,7 @@ apr_status_t h2_stream_in_consumed(h2_st
{
h2_session *session = stream->session;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (amount > 0) {
apr_off_t consumed = amount;
@@ -1345,6 +1399,7 @@ static ssize_t stream_data_cb(nghttp2_se
H2_SSSN_STRM_MSG(session, stream_id, "data_cb, stream not found"));
return NGHTTP2_ERR_CALLBACK_FAILURE;
}
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (!stream->output || !stream->response || !stream->out_buffer) {
return NGHTTP2_ERR_DEFERRED;
}
@@ -1476,6 +1531,7 @@ static apr_status_t stream_do_response(h
#endif
nghttp2_data_provider provider, *pprovider = NULL;
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
ap_assert(!stream->response);
ap_assert(stream->out_buffer);
@@ -1666,6 +1722,7 @@ void h2_stream_on_output_change(h2_strea
/* stream->pout_recv_write signalled a change. Check what has happend, read
* from it and act on seeing a response/data. */
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
if (!stream->output) {
/* c2 has not assigned the output beam to the stream (yet). */
ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c1,
@@ -1714,6 +1771,7 @@ void h2_stream_on_output_change(h2_strea
void h2_stream_on_input_change(h2_stream *stream)
{
+ H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
ap_assert(stream->input);
h2_beam_report_consumption(stream->input);
if (h2_stream_is_at(stream, H2_SS_CLOSED_L)
Modified: httpd/httpd/trunk/modules/http2/h2_stream.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_stream.h?rev=1910386&r1=1910385&r2=1910386&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_stream.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_stream.h Tue Jun 13 14:36:43 2023
@@ -63,7 +63,22 @@ typedef struct h2_stream_monitor {
trigger a state change */
} h2_stream_monitor;
+#ifdef AP_DEBUG
+#define H2_STRM_MAGIC_OK 0x5354524d
+#define H2_STRM_MAGIC_SDEL 0x5344454c
+#define H2_STRM_MAGIC_PDEL 0x5044454c
+
+#define H2_STRM_ASSIGN_MAGIC(s,m) ((s)->magic = m)
+#define H2_STRM_ASSERT_MAGIC(s,m) ap_assert((s)->magic == m)
+#else
+#define H2_STRM_ASSIGN_MAGIC(s,m) ((void)0)
+#define H2_STRM_ASSERT_MAGIC(s,m) ((void)0)
+#endif
+
struct h2_stream {
+#ifdef AP_DEBUG
+ uint32_t magic;
+#endif
int id; /* http2 stream identifier */
int initiated_on; /* initiating stream id (PUSH) or 0 */
apr_pool_t *pool; /* the memory pool for this stream */
Modified: httpd/httpd/trunk/modules/http2/h2_version.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_version.h?rev=1910386&r1=1910385&r2=1910386&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_version.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_version.h Tue Jun 13 14:36:43 2023
@@ -27,7 +27,7 @@
* @macro
* Version number of the http2 module as c string
*/
-#define MOD_HTTP2_VERSION "2.0.18-git"
+#define MOD_HTTP2_VERSION "2.0.19-git"
/**
* @macro
@@ -35,7 +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 0x020012
+#define MOD_HTTP2_VERSION_NUM 0x020013
#endif /* mod_h2_h2_version_h */