]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: mux-quic: use qcc_release in case of init failure
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Mon, 18 Dec 2023 18:12:32 +0000 (19:12 +0100)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 20 Dec 2023 14:27:11 +0000 (15:27 +0100)
qmux_init() may fail at different stage. In this case, an error is
returned and QCC allocated element are freed. Previously, extra care was
taken using different label to only liberate already allocate elements.

This patch removes the multi label and uses qcc_release(). This will be
simpler to ensure a QCC is always properly freed. The only important
thing is to ensure that mandatory fields are properly initialized to
NULL or equivalent to be able to use qcc_release() safely.

src/mux_quic.c

index f276d7776a34d81941ed49ee777010c21ebd9c28..9585bb4cc147bc91958d064bedf1cb159721a652 100644 (file)
@@ -2523,6 +2523,17 @@ static struct task *qcc_timeout_task(struct task *t, void *ctx, unsigned int sta
        return t;
 }
 
+/* Minimal initialization of <qcc> members to use qcc_release() safely. */
+static void _qcc_init(struct qcc *qcc)
+{
+       qcc->conn = NULL;
+       qcc->task = NULL;
+       qcc->wait_event.tasklet = NULL;
+       qcc->app_ops = NULL;
+       qcc->streams_by_id = EB_ROOT_UNIQUE;
+       LIST_INIT(&qcc->lfctl.frms);
+}
+
 static int qmux_init(struct connection *conn, struct proxy *prx,
                      struct session *sess, struct buffer *input)
 {
@@ -2534,24 +2545,19 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
        qcc = pool_alloc(pool_head_qcc);
        if (!qcc) {
                TRACE_ERROR("alloc failure", QMUX_EV_QCC_NEW);
-               goto fail_no_qcc;
+               goto err;
        }
 
-       qcc->conn = conn;
+       _qcc_init(qcc);
        conn->ctx = qcc;
        qcc->nb_hreq = qcc->nb_sc = 0;
        qcc->flags = 0;
 
-       qcc->app_ops = NULL;
-
-       qcc->streams_by_id = EB_ROOT_UNIQUE;
-
        /* Server parameters, params used for RX flow control. */
        lparams = &conn->handle.qc->rx.params;
 
        qcc->tx.sent_offsets = qcc->tx.offsets = 0;
 
-       LIST_INIT(&qcc->lfctl.frms);
        qcc->lfctl.ms_bidi = qcc->lfctl.ms_bidi_init = lparams->initial_max_streams_bidi;
        qcc->lfctl.ms_uni = lparams->initial_max_streams_uni;
        qcc->lfctl.msd_bidi_l = lparams->initial_max_stream_data_bidi_local;
@@ -2584,7 +2590,7 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
        qcc->wait_event.tasklet = tasklet_new();
        if (!qcc->wait_event.tasklet) {
                TRACE_ERROR("taslket alloc failure", QMUX_EV_QCC_NEW);
-               goto fail_no_tasklet;
+               goto err;
        }
 
        LIST_INIT(&qcc->send_list);
@@ -2595,7 +2601,7 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
 
        qcc->proxy = prx;
        /* haproxy timeouts */
-       if (conn_is_back(qcc->conn)) {
+       if (conn_is_back(conn)) {
                qcc->timeout = prx->timeout.server;
                qcc->shut_timeout = tick_isset(prx->timeout.serverfin) ?
                                    prx->timeout.serverfin : prx->timeout.server;
@@ -2612,7 +2618,7 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
        qcc->task = task_new_here();
        if (!qcc->task) {
                TRACE_ERROR("timeout task alloc failure", QMUX_EV_QCC_NEW);
-               goto fail_no_timeout_task;
+               goto err;
        }
        qcc->task->process = qcc_timeout_task;
        qcc->task->context = qcc;
@@ -2623,11 +2629,14 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
 
        HA_ATOMIC_STORE(&conn->handle.qc->qcc, qcc);
 
+       /* Register conn as app_ops may use it. */
+       qcc->conn = conn;
+
        if (qcc_install_app_ops(qcc, conn->handle.qc->app_ops)) {
-               TRACE_PROTO("Cannot install app layer", QMUX_EV_QCC_NEW|QMUX_EV_QCC_ERR, qcc->conn);
+               TRACE_PROTO("Cannot install app layer", QMUX_EV_QCC_NEW|QMUX_EV_QCC_ERR, conn);
                /* prepare a CONNECTION_CLOSE frame */
                quic_set_connection_close(conn->handle.qc, quic_err_transport(QC_ERR_APPLICATION_ERROR));
-               goto fail_install_app_ops;
+               goto err;
        }
 
        if (qcc->app_ops == &h3_ops)
@@ -2640,19 +2649,17 @@ static int qmux_init(struct connection *conn, struct proxy *prx,
        /* init read cycle */
        tasklet_wakeup(qcc->wait_event.tasklet);
 
-       TRACE_LEAVE(QMUX_EV_QCC_NEW, qcc->conn);
+       TRACE_LEAVE(QMUX_EV_QCC_NEW, conn);
        return 0;
 
- fail_install_app_ops:
-       if (qcc->app_ops && qcc->app_ops->release)
-               qcc->app_ops->release(qcc->ctx);
-       task_destroy(qcc->task);
- fail_no_timeout_task:
-       tasklet_free(qcc->wait_event.tasklet);
- fail_no_tasklet:
-       pool_free(pool_head_qcc, qcc);
- fail_no_qcc:
-       TRACE_LEAVE(QMUX_EV_QCC_NEW);
+ err:
+       if (qcc) {
+               /* In case of MUX init failure, session will ensure connection is freed. */
+               qcc->conn = NULL;
+               qcc_release(qcc);
+       }
+
+       TRACE_DEVEL("leaving on error", QMUX_EV_QCC_NEW, conn);
        return -1;
 }