From d1310f8d327b7102558e8c549ce09e4925b1824b Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Fri, 16 Sep 2022 13:30:59 +0200 Subject: [PATCH] BUG/MINOR: mux-quic: do not remotely close stream too early A stream is considered as remotely closed once we have received all the data with the FIN bit set. The condition to close the stream was wrong. In particular, if we receive an empty STREAM frame with FIN bit set, this would have close the stream even if we do not have yet received all the data. The condition is now adjusted to ensure that Rx buffer contains all the data up to the stream final size. In most cases, this bug is harmless. However, if compiled with DEBUG_STRICT=2, a BUG_ON_HOT crash would have been triggered if close is done too early. This was most notably the case sometimes on interop test suite with quinn or kwik clients. This can also be artificially reproduced by simulating reception of an empty STREAM frame with FIN bit set in qc_handle_strm_frm() : + if (strm_frm->fin) { + qcc_recv(qc->qcc, strm_frm->id, 0, + strm_frm->len, strm_frm->fin, + (char *)strm_frm->data); + } ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len, strm_frm->offset.key, strm_frm->fin, (char *)strm_frm->data); This must be backported up to 2.6. --- src/mux_quic.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/mux_quic.c b/src/mux_quic.c index 5b4a35469..f4b7e2f2e 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -818,10 +818,8 @@ static int qcc_decode_qcs(struct qcc *qcc, struct qcs *qcs) b = qcs_b_dup(&qcs->rx.ncbuf); - /* Signal FIN to application if STREAM FIN received and there is no gap - * in the Rx buffer. - */ - if (qcs->flags & QC_SF_SIZE_KNOWN && !ncb_is_fragmented(&qcs->rx.ncbuf)) + /* Signal FIN to application if STREAM FIN received with all data. */ + if (qcs_is_close_remote(qcs)) fin = 1; ret = qcc->app_ops->decode_qcs(qcs, &b, fin); @@ -958,6 +956,10 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset, } if (offset + len <= qcs->rx.offset) { + /* TODO offset may have been received without FIN first and now + * with it. In this case, it must be notified to be able to + * close the stream. + */ TRACE_DATA("already received offset", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs); goto out; } @@ -1027,8 +1029,10 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset, if (fin) qcs->flags |= QC_SF_SIZE_KNOWN; - if (qcs->flags & QC_SF_SIZE_KNOWN && !ncb_is_fragmented(&qcs->rx.ncbuf)) + if (qcs->flags & QC_SF_SIZE_KNOWN && + qcs->rx.offset_max == qcs->rx.offset + ncb_data(&qcs->rx.ncbuf, 0)) { qcs_close_remote(qcs); + } if (ncb_data(&qcs->rx.ncbuf, 0) && !(qcs->flags & QC_SF_DEM_FULL)) { qcc_decode_qcs(qcc, qcs);