BUG/MEDIUM: quic: crash after quic_conn allocation failures

This regression arrived with this commit:

	MINOR: quic-be: QUIC connection allocation adaptation (qc_new_conn())

where qc_new_conn() was modified. The ->cids allocation was moved without
checking if a quic_conn_release() call could lead to crashes due to uninitialized
quic_conn members. Indeed, if qc_new_conn() fails, then quic_conn_release() is
called. This bug could impact both QUIC servers and clients.

Such crashes could be reproduced with -dMfail option. To reach them, the
memory allocations must fail. So, this is relatively rare, except on systems
with limited memory.

This patch ensures all the quic_conn members which could lead to crash
from quic_conn_release() are initialized before any remaining memory allocations
required for the quic_conn.

The <conn_id> variable allocated by the client is no more attached to
the connection during its allocation, but after the ->cids trees is allocated.

No backport needed.

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 2358073a7bd7c04f2a5f2ace870ba6c59f93260f..ab9557d097f588f11875263947ae91f75373507c 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1113,14 +1113,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
                goto err;
        }
 
-       qc->cids = pool_alloc(pool_head_quic_cids);
-       if (!qc->cids) {
-               TRACE_ERROR("Could not allocate a new CID tree", QUIC_EV_CONN_INIT, qc);
-               goto err;
-       }
-
        qc->target = target;
-       *qc->cids = EB_ROOT;
        /* Now that quic_conn instance is allocated, quic_conn_release() will
         * ensure global accounting is decremented.
         */
@@ -1138,6 +1131,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
        qc->streams_by_id = EB_ROOT_UNIQUE;
 
        /* Required to call free_quic_conn_cids() from quic_conn_release() */
+       qc->cids = NULL;
        qc->tx.cc_buf_area = NULL;
        qc_init_fd(qc);
 
@@ -1194,7 +1188,6 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
                qc->odcid = *dcid;
                /* Copy the packet SCID to reuse it as DCID for sending */
                qc->dcid = *scid;
-               qc->tx.buf = BUF_NULL;
                conn_id->qc = qc;
        }
        /* QUIC Client (outgoing connection to servers) */
@@ -1215,7 +1208,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
                memcpy(&qc->odcid, qc->dcid.data, sizeof(qc->dcid.data));
                qc->odcid.len = qc->dcid.len;
 
-               conn_cid = new_quic_cid(qc->cids, qc, NULL, NULL);
+               conn_cid = new_quic_cid(NULL, qc, NULL, NULL);
                if (!conn_cid)
                        goto err;
 
@@ -1223,7 +1216,6 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
                dcid = &qc->dcid;
                conn_id = conn_cid;
 
-               qc->tx.buf = BUF_NULL;
                qc->next_cid_seq_num = 1;
                conn->handle.qc = qc;
        }
@@ -1251,10 +1243,21 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
                goto err;
        }
 
-       /* Listener only */
-       if (l && HA_ATOMIC_LOAD(&l->rx.quic_mode) == QUIC_SOCK_MODE_CONN &&
+       qc->cids = pool_alloc(pool_head_quic_cids);
+       if (!qc->cids) {
+               TRACE_ERROR("Could not allocate a new CID tree", QUIC_EV_CONN_INIT, qc);
+               goto err;
+       }
+
+       *qc->cids = EB_ROOT;
+       if (!l) {
+               /* Attach the current CID to the connection */
+               eb64_insert(qc->cids, &conn_id->seq_num);
+       }
+       else if (HA_ATOMIC_LOAD(&l->rx.quic_mode) == QUIC_SOCK_MODE_CONN &&
            (quic_tune.options & QUIC_TUNE_SOCK_PER_CONN) &&
            is_addr(local_addr)) {
+               /* Listener only */
                TRACE_USER("Allocate a socket for QUIC connection", QUIC_EV_CONN_INIT, qc);
                qc_alloc_fd(qc, local_addr, peer_addr);