Merge pull request #3602 from sysown/v2.2.1-3601

Closes #3601: Backport bugfixes to v2.2.1 - 4

Author: JavierJF

lib/MySQL_Protocol.cpp

@@ -2455,8 +2455,19 @@ stmt_execute_metadata_t * MySQL_Protocol::get_binds_from_pkt(void *ptr, unsigned
 			uint8_t null_byte=null_bitmap[i/8];
 			uint8_t idx=i%8;
 			my_bool is_null = (null_byte & ( 1 << idx )) >> idx;
+			if (binds[i].buffer_type == MYSQL_TYPE_NULL)
+				is_null = 1;
 			is_nulls[i]=is_null;
 			binds[i].is_null=&is_nulls[i];
+			// set length, defaults to 0
+			// for parameters with not fixed length, that will be assigned later
+			// we moved this initialization here due to #3585
+			binds[i].is_unsigned=0;
+			lengths[i]=0;
+			binds[i].length=&lengths[i];
+			// NOTE: We nullify buffers here to reflect that memory wasn't
+			// initalized. See #3546.
+			binds[i].buffer = NULL;
 		}
 		free(null_bitmap); // we are done with it
 
@@ -2475,10 +2486,6 @@ stmt_execute_metadata_t * MySQL_Protocol::get_binds_from_pkt(void *ptr, unsigned
 				binds[i].buffer_type=(enum enum_field_types)buffer_type;
 				p+=2;
 
-				// set length, defaults to 0
-				// for parameters with not fixed length, that will be assigned later
-				lengths[i]=0;
-				binds[i].length=&lengths[i];
 			}
 		}
 
@@ -2527,11 +2534,13 @@ stmt_execute_metadata_t * MySQL_Protocol::get_binds_from_pkt(void *ptr, unsigned
 						p++;
 						MYSQL_TIME ts;
 						memset(&ts,0,sizeof(MYSQL_TIME));
-						memcpy(&ts.neg,p,1);
-						memcpy(&ts.day,p+1,4);
-						memcpy(&ts.hour,p+5,1);
-						memcpy(&ts.minute,p+6,1);
-						memcpy(&ts.second,p+7,1);
+						if (l) {
+							memcpy(&ts.neg,p,1);
+							memcpy(&ts.day,p+1,4);
+							memcpy(&ts.hour,p+5,1);
+							memcpy(&ts.minute,p+6,1);
+							memcpy(&ts.second,p+7,1);
+						}
 						if (l>8) {
 							memcpy(&ts.second_part,p+8,4);
 						}
@@ -2549,9 +2558,11 @@ stmt_execute_metadata_t * MySQL_Protocol::get_binds_from_pkt(void *ptr, unsigned
 						p++;
 						MYSQL_TIME ts;
 						memset(&ts,0,sizeof(MYSQL_TIME));
-						memcpy(&ts.year,p,2);
-						memcpy(&ts.month,p+2,1);
-						memcpy(&ts.day,p+3,1);
+						if (l) {
+							memcpy(&ts.year,p,2);
+							memcpy(&ts.month,p+2,1);
+							memcpy(&ts.day,p+3,1);
+						}
 						if (l>4) {
 							memcpy(&ts.hour,p+4,1);
 							memcpy(&ts.minute,p+5,1);

lib/MySQL_Session.cpp

@@ -3951,6 +3951,9 @@ void MySQL_Session::handler_rc0_PROCESSING_STMT_EXECUTE(MySQL_Data_Stream *myds)
 				(buffer_type == MYSQL_TYPE_DATETIME)
 			) {
 				free(CurrentQuery.stmt_meta->binds[i].buffer);
+				// NOTE: This memory should be zeroed during initialization,
+				// but we also nullify it here for extra safety. See #3546.
+				CurrentQuery.stmt_meta->binds[i].buffer = NULL;
 			}
 		}
 	}

lib/ProxySQL_RESTAPI_Server.cpp

