From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Mon, 18 Dec 2023 18:12:32 +0000 (+0100)
Subject: MINOR: mux-quic: use qcc_release in case of init failure
X-Git-Tag: v3.0-dev1~78
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bcade776c2053e2c2a60301f4a89c93fd1654af1;p=thirdparty%2Fhaproxy.git

MINOR: mux-quic: use qcc_release in case of init failure

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.
---

diff --git a/src/mux_quic.c b/src/mux_quic.c
index f276d7776a..9585bb4cc1 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -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;
 }