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.
This commit is contained in:
Frédéric Lécaille 2022-04-27 07:17:56 +02:00 committed by Amaury Denoyelle
parent dafbde6c8c
commit 77cb38d22d
2 changed files with 40 additions and 0 deletions

View File

@ -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

View File

@ -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.