Merge pull request #5191 from sysown/v3.0-fix_proclist_use_after_free

Fix heap-after-use between `PROXYSQL INTERNAL SESSION` and `SHOW PROCESSLIST`

lib/Base_Session.cpp

@@ -304,6 +304,8 @@ void Base_Session<S, DS, B, T>::return_proxysql_internal(PtrSize_t* pkt) {
 			bool deprecate_eof_active = client_myds->myconn->options.client_flag & CLIENT_DEPRECATE_EOF;
 			SQLite3_to_MySQL(resultset, NULL, 0, &client_myds->myprot, false, deprecate_eof_active);
 			delete resultset;
+			// NOTE: End request before freeing the packet; otherwise logging could use invalid memory
+			static_cast<MySQL_Session*>(this)->RequestEnd(NULL);
 			l_free(pkt->size, pkt->ptr);
 			return;
 		}
@@ -331,6 +333,8 @@ void Base_Session<S, DS, B, T>::return_proxysql_internal(PtrSize_t* pkt) {
 			char txn_state = (nTxn ? 'T' : 'I');
 			SQLite3_to_Postgres(client_myds->PSarrayOUT, resultset, nullptr, 0, (const char*)pkt->ptr + 5, txn_state);
 			delete resultset;
+			// NOTE: End request before freeing the packet; otherwise logging could use invalid memory
+			static_cast<PgSQL_Session*>(this)->RequestEnd(NULL, false);
 			l_free(pkt->size, pkt->ptr);
 			return;
 		}

test/tap/groups/groups.json

@@ -53,6 +53,7 @@
   "prepare_statement_err3024_async-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ],
   "prepare_statement_err3024_libmysql-t" : [ "default-g2","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2" ],
   "prepare_statement_err3024-t" : [ "default-g2","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2" ],
+  "reg_test_proclist_use_after_free-t" : [ "default-g2","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2" ], 
   "reg_test_1493-mixed_compression-t" : [ "default-g2","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2" ],
   "reg_test_1574-mariadb_read_stmt_execute_response-t" : [ "default-g2","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2" ],
   "reg_test_1574-stmt_metadata-t" : [ "default-g2","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2" ],

test/tap/tests/reg_test_proclist_use_after_free-t.cpp

@@ -0,0 +1,87 @@
+/**
+ * @file reg_test_proclist_use_after_free-t.cpp
+ * @brief Created to find heap-after-use flow in 'SHOW PROCESSLIST'.
+ * @details Left as a regression test exercising the offending flow for ASAN runs.
+ */
+
+#include <atomic>
+#include <cstring>
+#include <stdio.h>
+#include <thread>
+
+#include "mysql.h"
+
+#include "tap.h"
+#include "command_line.h"
+#include "utils.h"
+
+int main(int argc, char** argv) {
+	CommandLine cl;
+
+	if (cl.getEnv()) {
+		diag("Failed to get the required environmental variables.");
+		return EXIT_FAILURE;
+	}
+
+	MYSQL* proxy = mysql_init(NULL);
+	MYSQL* admin = mysql_init(NULL);
+
+	if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy));
+		return EXIT_FAILURE;
+	}
+	if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin));
+		return EXIT_FAILURE;
+	}
+
+	MYSQL_QUERY_T(proxy,
+		"CREATE TABLE IF NOT EXISTS test.auto_inc_multiplex "
+			"(c1 INT NOT NULL AUTO_INCREMENT PRIMARY KEY, c2 VARCHAR(100), c3 VARCHAR(100))"
+	);
+	MYSQL_QUERY_T(admin, "SET mysql-auto_increment_delay_multiplex=5");
+	MYSQL_QUERY_T(admin, "LOAD MYSQL VARIABLES TO RUNTIME");
+
+	std::atomic<bool> p_done { false };
+
+	std::thread a_thread([&admin, &p_done] () -> int {
+		uint32_t timeout = 5;
+		uint32_t st_time = time(NULL);
+		uint32_t elapsed = 0;
+
+		while (p_done == false && elapsed < timeout) {
+			MYSQL_QUERY_T(admin, "SHOW PROCESSLIST");
+			mysql_free_result(mysql_store_result(admin));
+
+			elapsed = time(NULL) - st_time;
+		}
+
+		return 0;
+	});
+
+	std::thread p_thread([&proxy, &p_done] () -> int {
+		for (size_t c = 0; c < 10; c++) {
+			MYSQL_QUERY_T(proxy, "INSERT INTO test.auto_inc_multiplex (c2, c3) VALUES ('foo','bar')");
+
+			for (size_t i = 0; i < 5; i++) {
+				MYSQL_QUERY_T(proxy, "DO 1");
+
+				MYSQL_QUERY_T(proxy, "PROXYSQL INTERNAL SESSION");
+				mysql_free_result(mysql_store_result(proxy));
+			}
+		}
+
+		p_done = true;
+		return 0;
+	});
+
+	p_thread.join();
+	a_thread.join();
+
+	MYSQL_QUERY_T(proxy, "DELETE FROM test.auto_inc_multiplex");
+
+	mysql_close(proxy);
+	mysql_close(admin);
+
+	return exit_status();
+}