RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-03-11 05:48:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MINOR: h3: close connection on sending alloc errors

Browse Source 
When encoding new HTTP/3 frames, QCS Tx buffer must be allocated if
currently NULL. Previously, allocation failure was not properly checked,
leaving the connection in an unspecified state, or worse risking a
crash.

Fix this by setting <h3c.err> to H3_INTERNAL_ERROR each time the
allocation fails. This will stop sending and close the connection. In
the future, it may be better to put the connection on pause waiting for
allocation to succeed but this is too complicated to implement for now
in a reliable way.

Along with the current change, return of all HTX parsing functions
(h3_resp_*_send) were set to a negative value in case of error. A new
BUG_ON() in h3_snd_buf() ensures that if such a value is returned,
either a connection error is register (via <h3c.err>) or buffer is
temporarily full (flag QC_SF_BLK_MROOM).

This should fix github issue #2389.

This should be backported up to 2.6. Note that qcc_get_stream_txbuf()
does not exist in 2.9 and below. mux_get_buf() is its equivalent. An
explicit check b_is_null(&qcs.tx.buf) should be used there.
This commit is contained in:
Amaury Denoyelle 2023-12-22 09:00:13 +01:00
parent d077f7ccf4
commit 2144d24186
1 changed files with 27 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/h3.c
View File

@ -1542,8 +1542,11 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
list[hdr].n = ist("");
if (!(res = qcc_get_stream_txbuf(qcs)))
if (!(res = qcc_get_stream_txbuf(qcs))) {
TRACE_ERROR("cannot allocate Tx buffer", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
h3c->err = H3_INTERNAL_ERROR;
goto err;
}
/* At least 5 bytes to store frame type + length as a varint max size */
if (b_room(res) < 5)
@ -1617,7 +1620,7 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
err:
TRACE_DEVEL("leaving on error", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
return 0;
return -1;
}
/* Convert a series of HTX trailer blocks from <htx> buffer into <qcs> buffer
@ -1681,8 +1684,11 @@ static int h3_resp_trailers_send(struct qcs *qcs, struct htx *htx)
}
list[hdr].n = ist("");
if (!(res = qcc_get_stream_txbuf(qcs)))
if (!(res = qcc_get_stream_txbuf(qcs))) {
TRACE_ERROR("cannot allocate Tx buffer", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
h3c->err = H3_INTERNAL_ERROR;
goto err;
}
/* At least 9 bytes to store frame type + length as a varint max size */
if (b_room(res) < 9) {
@ -1758,7 +1764,7 @@ static int h3_resp_trailers_send(struct qcs *qcs, struct htx *htx)
err:
TRACE_DEVEL("leaving on error", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
return 0;
return -1;
}
/* Convert a series of HTX data blocks from <htx> buffer of size <count> into
@ -1772,6 +1778,8 @@ static int h3_resp_trailers_send(struct qcs *qcs, struct htx *htx)
static int h3_resp_data_send(struct qcs *qcs, struct htx *htx,
struct buffer *buf, size_t count)
{
struct h3s *h3s = qcs->ctx;
struct h3c *h3c = h3s->h3c;
struct buffer outbuf;
struct buffer *res;
size_t total = 0;
@ -1796,7 +1804,9 @@ static int h3_resp_data_send(struct qcs *qcs, struct htx *htx,
goto end;
if (!(res = qcc_get_stream_txbuf(qcs))) {
/* TODO */
TRACE_ERROR("cannot allocate Tx buffer", H3_EV_TX_FRAME|H3_EV_TX_DATA, qcs->qcc->conn, qcs);
h3c->err = H3_INTERNAL_ERROR;
goto err;
}
/* If HTX contains only one DATA block, try to exchange it with MUX
@ -1844,8 +1854,9 @@ static int h3_resp_data_send(struct qcs *qcs, struct htx *htx,
* on SEND.
*/
if (b_size(&outbuf) <= hsize) {
TRACE_STATE("not enough room for data frame", H3_EV_TX_FRAME|H3_EV_TX_DATA, qcs->qcc->conn, qcs);
qcs->flags |= QC_SF_BLK_MROOM;
goto end;
goto err;
}
if (b_size(&outbuf) < hsize + fsize)
@ -1873,6 +1884,10 @@ static int h3_resp_data_send(struct qcs *qcs, struct htx *htx,
end:
TRACE_LEAVE(H3_EV_TX_FRAME|H3_EV_TX_DATA, qcs->qcc->conn, qcs);
return total;
err:
TRACE_DEVEL("leaving on error", H3_EV_TX_FRAME|H3_EV_TX_DATA, qcs->qcc->conn, qcs);
return -1;
}
static size_t h3_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count)
@ -1885,7 +1900,7 @@ static size_t h3_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count)
struct htx_blk *blk;
uint32_t bsize;
int32_t idx;
int ret;
int ret = 0;
h3_debug_printf(stderr, "%s\n", __func__);
@ -1944,6 +1959,11 @@ static size_t h3_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count)
count -= bsize;
break;
}
/* If an error occured, either buffer space or connection error
* must be set to break current loop.
*/
BUG_ON(ret < 0 && !(qcs->flags & QC_SF_BLK_MROOM) && !h3c->err);
}
/* Interrupt sending on connection error. */