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 <ha_thread_info>) 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
<conn> 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 6720f2f9aee875433f5b4de7f0e60c7b70b59b65..a832038dba767ac6aef1323f45bad0ba8a424a27 100644 (file)
--- 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 8912dfc19cc9e4b7924fccd5d4d7ed665b1165d1..4cbfa2c5327688af8fa7df9ad3c8d3dce48b1fae 100644 (file)
--- 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;
        }