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.

diff --git a/src/quic_cid.c b/src/quic_cid.c
index 19c1f07f602aa14b8ae216cde3ea5406b60e4bd4..da3b0968eff97b66a81ae8e4c9fc7b94231d387f 100644 (file)
--- a/src/quic_cid.c
+++ b/src/quic_cid.c
@@ -163,22 +163,31 @@ int quic_get_cid_tid(const unsigned char *cid, size_t cid_len,
        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);
@@ -188,17 +197,15 @@ int quic_get_cid_tid(const unsigned char *cid, size_t cid_len,
                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;
 }
 
 /* Retrieve a quic_conn instance from the <pkt> DCID field. If the packet is an