Skip to content

Commit 63fa64a

Browse files
jaideepkarandeinikep
authored andcommitted
PXC-5220 [9.7]: Fix SSL/socket resource leak in GCS XCom join retry path
== Root Cause == In Gcs_xcom_control::try_send_add_node_request_to_seeds(), a connection established by connect_to_peer() was only closed inside the if (!finalized && connected) block. If m_view_control->is_finalized() became true between the connect_to_peer() return and the condition check (a TOCTOU race during retry_do_join), the block was skipped entirely. free_connection() only calls free() on the connection_descriptor struct itself (node_connection.h:90) — it does NOT call SSL_free() or close the socket. The SSL object allocated by SSL_new() in timed_connect_ssl_msec() (xcom_network_provider_ssl_native_lib.cc:694), plus all memory allocated internally by SSL_connect() (session state, cipher context, etc.), was permanently lost. == Valgrind Report (Build 816, PXC 9.7.1-1 RelWithDebInfo Ubuntu Noble) == 96,664 (7,640 direct, 89,024 indirect) bytes in 1 blocks are definitely lost at malloc (vgpreload_memcheck) by CRYPTO_zalloc by SSL_new (libssl.so.3) by timed_connect_ssl_msec (xcom_network_provider_ssl_native_lib.cc:694) by Xcom_network_provider::open_connection (xcom_network_provider.cc:332) by Network_provider_manager::open_xcom_connection (network_provider_manager.cc:241) by Gcs_xcom_control::connect_to_peer (gcs_xcom_control_interface.cc:623) by Gcs_xcom_control::try_send_add_node_request_to_seeds(gcs_xcom_control_interface.cc:563) by Gcs_xcom_control::send_add_node_request (gcs_xcom_control_interface.cc:543) by Gcs_xcom_control::retry_do_join (gcs_xcom_control_interface.cc:477) by Gcs_xcom_control::do_join (gcs_xcom_control_interface.cc:291) Observed in 9 of 11 affected mysqld PIDs (PIDs 16098, 164923, 61642 …). Each leak is 7,640 bytes direct + ~89–98 KB indirect per occurrence. == Fix == Add an else-if branch that calls xcom_client_close_connection(con) whenever connected == true but finalized == true. This ensures the SSL object and socket are always released exactly once: • !finalized && connected → existing path: add_node + close • finalized && connected → new path: close only • !connected → close already done inside connect_to_peer (disable_nagle failure path) or ssl_fd is null (connect failure); nothing to do == Failing Tests (Build 816) == group_replication.gr_ssl_options group_replication.gr_ssl_tls13_runtime_valid_configuration group_replication.gr_recovery_tlsv13_* group_replication.gr_rejoin_bootstrap group_replication.gr_rejoin_no_bootstrap group_replication.gr_clone_integration_* group_replication.gr_acf_receiver_* group_replication.gr_flush_logs group_replication.gr_primary_mode_group_operations_22_1 group_replication.gr_reset_slave_channel == Developer Notes == * free_connection() (node_connection.h:88) is intentionally a bare free() — it does not own the SSL or socket lifetime. Only xcom_client_close_connection() / close_xcom_connection() drives the provider's close_connection() which calls ssl_free_con() -> SSL_free(). * The race is narrow but reproducible under Valgrind (slow process) or high-load retry scenarios where is_finalized() transitions while the SSL handshake is in progress. * No behaviour change for the normal path (not finalized): the existing xcom_client_close_connection() call inside the if-block is untouched.
1 parent 67e5763 commit 63fa64a

1 file changed

Lines changed: 4 additions & 0 deletions

File tree

plugin/group_replication/libmysqlgcs/src/bindings/xcom/gcs_xcom_control_interface.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,10 @@ bool Gcs_xcom_control::try_send_add_node_request_to_seeds(
593593
In this case, we continue the loop and try again using the next peer.
594594
*/
595595
if (xcom_will_process) add_node_accepted = true;
596+
} else if (connected) {
597+
/* GCS was finalized while we were connecting; close the connection to
598+
free the SSL object and socket so they are not leaked. */
599+
m_xcom_proxy->xcom_client_close_connection(con);
596600
}
597601

598602
free_connection(con);

0 commit comments

Comments
 (0)