From 96d08d37d97c2f250c4436b920ae708d45135708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Thu, 11 Aug 2022 12:04:07 +0200 Subject: [PATCH] BUG/MINOR: quic: Possible infinite loop in quic_build_post_handshake_frames() This loop is due to the fact that we do not select the next node before the conditional "continue" statement. Furthermore the condition and the "continue" statement may be removed after replacing eb64_first() call by eb64_lookup_ge(): we are sure this condition may not be satisfied. Add some comments: this function initializes connection IDs with sequence number 1 upto non included. Take the opportunity of this patch to remove a "return" wich broke this traces rule: for any function, do not call TRACE_ENTER() without TRACE_LEAVE()! Add also TRACE_ERROR() for any encoutered errors. Must be backported to 2.6 --- src/xprt_quic.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/xprt_quic.c b/src/xprt_quic.c index 05ff1e7ef..8bc15efc5 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -3374,14 +3374,19 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc) /* Only servers must send a HANDSHAKE_DONE frame. */ if (qc_is_listener(qc)) { frm = pool_zalloc(pool_head_quic_frame); - if (!frm) - return 0; + if (!frm) { + TRACE_ERROR("frame allocation error", QUIC_EV_CONN_IO_CB, qc); + goto leave; + } LIST_INIT(&frm->reflist); frm->type = QUIC_FT_HANDSHAKE_DONE; LIST_APPEND(&frm_list, &frm->list); } + /* Initialize connection IDs minus one: there is + * already one connection ID used for the current connection. + */ first = 1; max = qc->tx.params.active_connection_id_limit; @@ -3390,13 +3395,17 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc) struct quic_connection_id *cid; frm = pool_zalloc(pool_head_quic_frame); - if (!frm) + if (!frm) { + TRACE_ERROR("frame allocation error", QUIC_EV_CONN_IO_CB, qc); goto err; + } LIST_INIT(&frm->reflist); cid = new_quic_cid(&qc->cids, qc, i); - if (!cid) + if (!cid) { + TRACE_ERROR("CID allocation error", QUIC_EV_CONN_IO_CB, qc); goto err; + } /* insert the allocated CID in the receiver datagram handler tree */ ebmb_insert(&quic_dghdlrs[tid].cids, &cid->node, cid->cid.len); @@ -3418,18 +3427,14 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc) list_for_each_entry_safe(frm, frmbak, &frm_list, list) pool_free(pool_head_quic_frame, frm); - node = eb64_first(&qc->cids); + node = eb64_lookup_ge(&qc->cids, first); while (node) { struct quic_connection_id *cid; - cid = eb64_entry(node, struct quic_connection_id, seq_num); if (cid->seq_num.key >= max) break; - if (cid->seq_num.key < first) - continue; - node = eb64_next(node); ebmb_delete(&cid->node); eb64_delete(&cid->seq_num);