BUG/MEDIUM: quic: do not use qcs from quic_stream on ACK parsing
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.
diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index d0beb50..c200370 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -1455,8 +1455,30 @@
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) {