From 937324d493eefb54d2f68820a9afcf5cfd66bdf5 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 17 Jun 2024 15:39:24 +0200 Subject: [PATCH] BUG/MAJOR: quic: do not loop on emission on closing/draining state To emit CONNECTION_CLOSE frame, a special buffer is allocated via qc_txb_store(). This is due to QUIC_FL_CONN_IMMEDIATE_CLOSE flag. However this flag is reset after qc_send_ppkts() invocation to prevent reemission of CONNECTION_CLOSE frame. qc_send() can invoke multiple times a series of qc_prep_pkts() + qc_send_ppkts() to emit several datagrams. However, this may cause a crash if on first loop a CONNECTION_CLOSE is emitted. On the next loop iteration, QUIC_FL_CONN_IMMEDIATE_CLOSE is resetted, thus qc_prep_pkts() will use the wrong buffer size as end delimiter. In some cases, this may cause a BUG_ON() crash due to b_add() outside of buffer. This bug can be reproduced by using a while loop of ngtcp2-client and interrupting them randomly via Ctrl+C. Here is the patch which introduce this regression : cdfceb10ae136b02e51f9bb346321cf0045d58e0 MINOR: quic: refactor qc_prep_pkts() loop --- src/quic_tx.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/quic_tx.c b/src/quic_tx.c index a1705784c..c963688ae 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -708,8 +708,13 @@ int qc_send(struct quic_conn *qc, int old_data, struct list *send_list) qc->flags |= QUIC_FL_CONN_RETRANS_OLD_DATA; } - /* Prepare and send packets until we could not further prepare packets. */ - while (!LIST_ISEMPTY(send_list)) { + /* Prepare and send packets until we could not further prepare packets. + * Sending must be interrupted if a CONNECTION_CLOSE was already sent + * previously and is currently not needed. + */ + while (!LIST_ISEMPTY(send_list) && + (!(qc->flags & (QUIC_FL_CONN_CLOSING|QUIC_FL_CONN_DRAINING)) || + (qc->flags & QUIC_FL_CONN_IMMEDIATE_CLOSE))) { /* Buffer must always be empty before qc_prep_pkts() usage. * qc_send_ppkts() ensures it is cleared on success. */ @@ -728,6 +733,12 @@ int qc_send(struct quic_conn *qc, int old_data, struct list *send_list) TRACE_DEVEL("stopping on qc_prep_pkts() return", QUIC_EV_CONN_TXPKT, qc); break; } + + if ((qc->flags & QUIC_FL_CONN_DRAINING) && + !(qc->flags & QUIC_FL_CONN_IMMEDIATE_CLOSE)) { + TRACE_DEVEL("draining connection", QUIC_EV_CONN_TXPKT, qc); + break; + } } qc_txb_release(qc);