]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MAJOR: quic: do not loop on emission on closing/draining state
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Mon, 17 Jun 2024 13:39:24 +0000 (15:39 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 19 Jun 2024 13:15:59 +0000 (15:15 +0200)
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

index a1705784c415846f14b8be2c447fd0bc12f788ae..c963688ae43562bc832797e59aefd3f47d450ca2 100644 (file)
@@ -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);