@@ -663,7 +663,12 @@ class gen_get_endpoint : public http_resource {
 
 void * restapi_server_thread(void *arg) {
 	httpserver::webserver * ws = (httpserver::webserver *)arg;
-    ws->start(true);
+	try {
+		ws->start(true);
+	} catch (const std::exception& ex) {
+		proxy_error("Exception starting 'RESTAPI Server': %s\n", ex.what());
+		throw;
+	}
 	return NULL;
 }
 

lib/mysql_connection.cpp

@@ -1003,7 +1003,7 @@ MDB_ASYNC_ST MySQL_Connection::handler(short event) {
 			}
 			if (!ret_mysql) {
 				// always increase the counter
-				proxy_error("Failed to mysql_real_connect() on %s:%d , FD (Conn:%d , MyDS:%d) , %d: %s.\n", parent->address, parent->port, mysql->net.fd , myds->fd, mysql_errno(mysql), mysql_error(mysql));
+				proxy_error("Failed to mysql_real_connect() on %u:%s:%d , FD (Conn:%d , MyDS:%d) , %d: %s.\n", parent->myhgc->hid, parent->address, parent->port, mysql->net.fd , myds->fd, mysql_errno(mysql), mysql_error(mysql));
     		NEXT_IMMEDIATE(ASYNC_CONNECT_FAILED);
 			} else {
     		NEXT_IMMEDIATE(ASYNC_CONNECT_SUCCESSFUL);

test/tap/tests/reg_test_3546-stmt_empty_params-t.cpp

@@ -0,0 +1,257 @@
+/**
+ * @file reg_test_3546-stmt_empty_params-t.cpp
+ * @brief This test is a regression test for exercising the code path that lead
+ *   to issue #3546. It's not meant to test a specific feature, but to server as
+ *   a regression test that should flag the issue under a memory analyzer.
+ * @details Memory corruption related to #3546 was double-free provoqued when a
+ *   prepared statement with param of types ['MYSQL_TYPE_DATE'|'MYSQL_TYPE_TIMESTAMP'|'MYSQL_TYPE_DATETIME'|'MYSQL_TYPE_TIME'],
+ *   was prepared and a later prepared  with 'NULL' parameters. Because the memory
+ *   for the buffered was not zeroed neither at initialization or during the later
+ *   `free` a corruption takes place during the second execution.
+ */
+
+#include <iostream>
+#include <chrono>
+#include <ctime>
+#include <cstring>
+#include <unistd.h>
+#include <time.h>
+#include <vector>
+#include <string>
+#include <stdio.h>
+
+#include <mysql.h>
+#include <mysql/mysqld_error.h>
+
+#include "proxysql_utils.h"
+#include "tap.h"
+#include "command_line.h"
+#include "utils.h"
+#include "errno.h"
+
+/**
+ * @brief String size of the columns created for the testing table.
+ */
+const int STRING_SIZE=32;
+/**
+ * @brief Number of iterations to perform.
+ */
+const uint32_t ITERATIONS = 100;
+/**
+ * @brief Id for the current writer hostgroup.
+ */
+const uint32_t WRITER_HOSTGROUP_ID = 0;
+
+int prepare_stmt(
+	MYSQL* proxysql_mysql, MYSQL_STMT* stmt, MYSQL_TIME* ts, my_bool* is_null
+) {
+	int res = EXIT_SUCCESS;
+	std::string query {
+		"SELECT /*+ ;hostgroup=0 */ id,c1,c2 FROM test.reg_test_3546 WHERE date IN (?)"
+	};
+
+	if (mysql_stmt_prepare(stmt, query.c_str(), strlen(query.c_str()))) {
+		diag("mysql_stmt_prepare at line %d failed: %s", __LINE__ , mysql_error(proxysql_mysql));
+		mysql_close(proxysql_mysql);
+		res = EXIT_FAILURE;
+		goto exit;
+	}
+
+	MYSQL_BIND bind_params;
+
+	memset(&bind_params, 0, sizeof(MYSQL_BIND));
+	bind_params.buffer_type= MYSQL_TYPE_DATE;
+	bind_params.buffer= ts;
+	bind_params.is_null= is_null;
+	bind_params.length= 0;
+
+	if (mysql_stmt_bind_param(stmt, &bind_params)) {
+		diag(
+			"mysql_stmt_bind_result at line %d failed: %s", __LINE__ ,
+			mysql_stmt_error(stmt)
+		);
+		res = EXIT_FAILURE;
+		goto exit;
+	}
+
+exit:
+	return res;
+}
+
+int main(int argc, char** argv) {
+
+	CommandLine cl;
+
+	plan(ITERATIONS);
+
+	if (cl.getEnv()) {
+		diag("Failed to get the required environmental variables.");
+		return -1;
+	}
+
+	MYSQL_STMT* stmt_param = nullptr;
+	MYSQL* proxysql_mysql = mysql_init(NULL);
+	MYSQL* proxysql_admin = mysql_init(NULL);
+
+	if (!mysql_real_connect(proxysql_mysql, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_mysql));
+		return -1;
+	}
+
+	if (!mysql_real_connect(proxysql_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(proxysql_admin));
+		return -1;
+	}
+
+	stmt_param = mysql_stmt_init(proxysql_mysql);
+	if (!stmt_param) {
+		diag("mysql_stmt_init(), out of memory");
+		goto exit;
+	}
+
+	// Insert the row to be queried with the prepared statement.
+	// *************************************************************************
+	MYSQL_QUERY(proxysql_mysql, "CREATE DATABASE IF NOT EXISTS test");
+	MYSQL_QUERY(proxysql_mysql, "DROP TABLE IF EXISTS test.reg_test_3546");
+	MYSQL_QUERY(
+		proxysql_mysql,
+		"CREATE TABLE IF NOT EXISTS test.reg_test_3546"
+		" (id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, `c1` BIGINT, `c2` varchar(32), `date` DATE)"
+	);
+	MYSQL_QUERY(proxysql_mysql, "INSERT INTO test.reg_test_3546(c1, c2, date) VALUES (100, 'abcde', '2009-01-01')");
+	mysql_close(proxysql_mysql);
+
+	// *************************************************************************
+
+	// Initialize the connection again
+	proxysql_mysql = mysql_init(NULL);
+
+	if (!mysql_real_connect(proxysql_mysql, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_mysql));
+		return -1;
+	}
+
+	{
+		// Set the number of maximum connections for servers in the writer hostgroup
+		std::string t_update_mysql_servers {
+			"UPDATE mysql_servers SET max_connections=1 WHERE hostgroup_id=%d"
+		};
+		std::string update_mysql_queries {};
+		string_format(t_update_mysql_servers, update_mysql_queries, WRITER_HOSTGROUP_ID);
+		MYSQL_QUERY(proxysql_admin, update_mysql_queries.c_str());
+		MYSQL_QUERY(proxysql_admin, "LOAD MYSQL SERVERS TO RUNTIME");
+
+		MYSQL_TIME ts;
+		char data_param[STRING_SIZE] = {};
+		my_bool is_null = 0;
+
+		if (prepare_stmt(proxysql_mysql, stmt_param, &ts, &is_null)) {
+			diag("'prepare_stmt' at line %d failed", __LINE__);
+			goto exit;
+		}
+
+		// Prepare parameters
+		ts.year = 2009;
+		ts.month = 1;
+		ts.day = 1;
+
+		for (uint32_t i = 0; i < ITERATIONS; i++) {
+			if (i % 2) {
+				is_null = 0;
+			} else {
+				is_null = 1;
+			}
+
+			if (mysql_stmt_execute(stmt_param)) {
+				diag(
+					"'mysql_stmt_execute' at line %d failed: %s", __LINE__ ,
+					mysql_stmt_error(stmt_param)
+				);
+				goto exit;
+			}
+
+			MYSQL_BIND bind[3];
+			memset(bind, 0, sizeof(bind));
+
+			int data_id = 0;
+			int64_t data_c1 = 0;
+			char data_c2[STRING_SIZE] { 0 };
+			char is_null[3] { 0 };
+			long unsigned int length[3] { 0 };
+			char error[3] { 0 };
+
+			bind[0].buffer_type = MYSQL_TYPE_LONG;
+			bind[0].buffer = (char *)&data_id;
+			bind[0].buffer_length = sizeof(int);
+			bind[0].is_null = &is_null[0];
+			bind[0].length = &length[0];
+
+			bind[1].buffer_type = MYSQL_TYPE_LONGLONG;
+			bind[1].buffer = (char *)&data_c1;
+			bind[1].buffer_length = sizeof(int64_t);
+			bind[1].is_null = &is_null[1];
+			bind[1].length = &length[1];
+
+			bind[2].buffer_type = MYSQL_TYPE_STRING;
+			bind[2].buffer = (char *)&data_c2;
+			bind[2].buffer_length = STRING_SIZE;
+			bind[2].is_null = &is_null[2];
+			bind[2].length = &length[2];
+			bind[2].error = &error[2];
+
+			if (mysql_stmt_bind_result(stmt_param, bind)) {
+				diag(
+					"mysql_stmt_bind_result at line %d failed: %s", __LINE__,
+					mysql_stmt_error(stmt_param)
+				);
+				goto exit;
+			}
+
+			int fetch_result = mysql_stmt_fetch(stmt_param);
+			if (fetch_result == 1) {
+				diag(
+					"mysql_stmt_fetch at line %d failed: %s", __LINE__,
+					mysql_stmt_error(stmt_param)
+				);
+				goto exit;
+			}
+
+			if (i % 2) {
+				bool data_match_expected =
+					(data_id == static_cast<int64_t>(1)) &&
+					(data_c1 == static_cast<int64_t>(100)) &&
+					(strcmp(data_c2, "abcde") == 0);
+
+				ok(
+					data_match_expected,
+					"Prepared statement SELECT result *SHOULD* match expected -"
+					" Exp=(id:1, c1:100, c2:'abcde'), Act=(id:%d, c1:%ld, c2:'%s')",
+					data_id,
+					data_c1,
+					data_c2
+				);
+			} else {
+				bool data_match_expected =
+					(data_id == static_cast<int64_t>(0)) &&
+					(data_c1 == static_cast<int64_t>(0)) &&
+					(strcmp(data_c2, "") == 0);
+
+				ok(
+					data_match_expected,
+					"Prepared statement SELECT result *SHOULD* match expected -"
+					" Exp=(id:0, c1:0, c2:''), Act=(id:%d, c1:%ld, c2:'%s')",
+					data_id,
+					data_c1,
+					data_c2
+				);
+			}
+		}
+	}
+
+exit:
+	if (stmt_param) { mysql_stmt_close(stmt_param); }
+	mysql_close(proxysql_mysql);
+	mysql_close(proxysql_admin);
+
+	return exit_status();
+}