]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MAJOR: mux-quic: do not crash on empty STREAM frame emission
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 4 Oct 2024 08:10:37 +0000 (10:10 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 4 Oct 2024 09:31:11 +0000 (11:31 +0200)
Most of the time STREAM frames emitted by QUIC MUX have some data in it.
However, it is possible to use an empty frame when a delayed FIN must be
transferred.

Recently, QUIC MUX send callback notification has been refactored. Now,
this callback is blindly called by quic_conn lower layer each time a
STREAM frame is built into a newly Tx packet. QUIC MUX is responsible to
ensure the notified frame corresponds to newly emitted data or
retransmission. Offsets are used for this comparison, but this requires
special care for empty FIN frames.

Sadly, the comparison written to determine if an empty FIN frame was
sent for the first time or retransmitted is not correct. This caused
such frame to always be dismissed as retransmission in QUIC MUX sent
callback. This prevented the related QCS instance to be removed from the
send_list, causing qcc_io_send() to retry a new emission. This was
finally interrupted by the BUG_ON() assertion to prevent an infinite
loop.

Fix this crash by updating the condition in QUIC MUX send callback. For
empty STREAM frame, it is sufficient to check if QC_SF_FIN_STREAM was
already removed or not to detect a retransmission. Indeed, empty STREAM
frames are never used outside of delayed FIN reporting.

No need to backport. This crash was introduced in the current dev branch
by the following commit.
  d7f4e5abf0b7129329d0ea716c104474fd934bc6
  MEDIUM: quic: strengthen MUX send notification

src/mux_quic.c

index 8b46580757e7c02fd2feda4667e6d568d7e64862..58695b0f3720d3fedd6b269bc7bda49172bf57e4 100644 (file)
@@ -564,10 +564,10 @@ static void qmux_ctrl_send(struct qc_stream_desc *stream, uint64_t data, uint64_
        BUG_ON(offset > qcs->tx.fc.off_real);
 
        /* check if the STREAM frame has already been notified. It can happen
-        * for retransmission.
+        * for retransmission. Special care must be taken to ensure an empty
+        * STREAM frame with FIN set is not considered as retransmitted
         */
-       if (offset + data < qcs->tx.fc.off_real ||
-           (!data && (!(qcs->flags & QC_SF_FIN_STREAM) || qc_stream_buf_get(qcs->stream) || qcs_prep_bytes(qcs)))) {
+       if (offset + data < qcs->tx.fc.off_real || (!data && !(qcs->flags & QC_SF_FIN_STREAM))) {
                TRACE_DEVEL("offset already notified", QMUX_EV_QCS_SEND, qcc->conn, qcs);
                goto out;
        }