BUG#17512527: LIST HANDLING INCORRECT IN MYSQL_PRUNE_STMT_LIST()

Nisha Gopalakrishnan · Nisha Gopalakrishnan · commit 4797ea0b772d · 2014-07-21T21:21:15.000+05:30
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-&gt;stmts' to a temporary 'pruned_list'.
When moving the first 'INIT_DONE' element to the pruned_list,
'element-&gt;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.
diff --git a/sql-common/client.c b/sql-common/client.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -3985,12 +3985,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
@@ -1,4 +1,4 @@
-/* Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2002, 2014, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -19353,6 +19353,51 @@ static void test_bug17309863()
 }
 #endif
 
+
+/**
+   BUG#17512527: LIST HANDLING INCORRECT IN MYSQL_PRUNE_STMT_LIST()
+*/
+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 },
   { "test_view_sp_list_fields", test_view_sp_list_fields },
@@ -19627,6 +19672,7 @@ static struct my_tests_st my_tests[]= {
 #ifndef EMBEDDED_LIBRARY
   { "test_bug17309863", test_bug17309863},
 #endif
+  { "test_bug17512527", test_bug17512527},
   { 0, 0 }
 };
 
