From: Amaury Denoyelle Date: Tue, 31 Dec 2024 15:06:32 +0000 (+0100) Subject: BUG/MEDIUM: mux-quic: do not attach on already closed stream X-Git-Tag: v3.2-dev3~19 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=caf60ac696a29799631a76beb16d0072f65eef12;p=thirdparty%2Fhaproxy.git BUG/MEDIUM: mux-quic: do not attach on already closed stream Due to QUIC packet reordering, a stream may be opened via a new RESET_STREAM or STOP_SENDING frame. This would cause either Tx or Rx channel to be immediately closed. This can cause an issue with current QUIC MUX implementation with QCS purging. QCS are inserted into QCC purge list when transfer could be considered as completed. In most cases, this happens after full request/response exchange. However, it can also happens after request reception if RESET_STREAM/STOP_SENDING are received first. A BUG_ON() crash will occur if a STREAM frame is received after. In this case, streamdesc instance will be attached via qcs_attach_sc() to handle the new request. However, QCS is already considered eligible to purging. It could cause it to be released while its streamdesc instance remains. A BUG_ON() crash detects this problem in qcc_purge_streams(). To fix this, extend qcc_decode_qcs() to skip app proto rcv_buf invokation if QCS is considered completed. A similar condition was already implemented when read was previously aborted after a STOP_SENDING emission by QUIC MUX. This crash was reproduced on haproxy.org. Here is the output of the backtrace : Core was generated by `./haproxy-dev -db -f /etc/haproxy/haproxy-current.cfg -sf 16495'. Program terminated with signal SIGILL, Illegal instruction. #0 0x00000000004e442b in qcc_purge_streams (qcc=0x774cca0) at src/mux_quic.c:2661 2661 BUG_ON_HOT(!qcs_is_completed(qcs)); [Current thread is 1 (LWP 1457)] [ ## gdb ## ] bt #0 0x00000000004e442b in qcc_purge_streams (qcc=0x774cca0) at src/mux_quic.c:2661 #1 0x00000000004e4db7 in qcc_io_process (qcc=0x774cca0) at src/mux_quic.c:2744 #2 0x00000000004e5a54 in qcc_io_cb (t=0x7f71193940c0, ctx=0x774cca0, status=573504) at src/mux_quic.c:2886 #3 0x0000000000b4f792 in run_tasks_from_lists (budgets=0x7ffdcea1e670) at src/task.c:603 #4 0x0000000000b5012f in process_runnable_tasks () at src/task.c:883 #5 0x00000000007de4a3 in run_poll_loop () at src/haproxy.c:2771 #6 0x00000000007deb9f in run_thread_poll_loop (data=0x1335a00 ) at src/haproxy.c:2985 #7 0x00000000007dfd8d in main (argc=6, argv=0x7ffdcea1e958) at src/haproxy.c:3570 This BUG_ON() crash can only happen since 3.1 refactoring. Indeed, purge list was only implemented on this version. As such, please backport it on 3.1 immediately. However, a logic issue remains for older version as a stream could be attached on a fully closed QCS. Thus, it should be backported up to 2.8, this time after a period of observation. --- diff --git a/src/mux_quic.c b/src/mux_quic.c index 1394a36593..6b5f46d5e3 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -1152,7 +1152,7 @@ static int qcc_decode_qcs(struct qcc *qcc, struct qcs *qcs) if (qcs_is_close_remote(qcs)) fin = 1; - if (!(qcs->flags & QC_SF_READ_ABORTED)) { + if (!(qcs->flags & QC_SF_READ_ABORTED) && !qcs_is_completed(qcs)) { ret = qcc->app_ops->rcv_buf(qcs, &b, fin); if (qcc->glitches != prev_glitches)