From b14c4ffd6b992ba1915673b8cd0c48a278fccfce Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 1 Sep 2025 15:07:53 +0200 Subject: [PATCH] BUG/MAJOR: mux-quic: fix crash on reload during emission MUX QUIC restricts buffer allocation per connection based on the underlying congestion window. If a QCS instance cannot allocate a new buffer, it is put in a buf_wait list. Typically, this will cause stream upper layer to subscribe for sending. A BUG_ON() was present on snd_buf and nego_ff callback prologue to ensure that these functions were not called if QCS is already in buf_wait list. The objective was to guarantee that there is no wake up on a stream if it cannot allocate a buffer. However, this BUG_ON() is not correct, as it can be fired legitimely. Indeed, stream layer can retry emission even if no wake up occured. This case can happen on reload. Thus, BUG_ON() will cause an unexpected crash. Fix this by removing these BUG_ON(). Instead, snd_buf/nego_ff callbacks ensure that QCS is not subscribed in buf_wait list. If this is the case, a nul value will be returned, which is sufficient for the stream layer to pause emission and subscribe if necessary. Occurences for this crash have been reported on the mailing list. It is also the subject of github issue #3080, which should be fixed with this patch. This must be backported up to 3.0. (cherry picked from commit dcf22616123432e3230c0a084cf13c5adc57b851) Signed-off-by: Christopher Faulet (cherry picked from commit 49fd57fe90939f15e7f75040a968266d7fa0112a) Signed-off-by: Christopher Faulet (cherry picked from commit 54115fc2205ffbfe19a36212178cd861a210883f) Signed-off-by: Christopher Faulet --- src/mux_quic.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/mux_quic.c b/src/mux_quic.c index 92df120..c39601a 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -3055,9 +3055,6 @@ static size_t qmux_strm_snd_buf(struct stconn *sc, struct buffer *buf, TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs); - /* Stream must not be woken up if already waiting for conn buffer. */ - BUG_ON(LIST_INLIST(&qcs->el_buf)); - /* Sending forbidden if QCS is locally closed (FIN or RESET_STREAM sent). */ BUG_ON(qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET)); @@ -3071,6 +3068,11 @@ static size_t qmux_strm_snd_buf(struct stconn *sc, struct buffer *buf, goto end; } + if (LIST_INLIST(&qcs->el_buf)) { + TRACE_DEVEL("leaving on no buf avail", QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs); + goto end; + } + if (qfctl_sblocked(&qcs->qcc->tx.fc)) { TRACE_DEVEL("leaving on connection flow control", QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs); @@ -3119,9 +3121,6 @@ static size_t qmux_strm_nego_ff(struct stconn *sc, struct buffer *input, TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs); - /* Stream must not be woken up if already waiting for conn buffer. */ - BUG_ON(LIST_INLIST(&qcs->el_buf)); - /* Sending forbidden if QCS is locally closed (FIN or RESET_STREAM sent). */ BUG_ON(qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET)); @@ -3143,6 +3142,12 @@ static size_t qmux_strm_nego_ff(struct stconn *sc, struct buffer *input, goto end; } + if (LIST_INLIST(&qcs->el_buf)) { + TRACE_DEVEL("leaving on no buf avail", QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs); + qcs->sd->iobuf.flags |= IOBUF_FL_FF_BLOCKED; + goto end; + } + if (qfctl_sblocked(&qcs->qcc->tx.fc)) { TRACE_DEVEL("leaving on connection flow control", QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs); if (!LIST_INLIST(&qcs->el_fctl)) { -- 1.7.10.4