Browse Source

QUIC LCIDM: Fix usage of LHASH

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22981)
Hugo Landau 6 months ago
parent
commit
708b4fb708
1 changed files with 23 additions and 0 deletions
  1. 23 0
      ssl/quic/quic_lcidm.c

+ 23 - 0
ssl/quic/quic_lcidm.c

@@ -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;