BUG/MEDIUM: mux_quic: prevent risk of infinite loop on recv

When a RESET_STREAM is received, QCS Rx channel is closed and pending Rx
data and buf are cleared without being transmitted to upper stream
layer.

This can cause an issue if this QCS instance is present in the QCC
recv_list. When qcc_io_recv() is executed after reset handling, an
infinite loop is triggered for the QCS instance as qcs_rx_avail_data()
always return 0.

This issue happened due to the poor writing of the while loop in
qcc_io_recv() which is not correctly protected against infinite
execution.

To prevent this issue, this patch rewrites the loop. Crucially,
LIST_DEL_INIT() is now performed unconditionally outside of the inner
loop. This guarantees that even if the inner loop is not executed, the
stream will be removed from QCC recv_list and iteration will progress.

This is functionally correct as a QCS should not be present in recv_list
if there is no avail data or demux is currently blocked. For the first
condition, qcc_decode_qcs() will be called again when new data is read
unless demux is blocked. In this case, QCS will be reinserted in the
list on unblocking, with a rescheduling to invoke qcc_decode_qcs().

In the context of the currently found reproducer linked to stream reset,
the QCS instance can be safely removed from the recv_list without
implication.

This must be backported up to 3.2.

diff --git a/src/mux_quic.c b/src/mux_quic.c
index a2222be37582ac30297d75c5e5c95f092e160180..f1187cdfeac582be905150df40222572d83448c9 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -3322,12 +3322,22 @@ static int qcc_io_recv(struct qcc *qcc)
 
                while (qcs_rx_avail_data(qcs) && !(qcs->flags & QC_SF_DEM_FULL)) {
                        ret = qcc_decode_qcs(qcc, qcs);
-                       LIST_DEL_INIT(&qcs->el_recv);
-
-                       if (ret <= 0)
+                       if (ret <= 0) {
+                               LIST_DEL_INIT(&qcs->el_recv);
                                goto done;
+                       }
+
                        total += ret;
                }
+
+               /* Always remove QCS from recv_list to prevent infinite loop.
+                * This is performed even if inner loop was not executed : QCS
+                * has nothing to do in recv_list if no avail Rx data or demux
+                * is blocked. Next decoding will be performed on new data read
+                * unless demux is blocked. In this case QCS will be reinserted
+                * in recv_list on unblocking to execute decode here again.
+                */
+               LIST_DEL_INIT(&qcs->el_recv);
        }
 
  done: