From b776304ed7ae0eb76c58ceec19da92024f945f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Fri, 22 Apr 2022 15:59:00 +0200 Subject: [PATCH] Fix invalid memory accesses by 'ProxySQL_Node_Entry::set_checksums' #3847 --- lib/ProxySQL_Cluster.cpp | 77 ++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/lib/ProxySQL_Cluster.cpp b/lib/ProxySQL_Cluster.cpp index 7f7cbc41b..ba42e4324 100644 --- a/lib/ProxySQL_Cluster.cpp +++ b/lib/ProxySQL_Cluster.cpp @@ -683,41 +683,6 @@ void ProxySQL_Node_Entry::set_checksums(MYSQL_RES *_r) { } } } - if (diff_ps) { - v = &checksums_values.proxysql_servers; - unsigned long long own_version = __sync_fetch_and_add(&GloVars.checksums_values.proxysql_servers.version,0); - unsigned long long own_epoch = __sync_fetch_and_add(&GloVars.checksums_values.proxysql_servers.epoch,0); - char* own_checksum = __sync_fetch_and_add(&GloVars.checksums_values.proxysql_servers.checksum,0); - if (v->version > 1) { - if ( - (own_version == 1) // we just booted - || - (v->epoch > own_epoch) // epoch is newer - ) { - if (v->diff_check >= diff_ps) { - proxy_info("Cluster: detected a peer %s:%d with proxysql_servers version %llu, epoch %llu, diff_check %u. Own version: %llu, epoch: %llu. Proceeding with remote sync\n", hostname, port, v->version, v->epoch, v->diff_check, own_version, own_epoch); - // v->checksum will be destroyed when calling pull_proxysql_servers_from_peer() - // thus we need to copy it now - char *expected_checksum = strdup(v->checksum); - GloProxyCluster->pull_proxysql_servers_from_peer((const char *)expected_checksum); - if (strncmp(expected_checksum, GloVars.checksums_values.proxysql_servers.checksum, 20)==0) { - // we copied from the remote server, let's also copy the same epoch - GloVars.checksums_values.proxysql_servers.epoch = v->epoch; - } - free(expected_checksum); - } - } - if ((v->epoch == own_epoch) && v->diff_check && ((v->diff_check % (diff_ps*10)) == 0)) { - proxy_error("Cluster: detected a peer %s:%d with proxysql_servers version %llu, epoch %llu, diff_check %u, checksum %s. Own version: %llu, epoch: %llu, checksum %s. Sync conflict, epoch times are EQUAL, can't determine which server holds the latest config, we won't sync. This message will be repeated every %llu checks until LOAD MYSQL SERVERS TO RUNTIME is executed on candidate master.\n", hostname, port, v->version, v->epoch, v->diff_check, v->checksum, own_version, own_epoch, own_checksum, (diff_ps*10)); - GloProxyCluster->metrics.p_counter_array[p_cluster_counter::sync_conflict_proxysql_servers_share_epoch]->Increment(); - } - } else { - if (v->diff_check && (v->diff_check % (diff_ps*10)) == 0) { - proxy_warning("Cluster: detected a peer %s:%d with proxysql_servers version %llu, epoch %llu, diff_check %u. Own version: %llu, epoch: %llu. diff_check is increasing, but version 1 doesn't allow sync. This message will be repeated every %llu checks until LOAD PROXYSQL SERVERS TO RUNTIME is executed on candidate master.\n", hostname, port, v->version, v->epoch, v->diff_check, own_version, own_epoch, (diff_ps*10)); - GloProxyCluster->metrics.p_counter_array[p_cluster_counter::sync_delayed_proxysql_servers_version_one]->Increment(); - } - } - } if (diff_mv) { v = &checksums_values.mysql_variables; unsigned long long own_version = __sync_fetch_and_add(&GloVars.checksums_values.mysql_variables.version, 0); @@ -814,6 +779,48 @@ void ProxySQL_Node_Entry::set_checksums(MYSQL_RES *_r) { } } } + // IMPORTANT-NOTE: This action should ALWAYS be performed the last, since the 'checksums_values' gets + // invalidated by 'pull_proxysql_servers_from_peer' and further memory accesses would be invalid. + if (diff_ps) { + v = &checksums_values.proxysql_servers; + unsigned long long own_version = __sync_fetch_and_add(&GloVars.checksums_values.proxysql_servers.version,0); + unsigned long long own_epoch = __sync_fetch_and_add(&GloVars.checksums_values.proxysql_servers.epoch,0); + char* own_checksum = __sync_fetch_and_add(&GloVars.checksums_values.proxysql_servers.checksum,0); + if (v->version > 1) { + // NOTE: Backup values: 'v' gets invalidated by 'pull_proxysql_servers_from_peer()' + unsigned long long v_epoch = v->epoch; + unsigned long long v_version = v->version; + unsigned int v_diff_check = v->diff_check; + char* v_exp_checksum = strdup(v->checksum); + + if ( + (own_version == 1) // we just booted + || + (v->epoch > own_epoch) // epoch is newer + ) { + if (v->diff_check >= diff_ps) { + proxy_info("Cluster: detected a peer %s:%d with proxysql_servers version %llu, epoch %llu, diff_check %u. Own version: %llu, epoch: %llu. Proceeding with remote sync\n", hostname, port, v->version, v->epoch, v->diff_check, own_version, own_epoch); + // thus we need to copy it now + GloProxyCluster->pull_proxysql_servers_from_peer((const char *)v_exp_checksum); + if (strncmp(v_exp_checksum, GloVars.checksums_values.proxysql_servers.checksum, 20)==0) { + // we copied from the remote server, let's also copy the same epoch + GloVars.checksums_values.proxysql_servers.epoch = v_epoch; + } + } + } + if ((v_epoch == own_epoch) && v_diff_check && ((v_diff_check % (diff_ps*10)) == 0)) { + proxy_error("Cluster: detected a peer %s:%d with proxysql_servers version %llu, epoch %llu, diff_check %u, checksum %s. Own version: %llu, epoch: %llu, checksum %s. Sync conflict, epoch times are EQUAL, can't determine which server holds the latest config, we won't sync. This message will be repeated every %llu checks until LOAD MYSQL SERVERS TO RUNTIME is executed on candidate master.\n", hostname, port, v_version, v_epoch, v_diff_check, v_exp_checksum, own_version, own_epoch, own_checksum, (diff_ps*10)); + GloProxyCluster->metrics.p_counter_array[p_cluster_counter::sync_conflict_proxysql_servers_share_epoch]->Increment(); + } + + free(v_exp_checksum); + } else { + if (v->diff_check && (v->diff_check % (diff_ps*10)) == 0) { + proxy_warning("Cluster: detected a peer %s:%d with proxysql_servers version %llu, epoch %llu, diff_check %u. Own version: %llu, epoch: %llu. diff_check is increasing, but version 1 doesn't allow sync. This message will be repeated every %llu checks until LOAD PROXYSQL SERVERS TO RUNTIME is executed on candidate master.\n", hostname, port, v->version, v->epoch, v->diff_check, own_version, own_epoch, (diff_ps*10)); + GloProxyCluster->metrics.p_counter_array[p_cluster_counter::sync_delayed_proxysql_servers_version_one]->Increment(); + } + } + } } void ProxySQL_Cluster::pull_mysql_query_rules_from_peer() {