From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Mon, 13 Nov 2023 09:55:30 +0000 (+0100)
Subject: BUG/MINOR: quic: fix decrement of half_open counter on qc alloc failure
X-Git-Tag: v2.9-dev10~87
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=78d244e9f7788bdd719ececca1f7e9308416a3b4;p=thirdparty%2Fhaproxy.git

BUG/MINOR: quic: fix decrement of half_open counter on qc alloc failure

Half open counter is used to comptabilize QUIC connections waiting for
address validation. It was recently reworked to adjust its scope. With
each decrement operation, a BUG_ON() was added to ensure the counter
never wraps.

This BUG_ON() could be triggered if an allocation fails for one of
quic_conn members in qc_new_conn(). This is because half open counter is
incremented at the end of qc_new_conn(). However, in case of alloc
failure, quic_conn_release() is called immediately to ensure the counter
is decremented if a connection is freed before peer address has been
validated.

To fix this, increment half open counter early in qc_new_conn() prior to
every quic_conn members allocations.

This issue was reproduced using -dMfail argument.

This issue has been introduced by
  commit 278808915b05bab1ff1e08e95fe32be22109719f
  MINOR: quic: reduce half open counters scope

No need to backport.
---

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 1c71344848..d3b89ab46e 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1289,6 +1289,18 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
 	qc->mux_state = QC_MUX_NULL;
 	qc->err = quic_err_transport(QC_ERR_NO_ERROR);
 
+	/* If connection is instantiated due to an INITIAL packet with an
+	 * already checked token, consider the peer address as validated.
+	 */
+	if (token_odcid->len) {
+		TRACE_STATE("validate peer address due to initial token",
+		            QUIC_EV_CONN_INIT, qc);
+		qc->flags |= QUIC_FL_CONN_PEER_VALIDATED_ADDR;
+	}
+	else {
+		HA_ATOMIC_INC(&qc->prx_counters->half_open_conn);
+	}
+
 	/* Now proceeds to allocation of qc members. */
 	qc->rx.buf.area = pool_alloc(pool_head_quic_conn_rxbuf);
 	if (!qc->rx.buf.area) {
@@ -1396,18 +1408,6 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
 	LIST_APPEND(&th_ctx->quic_conns, &qc->el_th_ctx);
 	qc->qc_epoch = HA_ATOMIC_LOAD(&qc_epoch);
 
-	/* If connection is instantiated due to an INITIAL packet with an
-	 * already checked token, consider the peer address as validated.
-	 */
-	if (token_odcid->len) {
-		TRACE_STATE("validate peer address due to initial token",
-		            QUIC_EV_CONN_INIT, qc);
-		qc->flags |= QUIC_FL_CONN_PEER_VALIDATED_ADDR;
-	}
-	else {
-		HA_ATOMIC_INC(&qc->prx_counters->half_open_conn);
-	}
-
 	TRACE_LEAVE(QUIC_EV_CONN_INIT, qc);
 
 	return qc;