BUG/MINOR: h3: properly handle alloc failure on finalize
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 15 Dec 2023 16:32:06 +0000 (17:32 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Tue, 2 Jan 2024 07:28:03 +0000 (08:28 +0100)
If H3 control stream Tx buffer cannot be allocated, return a proper
errur through h3_finalize(). This will cause the emission of a
CONNECTION_CLOSE with error H3_INTERNAL_ERROR and closure of the whole
connection.

This should be backported up to 2.6. Note that 2.9 has some difference
which will cause conflict. The main one is that qcc_get_stream_txbuf()
does not exist in this version. Instead the check in h3_control_send()
should be made after mux_get_buf(). Finally, it may be useful to first
pick previous commit (MINOR: h3: add traces for connection init stage)
to improve context similarity.

(cherry picked from commit 7a3602a1f55dacda5a669865e04474f5d503bab7)
[cf: H3_EV_TX_FRAME removed from trace messages because it does not exist]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

src/h3.c

index 977e9ec..5803909 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1428,7 +1428,11 @@ static struct buffer *mux_get_buf(struct qcs *qcs)
        return &qcs->tx.buf;
 }
 
-/* Function used to emit stream data from <qcs> control uni-stream */
+/* Function used to emit stream data from <qcs> control uni-stream.
+ *
+ * On success return the number of sent bytes. A negative code is used on
+ * error.
+ */
 static int h3_control_send(struct qcs *qcs, void *ctx)
 {
        int ret;
@@ -1467,6 +1471,11 @@ static int h3_control_send(struct qcs *qcs, void *ctx)
        }
 
        res = mux_get_buf(qcs);
+       if (!res) {
+               TRACE_ERROR("cannot allocate Tx buffer", H3_EV_TX_SETTINGS, qcs->qcc->conn, qcs);
+               goto err;
+       }
+
        if (b_room(res) < b_data(&pos)) {
                // TODO the mux should be put in blocked state, with
                // the stream in state waiting for settings to be sent
@@ -1482,6 +1491,10 @@ static int h3_control_send(struct qcs *qcs, void *ctx)
 
        TRACE_LEAVE(H3_EV_TX_SETTINGS, qcs->qcc->conn, qcs);
        return ret;
+
+ err:
+       TRACE_DEVEL("leaving on error", H3_EV_TX_SETTINGS, qcs->qcc->conn, qcs);
+       return -1;
 }
 
 static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
@@ -1522,7 +1535,7 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
                }
                else if (type == HTX_BLK_HDR) {
                        if (unlikely(hdr >= sizeof(list) / sizeof(list[0]) - 1)) {
-                               TRACE_ERROR("too many headers", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
+                               TRACE_ERROR("too many headers", H3_EV_TX_HDR, qcs->qcc->conn, qcs);
                                h3c->err = H3_INTERNAL_ERROR;
                                goto err;
                        }
@@ -1652,7 +1665,7 @@ static int h3_resp_trailers_send(struct qcs *qcs, struct htx *htx)
 
                if (type == HTX_BLK_TLR) {
                        if (unlikely(hdr >= sizeof(list) / sizeof(list[0]) - 1)) {
-                               TRACE_ERROR("too many headers", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
+                               TRACE_ERROR("too many headers", H3_EV_TX_HDR, qcs->qcc->conn, qcs);
                                h3c->err = H3_INTERNAL_ERROR;
                                goto err;
                        }
@@ -2149,9 +2162,11 @@ static int h3_finalize(void *ctx)
                goto err;
        }
 
-       h3_control_send(qcs, h3c);
        h3c->ctrl_strm = qcs;
 
+       if (h3_control_send(qcs, h3c) < 0)
+               goto err;
+
        TRACE_LEAVE(H3_EV_H3C_NEW, qcc->conn);
        return 0;