From b0e32c626360f06b6572439374627fef3b2e17e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Fri, 11 Aug 2023 11:21:31 +0200 Subject: [PATCH] BUG/MINOR: quic: Possible crash when issuing "show fd/sess" CLI commands ->xprt_ctx (struct ssl_sock_ctx) and ->conn (struct connection) must be kept by the remaining QUIC connection object (struct quic_cc_conn) after having release the previous one (struct quic_conn) to allow "show fd/sess" commands to be functional without causing haproxy crashes. No need to backport. --- include/haproxy/quic_conn-t.h | 9 +++++---- src/quic_conn.c | 18 +++++++++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h index a8472add5..92d1323ca 100644 --- a/include/haproxy/quic_conn-t.h +++ b/include/haproxy/quic_conn-t.h @@ -495,6 +495,11 @@ struct quic_conn_cntrs { /* Idle timer task */ \ struct task *idle_timer_task; \ unsigned int idle_expire; \ + struct ssl_sock_ctx *xprt_ctx; \ + /* Used only to reach the tasklet for the I/O handler from this \ + * quic_conn object. \ + */ \ + struct connection *conn; \ } struct quic_conn { @@ -531,14 +536,10 @@ struct quic_conn { /* List of packet number spaces attached to this connection */ struct list pktns_list; - struct ssl_sock_ctx *xprt_ctx; #ifdef USE_QUIC_OPENSSL_COMPAT struct quic_openssl_compat openssl_compat; #endif - /* Used only to reach the tasklet for the I/O handler from this quic_conn object. */ - struct connection *conn; - struct { /* Transport parameters sent by the peer */ struct quic_transport_params params; diff --git a/src/quic_conn.c b/src/quic_conn.c index 6751b360e..d0ffc992c 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -756,6 +756,8 @@ static void quic_release_cc_conn(struct quic_cc_conn *cc_qc) { struct quic_conn *qc = (struct quic_conn *)cc_qc; + TRACE_ENTER(QUIC_EV_CONN_IO_CB, cc_qc); + if (qc_test_fd(qc)) _HA_ATOMIC_DEC(&jobs); @@ -769,7 +771,11 @@ static void quic_release_cc_conn(struct quic_cc_conn *cc_qc) cc_qc->cids = NULL; pool_free(pool_head_quic_cc_buf, cc_qc->cc_buf_area); cc_qc->cc_buf_area = NULL; + /* free the SSL sock context */ + qc_free_ssl_sock_ctx(&cc_qc->xprt_ctx); pool_free(pool_head_quic_cc_conn, cc_qc); + + TRACE_ENTER(QUIC_EV_CONN_IO_CB); } /* QUIC connection packet handler task used when in "closing connection" state. */ @@ -872,8 +878,15 @@ static struct quic_cc_conn *qc_new_cc_conn(struct quic_conn *qc) cc_qc->idle_timer_task->context = cc_qc; cc_qc->idle_expire = qc->idle_expire; + cc_qc->xprt_ctx = qc->xprt_ctx; + qc->xprt_ctx = NULL; + cc_qc->conn = qc->conn; + qc->conn = NULL; + cc_qc->cc_buf_area = qc->tx.cc_buf_area; cc_qc->cc_dgram_len = qc->tx.cc_dgram_len; + TRACE_PRINTF(TRACE_LEVEL_PROTO, QUIC_EV_CONN_IO_CB, qc, 0, 0, 0, + "switch qc@%p to cc_qc@%p", qc, cc_qc); return cc_qc; } @@ -1418,6 +1431,8 @@ void quic_conn_release(struct quic_conn *qc) qc->cids = NULL; pool_free(pool_head_quic_cc_buf, qc->tx.cc_buf_area); qc->tx.cc_buf_area = NULL; + /* free the SSL sock context */ + qc_free_ssl_sock_ctx(&qc->xprt_ctx); } /* in the unlikely (but possible) case the connection was just added to @@ -1449,9 +1464,6 @@ void quic_conn_release(struct quic_conn *qc) task_destroy(qc->timer_task); qc->timer_task = NULL; - /* free the SSL sock context */ - qc_free_ssl_sock_ctx(&qc->xprt_ctx); - quic_tls_ku_free(qc); if (qc->ael) { struct quic_tls_ctx *actx = &qc->ael->tls_ctx;