From: Willy Tarreau <w@1wt.eu>
Date: Sun, 30 Jun 2024 04:23:30 +0000 (+0200)
Subject: BUG/MEDIUM: quic: fix possible exit from qc_check_dcid() without unlocking
X-Git-Tag: v3.1-dev3~34
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=192abc6f834dcd09f310299afe253b17f9985407;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: quic: fix possible exit from qc_check_dcid() without unlocking

Locking of the CID tree was extended in qc_check_dcid() by recent commit
05f59a5 ("BUG/MINOR: quic: fix race condition in qc_check_dcid()") but
there was a direct return from the middle of the function which was not
covered by the unlock, resulting in the function keeping the lock on
success return.

Let's just remove this return and replace it with a variable to merge all
exit paths.

This must be backported wherever the fix above is backported.
---

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 912cdcd4c1..927df07290 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1652,6 +1652,7 @@ int qc_check_dcid(struct quic_conn *qc, unsigned char *dcid, size_t dcid_len)
 	struct quic_connection_id *conn_id;
 	struct ebmb_node *node = NULL;
 	struct quic_cid_tree *tree = &quic_cid_trees[idx];
+	int ret;
 
 	/* Test against our default CID or client ODCID. */
 	if ((qc->scid.len == dcid_len &&
@@ -1668,16 +1669,17 @@ int qc_check_dcid(struct quic_conn *qc, unsigned char *dcid, size_t dcid_len)
 	 *
 	 * TODO set it to our default CID to avoid this operation next time.
 	 */
+	ret = 0;
 	HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
 	node = ebmb_lookup(&tree->root, dcid, dcid_len);
 	if (node) {
 		conn_id = ebmb_entry(node, struct quic_connection_id, node);
 		if (qc == conn_id->qc)
-			return 1;
+			ret = 1;
 	}
 	HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
 
-	return 0;
+	return ret;
 }
 
 /* Wake-up upper layer for sending if all conditions are met :