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.

diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h
index 43ea53044c93d9e025ace90fb33050d39354d4df..9602cc4c0a787ac100f77725d47097c9f2589c29 100644 (file)
--- 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 699007e93479d7a78f409ad0571279c274af6c84..892ae91302a5bfe07790ecd21201e04f2323c38b 100644 (file)
--- 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 4a6c1a88439b674090f94628668e1b77e23e140b..38c863deedbc8408654ff0842a6e0b838b525c98 100644 (file)
--- 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. */