From 6ff13aa4cd2086fb284234f7584599bfad3ecadb Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Fri, 22 Dec 2023 09:00:13 +0100 Subject: [PATCH] BUG/MINOR: h3: close connection on sending alloc errors 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 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 ) 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. (cherry picked from commit 2144d2418651c1f76b91cc1f6e745feecdefcb00) [cf: H3_EV_TX_FRAME removed from trace messages because it does not exist] Signed-off-by: Christopher Faulet --- src/h3.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/h3.c b/src/h3.c index 5803909..099f123 100644 --- a/src/h3.c +++ b/src/h3.c @@ -1554,6 +1554,11 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx) list[hdr].n = ist(""); res = mux_get_buf(qcs); + if (!res) { + TRACE_ERROR("cannot allocate Tx buffer", 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) @@ -1625,7 +1630,7 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx) err: TRACE_DEVEL("leaving on error", H3_EV_TX_HDR, qcs->qcc->conn, qcs); - return 0; + return -1; } /* Convert a series of HTX trailer blocks from buffer into buffer @@ -1690,6 +1695,11 @@ static int h3_resp_trailers_send(struct qcs *qcs, struct htx *htx) list[hdr].n = ist(""); res = mux_get_buf(qcs); + if (!res) { + TRACE_ERROR("cannot allocate Tx buffer", 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) { @@ -1763,13 +1773,15 @@ static int h3_resp_trailers_send(struct qcs *qcs, struct htx *htx) err: TRACE_DEVEL("leaving on error", H3_EV_TX_HDR, qcs->qcc->conn, qcs); - return 0; + return -1; } /* Returns the total of bytes sent. */ static int h3_resp_data_send(struct qcs *qcs, struct buffer *buf, size_t count) { struct htx *htx; + struct h3s *h3s = qcs->ctx; + struct h3c *h3c = h3s->h3c; struct buffer outbuf; struct buffer *res; size_t total = 0; @@ -1796,6 +1808,11 @@ static int h3_resp_data_send(struct qcs *qcs, struct buffer *buf, size_t count) goto end; res = mux_get_buf(qcs); + if (!res) { + TRACE_ERROR("cannot allocate Tx buffer", H3_EV_TX_DATA, qcs->qcc->conn, qcs); + h3c->err = H3_INTERNAL_ERROR; + goto err; + } if (unlikely(fsize == count && !b_data(res) && @@ -1834,8 +1851,9 @@ static int h3_resp_data_send(struct qcs *qcs, struct buffer *buf, size_t count) * on SEND. */ if (b_size(&outbuf) <= hsize) { + TRACE_STATE("not enough room for data 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) @@ -1861,6 +1879,10 @@ static int h3_resp_data_send(struct qcs *qcs, struct buffer *buf, size_t count) end: TRACE_LEAVE(H3_EV_TX_DATA, qcs->qcc->conn, qcs); return total; + + err: + TRACE_DEVEL("leaving on error", 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) @@ -1873,7 +1895,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__); @@ -1933,6 +1955,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. */ -- 1.7.10.4