upstream/haproxy
1
0
Fork 0
mirror of https://github.com/haproxy/haproxy.git synced 2026-04-29 02:00:37 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

BUG/MEDIUM: quic: do not use qcs from quic_stream on ACK parsing

Browse source 
The quic_stream frame stores the qcs instance. On ACK parsing, qcs is
accessed to clear the stream buffer. This can cause a segfault if the
MUX or the qcs is already released.

Consider the following scenario :

1. a STREAM frame is generated by the MUX
   transport layer emits the frame with PKN=1
   upper layer has finished the transfer so related qcs is detached

2. transport layer reemits the frame with PKN=2 because ACK was not
   received

3. ACK for PKN=1 is received, stream buffer is cleared
   at this stage, qcs may be freed by the MUX as it is detached

4. ACK for PKN=2 is received
   qcs for STREAM frame is dereferenced which will lead to a crash

To prevent this, qcs is never accessed from the quic_stream during ACK
parsing. Instead, a lookup is done on the MUX streams tree. If the MUX
is already released, no lookup is done. These checks prevents a possible
segfault.

This change may have an impact on the perf as now we are forced to use a
tree lookup operation. If this is the case, an alternative solution may
be to implement a refcount on qcs instances.
This commit is contained in:
Amaury Denoyelle 2022-03-29 11:51:17 +02:00
parent 10cea5cd6d
commit 8d5def0bab
1 changed files with 23 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
src/xprt_quic.c
View file

@ -1455,8 +1455,30 @@ static inline void qc_treat_acked_tx_frm(struct quic_conn *qc,
switch (frm->type) {
case QUIC_FT_STREAM_8 ... QUIC_FT_STREAM_F:
{
struct qcs *qcs = frm->stream.qcs;
struct quic_stream *strm = &frm->stream;
struct eb64_node *node = NULL;
struct qcs *qcs = NULL;
/* do not use strm->qcs as the qcs instance might be freed at
* this stage. Use the id to do a proper lookup.
*
* TODO if lookup operation impact on the perf is noticeable,
* implement a refcount on qcs instances.
*/
if (qc->mux_state == QC_MUX_READY) {
node = eb64_lookup(&qc->qcc->streams_by_id, strm->id);
qcs = eb64_entry(node, struct qcs, by_id);
}
if (!qcs) {
TRACE_PROTO("acked stream for released stream", QUIC_EV_CONN_ACKSTRM, qc, strm);
LIST_DELETE(&frm->list);
quic_tx_packet_refdec(frm->pkt);
pool_free(pool_head_quic_frame, frm);
/* early return */
return;
}
TRACE_PROTO("acked stream", QUIC_EV_CONN_ACKSTRM, qc, strm, qcs);
if (strm->offset.key <= qcs->tx.ack_offset) {