From: Amaury Denoyelle Date: Mon, 16 Jun 2025 08:42:34 +0000 (+0200) Subject: BUG/MEDIUM: quic: do not release BE quic-conn prior to upper conn X-Git-Tag: v3.3-dev2~29 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=74b95922efd890c7ddd6f916c6f6995e536d5362;p=thirdparty%2Fhaproxy.git BUG/MEDIUM: quic: do not release BE quic-conn prior to upper conn For frontend side, quic_conn is only released if MUX wasn't allocated, either due to handshake abort, in which case upper layer is never allocated, or after transfer completion when full conn + MUX layers are already released. On the backend side, initialization is not performed in the same order. Indeed, in this case, connection is first instantiated, the nthe quic_conn is created to execute the handshake, while MUX is still only allocated on handshake completion. As such, it is not possible anymore to free immediately quic_conn on handshake failure. Else, this can cause crash if the connection try to reaccess to its transport layer after quic_conn release. Such crash can easily be reproduced in case of connection error to the QUIC server. Here is an example of an experienced backtrace. Thread 1 "haproxy" received signal SIGSEGV, Segmentation fault. 0x0000555555739733 in quic_close (conn=0x55555734c0d0, xprt_ctx=0x5555573a6e50) at src/xprt_quic.c:28 28 qc->conn = NULL; [ ## gdb ## ] bt #0 0x0000555555739733 in quic_close (conn=0x55555734c0d0, xprt_ctx=0x5555573a6e50) at src/xprt_quic.c:28 #1 0x00005555559c9708 in conn_xprt_close (conn=0x55555734c0d0) at include/haproxy/connection.h:162 #2 0x00005555559c97d2 in conn_full_close (conn=0x55555734c0d0) at include/haproxy/connection.h:206 #3 0x00005555559d01a9 in sc_detach_endp (scp=0x7fffffffd648) at src/stconn.c:451 #4 0x00005555559d05b9 in sc_reset_endp (sc=0x55555734bf00) at src/stconn.c:533 #5 0x000055555598281d in back_handle_st_cer (s=0x55555734adb0) at src/backend.c:2754 #6 0x000055555588158a in process_stream (t=0x55555734be10, context=0x55555734adb0, state=516) at src/stream.c:1907 #7 0x0000555555dc31d9 in run_tasks_from_lists (budgets=0x7fffffffdb30) at src/task.c:655 #8 0x0000555555dc3dd3 in process_runnable_tasks () at src/task.c:889 #9 0x0000555555a1daae in run_poll_loop () at src/haproxy.c:2865 #10 0x0000555555a1e20c in run_thread_poll_loop (data=0x5555569d1c00 ) at src/haproxy.c:3081 #11 0x0000555555a1f66b in main (argc=5, argv=0x7fffffffde18) at src/haproxy.c:3671 To fix this, change the condition prior to calling quic_conn release. If member is not NULL, delay the release, similarly to the case when MUX is allocated. This allows connection to be freed first, and detach from quic_conn layer through close xprt operation. No need to backport. --- diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h index 6720f2f9a..a832038db 100644 --- a/include/haproxy/quic_conn-t.h +++ b/include/haproxy/quic_conn-t.h @@ -251,7 +251,7 @@ extern const struct quic_version *quic_version_2; /* The maximum number of bytes of CRYPTO data in flight during handshakes. */ #define QUIC_CRYPTO_IN_FLIGHT_MAX 4096 -/* Status of the connection/mux layer. This defines how to handle app data. +/* Status of the MUX layer. This defines how to handle app data. * * During a standard quic_conn lifetime it transitions like this : * QC_MUX_NULL -> QC_MUX_READY -> QC_MUX_RELEASED diff --git a/src/quic_conn.c b/src/quic_conn.c index 8912dfc19..4cbfa2c53 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -628,7 +628,7 @@ struct task *quic_conn_app_io_cb(struct task *t, void *context, unsigned int sta } out: - if ((qc->flags & QUIC_FL_CONN_CLOSING) && qc->mux_state != QC_MUX_READY) { + if ((qc->flags & QUIC_FL_CONN_CLOSING) && !qc->conn) { if (quic_conn_release(qc)) t = NULL; qc = NULL; @@ -942,7 +942,7 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state) quic_nictx_free(qc); } - if ((qc->flags & QUIC_FL_CONN_CLOSING) && qc->mux_state != QC_MUX_READY) { + if ((qc->flags & QUIC_FL_CONN_CLOSING) && !qc->conn) { if (quic_conn_release(qc)) t = NULL; qc = NULL; @@ -1494,8 +1494,8 @@ int quic_conn_release(struct quic_conn *qc) /* Must not delete a quic_conn if thread affinity rebind in progress. */ BUG_ON(qc->flags & QUIC_FL_CONN_TID_REBIND); - /* We must not free the quic-conn if the MUX is still allocated. */ - BUG_ON(qc->mux_state == QC_MUX_READY); + /* We must not free the quic-conn if upper conn is still allocated. */ + BUG_ON(qc->conn); cc_qc = NULL; if ((qc->flags & QUIC_FL_CONN_CLOSING) && !(qc->flags & QUIC_FL_CONN_EXP_TIMER) && @@ -1760,7 +1760,7 @@ struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int state) * responsible to call quic_close to release it. */ qc->flags |= QUIC_FL_CONN_EXP_TIMER; - if (qc->mux_state != QC_MUX_READY) { + if (!qc->conn) { quic_conn_release(qc); qc = NULL; }