BUG/MEDIUM: quic: Possible crash on STREAM frame loss
A crash is possible under such circumtances:
- The congestion window is drastically reduced to its miniaml value
when a quic listener is experiencing extreme packet loss ;
- we enqueue several STREAM frames to be resent and some of them could not be
transmitted ;
- some of the still in flight are acknowledged and trigger the
stream memory releasing ;
- when we come back to send the remaing STREAM frames, haproxy
crashes when it tries to build them.
To fix this issue, we mark the STREAM frame as lost when detected as lost.
Then a lookup if performed for the stream the STREAM frames are attached
to before building them. They are released if the stream is no more available
or the data range of the frame is consumed.
diff --git a/include/haproxy/quic_frame-t.h b/include/haproxy/quic_frame-t.h
index ebd416b..5636cc8 100644
--- a/include/haproxy/quic_frame-t.h
+++ b/include/haproxy/quic_frame-t.h
@@ -96,6 +96,8 @@
/* Flag a TX frame as acknowledged */
#define QUIC_FL_TX_FRAME_ACKED 0x01
+/* Flag a TX frame as lost */
+#define QUIC_FL_TX_FRAME_LOST 0x02
#define QUIC_STREAM_FRAME_TYPE_FIN_BIT 0x01
#define QUIC_STREAM_FRAME_TYPE_LEN_BIT 0x02
diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index 12833c3..0065e7f 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -1717,6 +1717,13 @@
}
else {
TRACE_PROTO("to resend frame", QUIC_EV_CONN_PRSAFRM, qc, frm);
+ if (QUIC_FT_STREAM_8 <= frm->type && frm->type <= QUIC_FT_STREAM_F) {
+ /* Mark this STREAM frame as lost. A look up their stream descriptor
+ * will be performed to check the stream is not consumed or released.
+ */
+ fprintf(stderr, "LOST STREAM FRAME\n");
+ frm->flags = QUIC_FL_TX_FRAME_LOST;
+ }
LIST_APPEND(&tmp, &frm->list);
}
}
@@ -5541,6 +5548,37 @@
break;
case QUIC_FT_STREAM_8 ... QUIC_FT_STREAM_F:
+ if (cf->flags & QUIC_FL_TX_FRAME_LOST) {
+ struct eb64_node *node = NULL;
+ struct qc_stream_desc *stream_desc = NULL;
+ struct quic_stream *strm = &cf->stream;
+
+ /* As this frame has been already lost, ensure the stream is always
+ * available or the range of this frame is not consumed before
+ * resending it.
+ */
+ node = eb64_lookup(&qc->streams_by_id, strm->id);
+ if (!node) {
+ TRACE_PROTO("released stream", QUIC_EV_CONN_PRSAFRM, qc, strm);
+ LIST_DELETE(&cf->list);
+ pool_free(pool_head_quic_frame, cf);
+ continue;
+ }
+
+ stream_desc = eb64_entry(node, struct qc_stream_desc, by_id);
+ if (strm->offset.key + strm->len <= stream_desc->ack_offset) {
+ TRACE_PROTO("ignored frame frame in already acked range",
+ QUIC_EV_CONN_PRSAFRM, qc, strm);
+ LIST_DELETE(&cf->list);
+ pool_free(pool_head_quic_frame, cf);
+ continue;
+ }
+ else if (strm->offset.key < stream_desc->ack_offset) {
+ strm->offset.key = stream_desc->ack_offset;
+ TRACE_PROTO("updated partially acked frame",
+ QUIC_EV_CONN_PRSAFRM, qc, strm);
+ }
+ }
/* Note that these frames are accepted in short packets only without
* "Length" packet field. Here, <*len> is used only to compute the
* sum of the lengths of the already built frames for this packet.