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 ebd416bfc79915eee43d7316a8c3e4eb5681d3cb..5636cc810d32bf1483a6b648908bd254e2da9f96 100644 (file)
--- a/include/haproxy/quic_frame-t.h
+++ b/include/haproxy/quic_frame-t.h
@@ -96,6 +96,8 @@ enum quic_frame_type {
 
 /* 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 12833c352efcd4baa5d39b15a6cbbf6a87a436ab..0065e7fcf0658936a5e878660393b39f522f4d58 100644 (file)
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -1717,6 +1717,13 @@ static inline void qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
                }
                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 @@ static inline int qc_build_frms(struct list *outlist, struct list *inlist,
                        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.