]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: quic: fix race condition in qc_check_dcid()
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 27 Jun 2024 16:15:08 +0000 (18:15 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 28 Jun 2024 14:28:33 +0000 (16:28 +0200)
qc_check_dcid() is a function which check that a DCID is associated to
the expected quic_conn instance. This is used for quic_conn socket
receive handler as there is a tiny risk that a datagram to another
connection was received on this socket.

As other operations on global CID tree, a lock must be used to protect
against race condition. However, as previous commit, lock was not held
long enough as CID tree node is accessed outside of the lock region. To
fix this, increase critical section until CID dereferencement is done.

The impact of this bug should be similar to the previous one. However,
risk of crash are even less reduced as it should be extremely rare to
receive datagram for other connections on a quic_conn socket. As such,
most of the time first check condition of qc_check_dcid() is enough.

This may fix first crash of issue github #2607.

This must be backported up to 2.8.

src/quic_conn.c

index 9a3ce2964a9a7f2d673594bf895bd001bdefb65b..912cdcd4c12eed5689733d4d78bc18bbac076518 100644 (file)
@@ -1670,13 +1670,12 @@ int qc_check_dcid(struct quic_conn *qc, unsigned char *dcid, size_t dcid_len)
         */
        HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
        node = ebmb_lookup(&tree->root, dcid, dcid_len);
-       HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
-
        if (node) {
                conn_id = ebmb_entry(node, struct quic_connection_id, node);
                if (qc == conn_id->qc)
                        return 1;
        }
+       HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
 
        return 0;
 }