RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-01-20 04:30:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MEDIUM: quic: properly handle duplicated STREAM frames

Browse Source 
When a STREAM frame is re-emitted, it will point to the same stream
buffer as the original one. If an ACK is received for either one of
these frame, the underlying buffer may be freed. Thus, if the second
frame is declared as lost and schedule for retransmission, we must
ensure that the underlying buffer is still allocated or interrupt the
retransmission.

Stream buffer is stored as an eb_tree indexed by the stream ID. To avoid
to lookup over a tree each time a STREAM frame is re-emitted, a lost
STREAM frame is flagged as QUIC_FL_TX_FRAME_LOST.

In most cases, this code is functional. However, there is several
potential issues which may cause a segfault :
- when explicitely probing with a STREAM frame, the frame won't be
  flagged as lost
- when splitting a STREAM frame during retransmission, the flag is not
  copied

To fix both these cases, QUIC_FL_TX_FRAME_LOST flag has been converted
to a <dup> field in quic_stream structure. This field is now properly
copied when splitting a STREAM frame. Also, as this is now an inner
quic_frame field, it will be copied automatically on qc_frm_dup()
invocation thus ensuring that it will be set on probing.

This issue was encounted randomly with the following backtrace :
 #0  __memmove_avx512_unaligned_erms ()
 #1  0x000055f4d5a48c01 in memcpy (__len=18446698486215405173, __src=<optimized out>,
 #2  quic_build_stream_frame (buf=0x7f6ac3fcb400, end=<optimized out>, frm=0x7f6a00556620,
 #3  0x000055f4d5a4a147 in qc_build_frm (buf=buf@entry=0x7f6ac3fcb5d8,
 #4  0x000055f4d5a23300 in qc_do_build_pkt (pos=<optimized out>, end=<optimized out>,
 #5  0x000055f4d5a25976 in qc_build_pkt (pos=0x7f6ac3fcba10,
 #6  0x000055f4d5a30c7e in qc_prep_app_pkts (frms=0x7f6a0032bc50, buf=0x7f6a0032bf30,
 #7  qc_send_app_pkts (qc=0x7f6a0032b310, frms=0x7f6a0032bc50) at src/quic_conn.c:4184
 #8  0x000055f4d5a35f42 in quic_conn_app_io_cb (t=0x7f6a0009c660, context=0x7f6a0032b310,

This should fix github issue #2051.

This should be backported up to 2.6.
This commit is contained in:
Amaury Denoyelle 2023-02-22 10:44:27 +01:00
parent 8c20a74c90
commit c8a0efbda8
2 changed files with 6 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/haproxy/quic_frame-t.h
View File

@ -99,8 +99,6 @@ 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
@ -175,6 +173,8 @@ struct quic_stream {
* for RX pointer into the packet buffer.
*/
const unsigned char *data;
char dup; /* set for duplicated frame : this forces to check for the underlying qc_stream_buf instance before emitting it. */
};
struct quic_max_data {

11
src/quic_conn.c
View File

@ -1885,12 +1885,6 @@ static inline int qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
close = 1;
}
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.
*/
frm->flags |= QUIC_FL_TX_FRAME_LOST;
}
LIST_APPEND(&tmp, &frm->list);
TRACE_DEVEL("frame requeued", QUIC_EV_CONN_PRSAFRM, qc, frm);
}
@ -2513,6 +2507,8 @@ static void qc_dup_pkt_frms(struct quic_conn *qc,
TRACE_DEVEL("updated partially acked frame",
QUIC_EV_CONN_PRSAFRM, qc, frm);
}
strm_frm->dup = 1;
break;
}
@ -6987,7 +6983,7 @@ 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) {
if (cf->stream.dup) {
struct eb64_node *node = NULL;
struct qc_stream_desc *stream_desc = NULL;
struct quic_stream *strm = &cf->stream;
@ -7094,6 +7090,7 @@ static inline int qc_build_frms(struct list *outlist, struct list *inlist,
/* FIN bit reset */
new_cf->type &= ~QUIC_STREAM_FRAME_TYPE_FIN_BIT;
new_cf->stream.data = cf->stream.data;
new_cf->stream.dup = cf->stream.dup;
TRACE_DEVEL("split frame", QUIC_EV_CONN_PRSAFRM, qc, new_cf);
if (cf->origin) {
TRACE_DEVEL("duplicated frame", QUIC_EV_CONN_PRSAFRM, qc);