From afb7b9d8e5a70a741bbb890945fa9ff51dad027d Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 19 Sep 2022 11:58:24 +0200 Subject: [PATCH] BUG/MEDIUM: mux-quic: fix nb_hreq decrement nb_hreq is a counter on qcc for active HTTP requests. It is incremented for each qcs where a full HTTP request was received. It is decremented when the stream is closed locally : - on HTTP response fully transmitted - on stream reset A bug will occur if a stream is resetted without having processed a full HTTP request. nb_hreq will be decremented whereas it was not incremented. This will lead to a crash when building with DEBUG_STRICT=2. If BUG_ON_HOT are not active, nb_hreq counter will wrap which may break the timeout logic for the connection. This bug was triggered on haproxy.org. It can be reproduced by simulating the reception of a STOP_SENDING frame instead of a STREAM one by patching qc_handle_strm_frm() : + if (quic_stream_is_bidi(strm_frm->id)) + qcc_recv_stop_sending(qc->qcc, strm_frm->id, 0); + //ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len, + // strm_frm->offset.key, strm_frm->fin, + // (char *)strm_frm->data); To fix this bug, a qcs is now flagged with a new QC_SF_HREQ_RECV. This is set when the full HTTP request is received. When the stream is closed locally, nb_hreq will be decremented only if this flag was set. This must be backported up to 2.6. --- include/haproxy/mux_quic-t.h | 1 + include/haproxy/mux_quic.h | 5 +++++ src/mux_quic.c | 4 +++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h index 43ea53044..9602cc4c0 100644 --- a/include/haproxy/mux_quic-t.h +++ b/include/haproxy/mux_quic-t.h @@ -117,6 +117,7 @@ struct qcc { #define QC_SF_DEM_FULL 0x00000020 /* demux blocked on request channel buffer full */ #define QC_SF_READ_ABORTED 0x00000040 /* stream rejected by app layer */ #define QC_SF_TO_RESET 0x00000080 /* a RESET_STREAM must be sent */ +#define QC_SF_HREQ_RECV 0x00000100 /* a full HTTP request has been received */ /* Maximum size of stream Rx buffer. */ #define QC_S_RX_BUF_SZ (global.tune.bufsize - NCB_RESERVED_SZ) diff --git a/include/haproxy/mux_quic.h b/include/haproxy/mux_quic.h index 699007e93..892ae9130 100644 --- a/include/haproxy/mux_quic.h +++ b/include/haproxy/mux_quic.h @@ -92,6 +92,11 @@ static inline struct stconn *qc_attach_sc(struct qcs *qcs, struct buffer *buf) if (!sc_new_from_endp(qcs->sd, sess, buf)) return NULL; + /* QC_SF_HREQ_RECV must be set once for a stream. Else, nb_hreq counter + * will be incorrect for the connection. + */ + BUG_ON_HOT(qcs->flags & QC_SF_HREQ_RECV); + qcs->flags |= QC_SF_HREQ_RECV; ++qcc->nb_sc; ++qcc->nb_hreq; diff --git a/src/mux_quic.c b/src/mux_quic.c index 4a6c1a884..38c863dee 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -414,7 +414,9 @@ static void qcs_close_local(struct qcs *qcs) if (quic_stream_is_bidi(qcs->id)) { qcs->st = (qcs->st == QC_SS_HREM) ? QC_SS_CLO : QC_SS_HLOC; - qcc_rm_hreq(qcs->qcc); + + if (qcs->flags & QC_SF_HREQ_RECV) + qcc_rm_hreq(qcs->qcc); } else { /* Only local uni streams are valid for this operation. */