BUG/MEDIUM: quic: fix race-condition in quic_get_cid_tid()
haproxy generates CID for clients which reuse them as DCID on their
packets. These CID are stored in a global tree quic_cid_trees. Each
operation on this tree must be done under lock protection.
quic_get_cid_tid() is a function which lookups a CID in global tree and
return the associated thread ID. This is used on datagram reception on
listener socket before redispatching the datagram to the correct thread.
This function uses a lock to protect quic_cid_trees access. However,
lock region is too small as CID tree node is accessed outside of it. Fix
this by extending lock protection for CID dereferencement until thread
ID is retrieved.
The impact of this bug is unknown, but it may possible cause crashes.
However, it is probably rare as most of datagram reception is done on
quic_conn socket which does not uses quic_get_cid_tid().
This may fix first crash of github issue #2607.
This must be backported up to 2.8.
(cherry picked from commit 72267ff35f7c82f5a32d99a03124b73d95b00a01)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit ab76fa58e85e069d27a1699f81d60e32133a9fe4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 01cfd4cb396ea6deb95b734075625b2e9904f8d0)
[cf: applied in quic_conn.c]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/quic_conn.c b/src/quic_conn.c
index 0286047..dbeeb74 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -4139,22 +4139,31 @@
struct quic_cid_tree *tree;
struct quic_connection_id *conn_id;
struct ebmb_node *node;
+ int cid_tid = -1;
tree = &quic_cid_trees[_quic_cid_tree_idx(cid)];
HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
node = ebmb_lookup(&tree->root, cid, cid_len);
+ if (node) {
+ conn_id = ebmb_entry(node, struct quic_connection_id, node);
+ cid_tid = HA_ATOMIC_LOAD(&conn_id->tid);
+ }
HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
- if (!node) {
+ /* If CID not found, it may be an ODCID, thus not stored in global CID
+ * tree. Derive it to its associated DCID value and reperform a lookup.
+ */
+ if (cid_tid < 0) {
struct quic_cid orig, derive_cid;
struct quic_rx_packet pkt;
if (!qc_parse_hd_form(&pkt, &pos, pos + len))
- goto not_found;
+ goto out;
+ /* ODCID are only used in INITIAL or 0-RTT packets */
if (pkt.type != QUIC_PACKET_TYPE_INITIAL &&
pkt.type != QUIC_PACKET_TYPE_0RTT) {
- goto not_found;
+ goto out;
}
memcpy(orig.data, cid, cid_len);
@@ -4164,17 +4173,15 @@
tree = &quic_cid_trees[quic_cid_tree_idx(&derive_cid)];
HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
node = ebmb_lookup(&tree->root, cid, cid_len);
+ if (node) {
+ conn_id = ebmb_entry(node, struct quic_connection_id, node);
+ cid_tid = HA_ATOMIC_LOAD(&conn_id->tid);
+ }
HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
}
- if (!node)
- goto not_found;
-
- conn_id = ebmb_entry(node, struct quic_connection_id, node);
- return HA_ATOMIC_LOAD(&conn_id->tid);
-
- not_found:
- return -1;
+ out:
+ return cid_tid;
}
/* Allocate a new CID and attach it to <root> ebtree.