From: Frédéric Lécaille <flecaille@haproxy.com>
Date: Fri, 16 Jun 2023 14:10:58 +0000 (+0200)
Subject: BUG/MINOR: quic: Possible endless loop in quic_lstnr_dghdlr()
X-Git-Tag: v2.9-dev1~55
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4ae29be18c5b212dd2a1a8e9fa0ee2fcb9dbb4b3;p=thirdparty%2Fhaproxy.git

BUG/MINOR: quic: Possible endless loop in quic_lstnr_dghdlr()

This may happen when the initilization of a new QUIC conn fails with qc_new_conn()
when receiving an Initial paquet. This is done after having allocated a CID with
new_quic_cid() called by quic_rx_pkt_retrieve_conn() which stays in the listener
connections tree without a QUIC connection attached to. Then when the listener
receives another Initial packet for the same CID, quic_rx_pkt_retrieve_conn()
returns NULL again (no QUIC connection) but with an thread ID already bound to the
connection, leading the datagram to be requeued in the same datagram handler thread
queue. And so on.

To fix this, the connection is created after having created the connection ID.
If this fails, the connection is deallocated.

During the race condition, when two different threads handle two datagrams for
the same connection, in addition to releasing the newer created connection ID,
the newer QUIC connection must also be released.

Must be backported as far as 2.7.
---

diff --git a/src/quic_conn.c b/src/quic_conn.c
index e5fd58b0c9..f30df6228f 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -6901,6 +6901,15 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 			if (!conn_id)
 				goto err;
 
+			qc = qc_new_conn(pkt->version, ipv4, &pkt->dcid, &pkt->scid, &token_odcid,
+			                 conn_id, &dgram->daddr, &pkt->saddr, 1,
+			                 !!pkt->token_len, l);
+			if (qc == NULL) {
+				eb64_delete(&conn_id->seq_num);
+				pool_free(pool_head_quic_connection_id, conn_id);
+				goto err;
+			}
+
 			tree = &quic_cid_trees[quic_cid_tree_idx(&conn_id->cid)];
 			HA_RWLOCK_WRLOCK(QC_CID_LOCK, &tree->lock);
 			node = ebmb_insert(&tree->root, &conn_id->node, conn_id->cid.len);
@@ -6909,18 +6918,14 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 
 				conn_id = ebmb_entry(node, struct quic_connection_id, node);
 				*new_tid = HA_ATOMIC_LOAD(&conn_id->tid);
+				quic_conn_release(qc);
+				qc = NULL;
 			}
 			HA_RWLOCK_WRUNLOCK(QC_CID_LOCK, &tree->lock);
 
 			if (*new_tid != -1)
 				goto out;
 
-			qc = qc_new_conn(pkt->version, ipv4, &pkt->dcid, &pkt->scid, &token_odcid,
-			                 conn_id, &dgram->daddr, &pkt->saddr, 1,
-			                 !!pkt->token_len, l);
-			if (qc == NULL)
-				goto err;
-
 			HA_ATOMIC_INC(&prx_counters->half_open_conn);
 		}
 	}