QUIC LCIDM: Fix usage of LHASH
authorHugo Landau <hlandau@openssl.org>
Fri, 8 Dec 2023 09:58:21 +0000 (09:58 +0000)
committerHugo Landau <hlandau@openssl.org>
Wed, 13 Dec 2023 15:26:59 +0000 (15:26 +0000)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22981)

ssl/quic/quic_lcidm.c

index a46976a30812c7e93aa61b4732c959b4c12965c6..a3315164c7cb03adc6ed56d2d4cb4f42066dfa8e 100644 (file)
@@ -136,6 +136,25 @@ void ossl_quic_lcidm_free(QUIC_LCIDM *lcidm)
     if (lcidm == NULL)
         return;
 
+    /*
+     * Calling OPENSSL_lh_delete during a doall call is unsafe with our
+     * current LHASH implementation for several reasons:
+     *
+     * - firstly, because deletes can cause the hashtable to be contracted,
+     *   resulting in rehashing which might cause items in later buckets to
+     *   move to earlier buckets, which might cause doall to skip an item,
+     *   resulting in a memory leak;
+     *
+     * - secondly, because doall in general is not safe across hashtable
+     *   size changes, as it caches hashtable size and pointer values
+     *   while operating.
+     *
+     * The fix for this is to disable hashtable contraction using the following
+     * call, which guarantees that no rehashing will occur so long as we only
+     * call delete and not insert.
+     */
+    lh_QUIC_LCIDM_CONN_set_down_load(lcidm->conns, 0);
+
     lh_QUIC_LCIDM_CONN_doall_arg(lcidm->conns, lcidm_delete_conn_, lcidm);
 
     lh_QUIC_LCID_free(lcidm->lcids);
@@ -210,6 +229,9 @@ static void lcidm_delete_conn_lcid_(QUIC_LCID *lcid_obj, void *arg)
 
 static void lcidm_delete_conn(QUIC_LCIDM *lcidm, QUIC_LCIDM_CONN *conn)
 {
+    /* See comment in ossl_quic_lcidm_free */
+    lh_QUIC_LCID_set_down_load(conn->lcids, 0);
+
     lh_QUIC_LCID_doall_arg(conn->lcids, lcidm_delete_conn_lcid_, lcidm);
     lh_QUIC_LCIDM_CONN_delete(lcidm->conns, conn);
     lh_QUIC_LCID_free(conn->lcids);
@@ -460,6 +482,7 @@ int ossl_quic_lcidm_retire(QUIC_LCIDM *lcidm,
 
     args.retire_prior_to    = retire_prior_to;
     args.earliest_seq_num   = UINT64_MAX;
+
     lh_QUIC_LCID_doall_arg(conn->lcids, retire_for_conn, &args);
     if (args.earliest_seq_num_lcid_obj == NULL)
         return 1;