From d7755375a51726e3d13c4f891ca8ab8cc1ba9a4d Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 3 Oct 2022 17:20:31 +0200 Subject: [PATCH] BUG/MINOR: mux-quic: ignore STOP_SENDING for locally closed stream It is possible to receive a STOP_SENDING frame for a locally closed stream. This was not properly managed as this would result in a BUG_ON() crash from qcs_idle_open() call under qcc_recv_stop_sending(). Now, STOP_SENDING frames are ignored when received on streams already locally closed. This has two consequences depending on the reason of closure : * if a RESET_STREAM was already emitted and closed the stream, this patch prevents to emit a new RESET_STREAM. This behavior is thus better. * if stream was closed due to all data transmitted, no RESET_STREAM will be built. This is contrary to the RFC 9000 which advice to transmit it, even on "Data Sent" state. However, this is not mandatory so the new behavior is acceptable, even if it could be improved. This crash has been detected on haproxy.org. This can be artifically reproduced by adding the following snippet at the end of qc_send_mux() when doing a request with a small payload response : qcc_recv_stop_sending(qc->qcc, 0, 0); This must be backported up to 2.6. --- src/mux_quic.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/mux_quic.c b/src/mux_quic.c index d27a36c60e..0482e92bc7 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -1067,6 +1067,27 @@ int qcc_recv_stop_sending(struct qcc *qcc, uint64_t id, uint64_t err) goto out; TRACE_PROTO("receiving STOP_SENDING", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs); + + /* RFC 9000 3.5. Solicited State Transitions + * + * An endpoint is expected to send another STOP_SENDING frame if a + * packet containing a previous STOP_SENDING is lost. However, once + * either all stream data or a RESET_STREAM frame has been received for + * the stream -- that is, the stream is in any state other than "Recv" + * or "Size Known" -- sending a STOP_SENDING frame is unnecessary. + */ + + /* TODO thanks to previous RFC clause, STOP_SENDING is ignored if current stream + * has already been closed locally. This is useful to not emit multiple + * RESET_STREAM for a single stream. This is functional if stream is + * locally closed due to all data transmitted, but in this case the RFC + * advices to use an explicit RESET_STREAM. + */ + if (qcs_is_close_local(qcs)) { + TRACE_STATE("ignoring STOP_SENDING", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs); + goto out; + } + qcs_idle_open(qcs); /* RFC 9000 3.5. Solicited State Transitions