bk commit into 5.1 tree (davi:1.2607) BUG#31850

[Davi Arnaut]

Below is the list of changes that have just been committed into a local
5.1 repository of davi. When davi does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-11-01 17:29:20-02:00, davi@stripped +2 -0
  Bug#31850 Test crashes in "embedded" server
  
  The mysql_change_user command fails to properly update the database pointer
  when no database is selected, leading to "use after free" errors. The same
  happens on the user privilege pointer in the thread security context.
  
  The solution is to properly reset and update the database name. Also update
  the user_priv pointer so that it doesn't point to freed memory.

  sql/sql_connect.cc@stripped, 2007-11-01 17:29:17-02:00, davi@stripped +9 -14
    After a successful call to check_user() without specifying a new
    database name, the previous database thd->db) is freed but the
    pointer is not updated to NULL.

  sql/sql_parse.cc@stripped, 2007-11-01 17:29:17-02:00, davi@stripped +3 -2
    Update the security_ctx->priv_user pointer as it is a alias for
    the user security_ctx->user pointer. Also remove unneeded cast,
    the x_free macro casts the argument.

diff -Nrup a/sql/sql_connect.cc b/sql/sql_connect.cc
--- a/sql/sql_connect.cc	2007-10-30 15:10:08 -02:00
+++ b/sql/sql_connect.cc	2007-11-01 17:29:17 -02:00
@@ -316,17 +316,20 @@ int check_user(THD *thd, enum enum_serve
 {
   DBUG_ENTER("check_user");
   LEX_STRING db_str= { (char *) db, db ? strlen(db) : 0 };
-  
+
+  /*
+    Clear thd->db as it points to something, that will be freed when
+    connection is closed. We don't want to accidentally free a wrong
+    pointer if connect failed. Also in case of 'CHANGE USER' failure,
+    current database will be switched to 'no database selected'.
+  */
+  thd->reset_db(NULL, 0);
+
 #ifdef NO_EMBEDDED_ACCESS_CHECKS
   thd->main_security_ctx.master_access= GLOBAL_ACLS;       // Full rights
   /* Change database if necessary */
   if (db && db[0])
   {
-    /*
-      thd->db is saved in caller and needs to be freed by caller if this
-      function returns 0
-    */
-    thd->reset_db(NULL, 0);
     if (mysql_change_db(thd, &db_str, FALSE))
     {
       /* Send the error to the client */
@@ -357,14 +360,6 @@ int check_user(THD *thd, enum enum_serve
       passwd_len != SCRAMBLE_LENGTH &&
       passwd_len != SCRAMBLE_LENGTH_323)
     DBUG_RETURN(ER_HANDSHAKE_ERROR);
-
-  /*
-    Clear thd->db as it points to something, that will be freed when 
-    connection is closed. We don't want to accidentally free a wrong pointer
-    if connect failed. Also in case of 'CHANGE USER' failure, current
-    database will be switched to 'no database selected'.
-  */
-  thd->reset_db(NULL, 0);
 
   USER_RESOURCES ur;
   int res= acl_getroot(thd, &ur, passwd, passwd_len);
diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
--- a/sql/sql_parse.cc	2007-10-30 20:51:02 -02:00
+++ b/sql/sql_parse.cc	2007-11-01 17:29:17 -02:00
@@ -911,6 +911,7 @@ bool dispatch_command(enum enum_server_c
 
     /* Clear variables that are allocated */
     thd->user_connect= 0;
+    thd->security_ctx->priv_user= thd->security_ctx->user;
     res= check_user(thd, COM_CHANGE_USER, passwd, passwd_len, db, FALSE);
 
     if (res)
@@ -933,8 +934,8 @@ bool dispatch_command(enum enum_server_c
       if (save_user_connect)
 	decrease_user_connections(save_user_connect);
 #endif /* NO_EMBEDDED_ACCESS_CHECKS */
-      x_free((uchar*) save_db);
-      x_free((uchar*)  save_security_ctx.user);
+      x_free(save_db);
+      x_free(save_security_ctx.user);
 
       if (cs_number)
       {