From: Amaury Denoyelle Date: Fri, 25 Sep 2026 14:03:07 +0000 (+0200) Subject: BUG/MAJOR: mux_quic: fix potential crash on RESET_STREAM receive X-Git-Tag: v3.5-dev8~106 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=a33abe9e8300e17fa106f0d0d6160fbd8ed13880;p=haproxy.git BUG/MAJOR: mux_quic: fix potential crash on RESET_STREAM receive When a RESET_STREAM frame is received and upper layer is not allocated, QCS instance is immediately locally closed and inserted into QCC purge list. This operation has been performed since the following patch. Before, the QCS would remain allocated and could possibly prevent the connection closure if accounted as a request stream. commit b7e0aa231aa7af214c97f9540f7bba8498898b11 BUG/MAJOR: mux_quic: fix leak on RESET_STREAM reception Sadly, this causes a crash if QCS is already inserted in QCC send list. This is because element is used both for send and purge list, as a QCS cannot be present in both at the same time. Fix this skipping insertion in purge list if QCS is already present in the send list. QCS is not closed locally either in this case. This allows the QCS to finalize its emission before being closed and purged. This latter case requires to extend check in qcm_ctrl_send() for QCS purging when everything is emitted. This is now also performed if a RESET_STREAM has been received and upper stream layer is not present. These modifications reveal that it is probably not valid to silently close a QCS without notifying the peer. Reaction to a RESET_STREAM is in fact dependent on the application layer protocol and the QUIC stream type. For now, the current code base should ensure that a bidi HTTP/3 stream is not kept forever after peer reset. A more thoughtful approach may be implemented later in a future patch. This must be backported up to 2.8. --- diff --git a/src/mux_quic.c b/src/mux_quic.c index 23433aacc..0e11fc7cf 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -720,7 +720,17 @@ static void qcm_ctrl_send(struct qc_stream_desc *stream, uint64_t data, uint64_t LIST_DEL_INIT(&qcs->el_send); TRACE_STATE("stream sent done", QMUX_EV_QCS_SEND, qcc->conn, qcs); - if (qcs->flags & (QC_SF_FIN_STREAM|QC_SF_DETACH)) { + /* Everything sent, close the stream if no new data will come. + * This is the case if either FIN has been sent or upper layer + * is detached. + * + * This is also performed if peer has resetted its input as no + * more transfer should occur on this QCS instance. + * TODO this assumption may be invalid depending on QUIC stream + * type - app proto should be responsible to deal with RS. + */ + if ((qcs->flags & (QC_SF_FIN_STREAM|QC_SF_DETACH)) || + (!qcs_sc(qcs) && (qcs->flags & QC_SF_RECV_RESET))) { /* Close stream locally. */ qcs_close_local(qcs); @@ -2360,13 +2370,11 @@ int qcc_recv_reset_stream(struct qcc *qcc, uint64_t id, uint64_t err, uint64_t f /* Remove stream from recv_list if present. */ LIST_DEL_INIT(&qcs->el_recv); - /* Check if RESET_STREAM received before stream layer initialization. - * If true, prepare QCS purgeing immediately. + /* No more data will be received after peer reset. QCS can be purged + * if there is nothing to sent on our side. */ - if (!qcs_sc(qcs) && !(qcs->flags & QC_SF_DETACH)) { + if (!qcs_sc(qcs) && !LIST_INLIST(&qcs->el_send)) { qcs_close_local(qcs); - - BUG_ON(LIST_INLIST(&qcs->el_send)); TRACE_STATE("add stream in purg_list", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs); LIST_APPEND(&qcs->qcc->purg_list, &qcs->el_send); tasklet_wakeup(qcs->qcc->wait_event.tasklet);