Merge lp:~gl-az/percona-server/BT-23597-bug1166638-5.6 into lp:percona-server/5.6

[George Ormond Lorch III]

Merged from 5.5:
Fix for https://bugs.launchpad.net/percona-server/+bug/1166638 - Utility user setup leaks memory | Valgrind: still reachable: malloc in acl_init_utility_user & init_dynamic_array2

Originally this was just written off as a known, one time leak, which is is, but the failure causes scripted testing to fail and is really just sloppy.

To fix, created new status flags that indicate whather or not acl_utility_user and acl_utility_user_schema_access have been initialized. Create new function to clean up utility user resources and call it from acl_free and from acl_init_utility_user to free up if being reinitialized or if there is an error during the initialization. For 5.6 had to move some things around due to the struct++'ing that upstream devs did (turning structs into classes without properly building out rational/smart class interfaces for data access and management).

http://jenkins.percona.com/view/PS%205.6/job/percona-server-5.6-param/319/

[George Ormond Lorch III (gl-az)]

http://jenkins.percona.com/view/PS%205.6/job/percona-server-5.6-param/319/

[Laurynas Biveinis (laurynas-biveinis)]

- Same comment as in 5.5 re. acl_free_utility_user() header comment.
- Any reason 5.6 acl_free_utility_user() does not bzero acl_utility_user like 5.5 does?

review: Needs Information

[George Ormond Lorch III (gl-az)]

On 9/29/2013 2:04 AM, Laurynas Biveinis wrote:
> Review: Needs Information
>
> - Same comment as in 5.5 re. acl_free_utility_user() header comment.
> - Any reason 5.6 acl_free_utility_user() does not bzero acl_utility_user like 5.5 does?
>
in 5.6 ACL_USER is a class with private members, 5.5 it is just a
struct...the 5.6 struct++ization of the codebase isn't very friendly to
bzeroing of things :)

--
George O. Lorch III
Software Engineer, Percona
[phone] x542 US/Arizona (GMT -7)
skype: george.ormond.lorch.iii

[Laurynas Biveinis (laurynas-biveinis)]

I think that ACL_USER satisfies C++ POD requirements and is thus safe to memset/memcpy, but still OK to leave this out if you prefer.

[George Ormond Lorch III (gl-az)]

Fixed acl_free_utility_user comment and went ahead and added back on memset (in place of 5.5 bzero).

[Laurynas Biveinis (laurynas-biveinis)]

review: Approve

=== modified file 'Percona-Server/sql/sql_acl.cc'
--- Percona-Server/sql/sql_acl.cc	2013-08-27 20:55:20 +0000
+++ Percona-Server/sql/sql_acl.cc	2013-09-30 17:15:58 +0000
@@ -891,8 +891,11 @@
 };
 
 static ACL_USER acl_utility_user;
+LEX_STRING acl_utility_user_name, acl_utility_user_host_name;
 static DYNAMIC_ARRAY acl_utility_user_schema_access;
+static bool acl_utility_user_initialized= false;
 static my_bool acl_init_utility_user(my_bool check_no_resolve);
+static void acl_free_utility_user();
 
 /**
   Convert scrambled password to binary form, according to scramble type, 
@@ -1462,6 +1465,7 @@
 
 void acl_free(bool end)
 {
+  acl_free_utility_user();
   free_root(&global_acl_memory,MYF(0));
   delete_dynamic(&acl_users);
   delete_dynamic(&acl_dbs);
@@ -1652,7 +1656,6 @@
 my_bool
 acl_init_utility_user(my_bool check_no_resolve)
 {
-  LEX_STRING acl_user_name, acl_host_name;
   char password[CRYPT_MAX_PASSWORD_SIZE + 1];
   uint i, passlen;
   my_bool ret= TRUE;
@@ -1660,15 +1663,23 @@
   if (!utility_user)
     goto end;
 
+  acl_free_utility_user();
+
+  /* Allocate all initial resources necessary */
+  acl_utility_user_name.str= (char *) my_malloc(USERNAME_LENGTH+1, MY_ZEROFILL);
+  acl_utility_user_host_name.str= (char *) my_malloc(HOSTNAME_LENGTH+1, MY_ZEROFILL);
+  (void) my_init_dynamic_array(&acl_utility_user_schema_access, sizeof(char *),
+                               5, 10);
+  acl_utility_user_initialized= true;
+
   /* parse out the option to its component user and host name parts */
-  acl_user_name.str= (char *) my_malloc(USERNAME_LENGTH+1, MY_ZEROFILL);
-  acl_host_name.str= (char *) my_malloc(HOSTNAME_LENGTH+1, MY_ZEROFILL);
   parse_user(utility_user, strlen(utility_user),
-             acl_user_name.str, &acl_user_name.length,
-             acl_host_name.str, &acl_host_name.length);
+             acl_utility_user_name.str, &acl_utility_user_name.length,
+             acl_utility_user_host_name.str,
+	     &acl_utility_user_host_name.length);
 
   /* Check to see if the username is anonymous */
-  if (!acl_user_name.str || acl_user_name.str[0] == '\0')
+  if (!acl_utility_user_name.str || acl_utility_user_name.str[0] == '\0')
   {
     sql_print_error("'utility user' specified as '%s' is anonymous"
                     " and not allowed.",
@@ -1688,9 +1699,9 @@
   }
 
   /* set up some of the static utility user struct fields */
-  acl_utility_user.user= acl_user_name.str;
+  acl_utility_user.user= acl_utility_user_name.str;
 
-  acl_utility_user.host.update_hostname(acl_host_name.str);
+  acl_utility_user.host.update_hostname(acl_utility_user_host_name.str);
 
   acl_utility_user.sort= get_sort(2, acl_utility_user.host.get_host(),
                                    acl_utility_user.user);
@@ -1700,7 +1711,7 @@
   {
     ACL_USER *user= dynamic_element(&acl_users, i, ACL_USER*);
     if (user->user
-        && strcmp(acl_user_name.str, user->user) == 0)
+        && strcmp(acl_utility_user_name.str, user->user) == 0)
     {
       if (user->sort == acl_utility_user.sort)
       {
@@ -1774,9 +1785,6 @@
   (void) push_dynamic(&acl_users,(uchar*) &acl_utility_user);
         
   /* initialize the schema access list if specified */
-  (void) my_init_dynamic_array(&acl_utility_user_schema_access, sizeof(char *),
-                               5, 10);
-
   if (utility_user_schema_access)
   {
     char *cur_pos= utility_user_schema_access;
@@ -1810,15 +1818,38 @@
   goto end;
 
 cleanup:
-  my_free(acl_user_name.str);
-  my_free(acl_host_name.str);
-  memset(&acl_utility_user, 0, sizeof(acl_utility_user));
+  acl_free_utility_user();
 
 end:
   return ret;
 }
 
 /*
+  Free up any resources allocated during acl_init_utility_user.
+*/
+static
+void
+acl_free_utility_user()
+{
+  if (acl_utility_user_initialized)
+  {
+    uint i;
+    for (i=0 ; i < acl_utility_user_schema_access.elements ; i++)
+    {
+      char** dbname= dynamic_element(&acl_utility_user_schema_access, i, char**);
+      if (*dbname)
+        my_free(*dbname);
+    }
+    delete_dynamic(&acl_utility_user_schema_access);
+
+    my_free(acl_utility_user_name.str);
+    my_free(acl_utility_user_host_name.str);
+    memset(&acl_utility_user, 0, sizeof(acl_utility_user));
+    acl_utility_user_initialized= false;
+  }
+}
+
+/*
   Determines if the user specified by user, host, ip matches the utility user
 */
 my_bool