[patch] Patch back ported to MySQL 5.5 branch

fix_use_after_free_in_mysql_prune_stmt_list.patch (text/plain), 4.52 KB, created by Thomas Deutschmann (RETIRED) on 2017-03-01 19:56:11 UTC

From 1037977895aa4a145de16719df0a2375c71bbf26 Mon Sep 17 00:00:00 2001
From: Nisha Gopalakrishnan <[e-mail]>
Date: Mon, 21 Jul 2014 21:21:15 +0530
Subject: [PATCH] BUG#17512527: LIST HANDLING INCORRECT IN
 MYSQL_PRUNE_STMT_LIST()

Analysis:
---------
Invalid memory access maybe observed when using prepared statements if:
a) The mysql client connection is lost after statement preparation
   is complete and
b) There is at least one statement which is in initialized state but
   not prepared yet.

When the client detects a closed connection, it calls end_server()
to shutdown the connection. As part of the clean up, the
mysql_prune_stmt_list() removes the statements which has transitioned
beyond the initialized state and retains only the statements which
are in a initialized state. During this processing, the initialized
statements are moved from 'mysql->stmts' to a temporary 'pruned_list'.
When moving the first 'INIT_DONE' element to the pruned_list,
'element->next' is set to NULL. Hence the rest of the list is never
traversed and the statements which have transitioned beyond the
initialized state are never invalidated.

When the mysql_stmt_close() is called for the statement which is not
invalidated; the statements list is updated in order to remove the
statement. This would end up accessing freed memory(freed by the
mysql_stmt_close() for a previous statement in the list).

Fix:
---
mysql_prune_stmt_list() called list_add() incorrectly to create a
temporary list. The use case of list_add() is to add a single
element to the front of the doubly linked list.
mysql_prune_stmt_list() called list_add() by passing an entire
list as the 'element'.

mysql_prune_stmt_list() now uses list_delete() to remove the
statement which has transitioned beyond the initialized phase.
Thus the statement list would contain only elements where the
the state of the statement is initialized.

Note: Run the test with valgrind-mysqltest and leak-check=full
option to see the invalid memory access.

Back-ported to MySQL 5.5 branch by Balint Reczey

Conflicts:
	sql-common/client.c
	tests/mysql_client_test.c
---
 sql-common/client.c       | 11 +++++++----
 tests/mysql_client_test.c | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/sql-common/client.c b/sql-common/client.c
index cd9b6a7..be60cc1 100644
--- a/sql-common/client.c
+++ b/sql-common/client.c
@@ -3790,12 +3790,15 @@ static void mysql_close_free(MYSQL *mysql)
 */
 static void mysql_prune_stmt_list(MYSQL *mysql)
 {
-  LIST *element= mysql->stmts;
-  LIST *pruned_list= 0;
+  LIST *pruned_list= NULL;
 
-  for (; element; element= element->next)
+  while(mysql->stmts)
   {
-    MYSQL_STMT *stmt= (MYSQL_STMT *) element->data;
+    LIST *element= mysql->stmts;
+    MYSQL_STMT *stmt;
+
+    mysql->stmts= list_delete(element, element);
+    stmt= (MYSQL_STMT *) element->data;
     if (stmt->state != MYSQL_STMT_INIT_DONE)
     {
       stmt->mysql= 0;
diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c
index e600d82..d3f3899 100644
--- a/tests/mysql_client_test.c
+++ b/tests/mysql_client_test.c
@@ -18648,6 +18648,46 @@ static void test_bug13001491()
   myquery(rc);
 }
 
+static void test_bug17512527()
+{
+  MYSQL *conn1, *conn2;
+  MYSQL_STMT *stmt1, *stmt2;
+  const char *stmt1_txt= "SELECT NOW();";
+  const char *stmt2_txt= "SELECT 1;";
+  unsigned long thread_id;
+  char query[MAX_TEST_QUERY_LENGTH];
+  int rc;
+
+  conn1= client_connect(0, MYSQL_PROTOCOL_DEFAULT, 1);
+  conn2= client_connect(0, MYSQL_PROTOCOL_DEFAULT, 0);
+
+  stmt1 = mysql_stmt_init(conn1);
+  check_stmt(stmt1);
+  rc= mysql_stmt_prepare(stmt1, stmt1_txt, strlen(stmt1_txt));
+  check_execute(stmt1, rc);
+
+  thread_id= mysql_thread_id(conn1);
+  sprintf(query, "KILL %lu", thread_id);
+  if (thread_query(query))
+    exit(1);
+
+  /*
+    After the connection is killed, the connection is
+    re-established due to the reconnect flag.
+  */
+  stmt2 = mysql_stmt_init(conn1);
+  check_stmt(stmt2);
+
+  rc= mysql_stmt_prepare(stmt2, stmt2_txt, strlen(stmt2_txt));
+  check_execute(stmt1, rc);
+
+  mysql_stmt_close(stmt2);
+  mysql_stmt_close(stmt1);
+
+  mysql_close(conn1);
+  mysql_close(conn2);
+}
+
 
 static struct my_tests_st my_tests[]= {
   { "disable_query_logs", disable_query_logs },
@@ -18911,6 +18951,7 @@ static struct my_tests_st my_tests[]= {
   { "test_bug12337762", test_bug12337762 },
   { "test_bug11754979", test_bug11754979 },
   { "test_bug13001491", test_bug13001491 },
+  { "test_bug17512527", test_bug17512527},
   { 0, 0 }
 };
 
-- 
2.1.4