http: consolidate nghttp2_session_mem_recv() call paths
Previously there were several locations that called nghttp2_session_mem_recv and handled responses slightly differently. Those have been converted to call the existing h2_process_pending_input() function. Moved the end-of-session check to h2_process_pending_input() since the only place the end-of-session state can change is after nghttp2 processes additional input frames. This will likely fix the fuzzing error. While I don't have a root cause the out-of-bounds read seems like a use after free, so moving the nghttp2_session_check_request_allowed() call to a location with a guaranteed nghttp2 session seems reasonable. Also updated a few nghttp2 callsites to include error messages and added a few additional error checks. Closes #5648
This commit is contained in:
Laramie Leavitt
Daniel Stenberg
Родитель
4ba275a46a
Коммит
25a25f45ae
117
lib/http2.c
117
lib/http2.c
|
|
@ -1207,13 +1207,6 @@ void Curl_http2_done(struct Curl_easy *data, bool premature)
|
|||
}
|
||||
http->stream_id = 0;
|
||||
}
|
||||
|
||||
if(0 == nghttp2_session_check_request_allowed(httpc->h2)) {
|
||||
/* No more requests are allowed in the current session, so the connection
|
||||
may not be reused. This is set when a GOAWAY frame has been received or
|
||||
when the limit of stream identifiers has been reached. */
|
||||
connclose(data->conn, "http/2: No new requests allowed");
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -1288,7 +1281,7 @@ CURLcode Curl_http2_request_upgrade(struct dynbuf *req,
|
|||
binlen = nghttp2_pack_settings_payload(binsettings, H2_BINSETTINGS_LEN,
|
||||
httpc->local_settings,
|
||||
httpc->local_settings_num);
|
||||
if(!binlen) {
|
||||
if(binlen <= 0) {
|
||||
failf(conn->data, "nghttp2 unexpectedly failed on pack_settings_payload");
|
||||
Curl_dyn_free(req);
|
||||
return CURLE_FAILED_INIT;
|
||||
|
|
@ -1371,6 +1364,14 @@ static int h2_process_pending_input(struct connectdata *conn,
|
|||
return -1;
|
||||
}
|
||||
|
||||
if(nghttp2_session_check_request_allowed(httpc->h2) == 0) {
|
||||
/* No more requests are allowed in the current session, so
|
||||
the connection may not be reused. This is set when a
|
||||
GOAWAY frame has been received or when the limit of stream
|
||||
identifiers has been reached. */
|
||||
connclose(conn, "http/2: No new requests allowed");
|
||||
}
|
||||
|
||||
if(should_close_session(httpc)) {
|
||||
H2BUGF(infof(data,
|
||||
"h2_process_pending_input: nothing to do in this session\n"));
|
||||
|
|
@ -1383,7 +1384,6 @@ static int h2_process_pending_input(struct connectdata *conn,
|
|||
}
|
||||
return -1;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -1564,8 +1564,6 @@ static int h2_session_send(struct Curl_easy *data,
|
|||
static ssize_t http2_recv(struct connectdata *conn, int sockindex,
|
||||
char *mem, size_t len, CURLcode *err)
|
||||
{
|
||||
CURLcode result = CURLE_OK;
|
||||
ssize_t rv;
|
||||
ssize_t nread;
|
||||
struct http_conn *httpc = &conn->proto.httpc;
|
||||
struct Curl_easy *data = conn->data;
|
||||
|
|
@ -1632,8 +1630,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
|
|||
/* We have paused nghttp2, but we have no pause data (see
|
||||
on_data_chunk_recv). */
|
||||
httpc->pause_stream_id = 0;
|
||||
if(h2_process_pending_input(conn, httpc, &result) != 0) {
|
||||
*err = result;
|
||||
if(h2_process_pending_input(conn, httpc, err) != 0) {
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
|
@ -1661,8 +1658,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
|
|||
frames, then we have to call it again with 0-length data.
|
||||
Without this, on_stream_close callback will not be called,
|
||||
and stream could be hanged. */
|
||||
if(h2_process_pending_input(conn, httpc, &result) != 0) {
|
||||
*err = result;
|
||||
if(h2_process_pending_input(conn, httpc, err) != 0) {
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
|
@ -1688,7 +1684,6 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
|
|||
return -1;
|
||||
}
|
||||
else {
|
||||
char *inbuf;
|
||||
/* remember where to store incoming data for this stream and how big the
|
||||
buffer is */
|
||||
stream->mem = mem;
|
||||
|
|
@ -1697,16 +1692,15 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
|
|||
|
||||
if(httpc->inbuflen == 0) {
|
||||
nread = ((Curl_recv *)httpc->recv_underlying)(
|
||||
conn, FIRSTSOCKET, httpc->inbuf, H2_BUFSIZE, &result);
|
||||
conn, FIRSTSOCKET, httpc->inbuf, H2_BUFSIZE, err);
|
||||
|
||||
if(nread == -1) {
|
||||
if(result != CURLE_AGAIN)
|
||||
if(*err != CURLE_AGAIN)
|
||||
failf(data, "Failed receiving HTTP2 data");
|
||||
else if(stream->closed)
|
||||
/* received when the stream was already closed! */
|
||||
return http2_handle_stream_close(conn, data, stream, err);
|
||||
|
||||
*err = result;
|
||||
return -1;
|
||||
}
|
||||
|
||||
|
|
@ -1719,47 +1713,18 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
|
|||
H2BUGF(infof(data, "nread=%zd\n", nread));
|
||||
|
||||
httpc->inbuflen = nread;
|
||||
inbuf = httpc->inbuf;
|
||||
|
||||
DEBUGASSERT(httpc->nread_inbuf == 0);
|
||||
}
|
||||
else {
|
||||
nread = httpc->inbuflen - httpc->nread_inbuf;
|
||||
inbuf = httpc->inbuf + httpc->nread_inbuf;
|
||||
|
||||
(void)nread; /* silence warning, used in debug */
|
||||
H2BUGF(infof(data, "Use data left in connection buffer, nread=%zd\n",
|
||||
nread));
|
||||
}
|
||||
rv = nghttp2_session_mem_recv(httpc->h2, (const uint8_t *)inbuf, nread);
|
||||
|
||||
if(nghttp2_is_fatal((int)rv)) {
|
||||
failf(data, "nghttp2_session_mem_recv() returned %zd:%s\n",
|
||||
rv, nghttp2_strerror((int)rv));
|
||||
*err = CURLE_RECV_ERROR;
|
||||
if(h2_process_pending_input(conn, httpc, err) != 0)
|
||||
return -1;
|
||||
}
|
||||
H2BUGF(infof(data, "nghttp2_session_mem_recv() returns %zd\n", rv));
|
||||
if(nread == rv) {
|
||||
H2BUGF(infof(data, "All data in connection buffer processed\n"));
|
||||
httpc->inbuflen = 0;
|
||||
httpc->nread_inbuf = 0;
|
||||
}
|
||||
else {
|
||||
httpc->nread_inbuf += rv;
|
||||
H2BUGF(infof(data, "%zu bytes left in connection buffer\n",
|
||||
httpc->inbuflen - httpc->nread_inbuf));
|
||||
}
|
||||
/* Always send pending frames in nghttp2 session, because
|
||||
nghttp2_session_mem_recv() may queue new frame */
|
||||
rv = h2_session_send(data, httpc->h2);
|
||||
if(rv != 0) {
|
||||
*err = CURLE_SEND_ERROR;
|
||||
return -1;
|
||||
}
|
||||
|
||||
if(should_close_session(httpc)) {
|
||||
H2BUGF(infof(data, "http2_recv: nothing to do in this session\n"));
|
||||
*err = CURLE_HTTP2;
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
if(stream->memlen) {
|
||||
ssize_t retlen = stream->memlen;
|
||||
|
|
@ -2112,7 +2077,7 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
|
|||
h2_pri_spec(conn->data, &pri_spec);
|
||||
|
||||
H2BUGF(infof(conn->data, "http2_send request allowed %d (easy handle %p)\n",
|
||||
nghttp2_session_check_request_allowed(h2), (void *)conn->data));
|
||||
nghttp2_session_check_request_allowed(h2), (void *)conn->data));
|
||||
|
||||
switch(conn->data->state.httpreq) {
|
||||
case HTTPREQ_POST:
|
||||
|
|
@ -2138,7 +2103,9 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
|
|||
Curl_safefree(nva);
|
||||
|
||||
if(stream_id < 0) {
|
||||
H2BUGF(infof(conn->data, "http2_send() send error\n"));
|
||||
H2BUGF(infof(conn->data,
|
||||
"http2_send() nghttp2_submit_request error (%s)%d\n",
|
||||
nghttp2_strerror(stream_id), stream_id));
|
||||
*err = CURLE_SEND_ERROR;
|
||||
return -1;
|
||||
}
|
||||
|
|
@ -2148,10 +2115,13 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
|
|||
stream->stream_id = stream_id;
|
||||
|
||||
/* this does not call h2_session_send() since there can not have been any
|
||||
* priority upodate since the nghttp2_submit_request() call above */
|
||||
* priority update since the nghttp2_submit_request() call above */
|
||||
rv = nghttp2_session_send(h2);
|
||||
|
||||
if(rv != 0) {
|
||||
H2BUGF(infof(conn->data,
|
||||
"http2_send() nghttp2_session_send error (%s)%d\n",
|
||||
nghttp2_strerror(rv), rv));
|
||||
|
||||
*err = CURLE_SEND_ERROR;
|
||||
return -1;
|
||||
}
|
||||
|
|
@ -2236,7 +2206,6 @@ CURLcode Curl_http2_switched(struct connectdata *conn,
|
|||
CURLcode result;
|
||||
struct http_conn *httpc = &conn->proto.httpc;
|
||||
int rv;
|
||||
ssize_t nproc;
|
||||
struct Curl_easy *data = conn->data;
|
||||
struct HTTP *stream = conn->data->req.protop;
|
||||
|
||||
|
|
@ -2310,41 +2279,13 @@ CURLcode Curl_http2_switched(struct connectdata *conn,
|
|||
|
||||
if(nread)
|
||||
memcpy(httpc->inbuf, mem, nread);
|
||||
|
||||
httpc->inbuflen = nread;
|
||||
|
||||
nproc = nghttp2_session_mem_recv(httpc->h2, (const uint8_t *)httpc->inbuf,
|
||||
httpc->inbuflen);
|
||||
DEBUGASSERT(httpc->nread_inbuf == 0);
|
||||
|
||||
if(nghttp2_is_fatal((int)nproc)) {
|
||||
failf(data, "nghttp2_session_mem_recv() failed: %s(%d)",
|
||||
nghttp2_strerror((int)nproc), (int)nproc);
|
||||
if(-1 == h2_process_pending_input(conn, httpc, &result))
|
||||
return CURLE_HTTP2;
|
||||
}
|
||||
|
||||
H2BUGF(infof(data, "nghttp2_session_mem_recv() returns %zd\n", nproc));
|
||||
|
||||
if((ssize_t)nread == nproc) {
|
||||
httpc->inbuflen = 0;
|
||||
httpc->nread_inbuf = 0;
|
||||
}
|
||||
else {
|
||||
httpc->nread_inbuf += nproc;
|
||||
}
|
||||
|
||||
/* Try to send some frames since we may read SETTINGS already. */
|
||||
rv = h2_session_send(data, httpc->h2);
|
||||
|
||||
if(rv != 0) {
|
||||
failf(data, "nghttp2_session_send() failed: %s(%d)",
|
||||
nghttp2_strerror(rv), rv);
|
||||
return CURLE_HTTP2;
|
||||
}
|
||||
|
||||
if(should_close_session(httpc)) {
|
||||
H2BUGF(infof(data,
|
||||
"nghttp2_session_send(): nothing to do in this session\n"));
|
||||
return CURLE_HTTP2;
|
||||
}
|
||||
|
||||
return CURLE_OK;
|
||||
}
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче