Commit 88843709 authored by unknown's avatar unknown
Browse files

A fix for Bug#19022 "Memory bug when switching db during trigger execution". 

No test case as the bug is in an existing test case (rpl_trigger.test
when it is run under valgrind).
The warning was caused by memory corruption in replication slave: thd->db
was pointing at a stack address that was previously used by 
sp_head::execute()::old_db. This happened because mysql_change_db
behaved differently in replication slave and did not make a copy of the 
argument to assign to thd->db. 
The solution is to always free the old value of thd->db and allocate a new
copy, regardless whether we're running in a replication slave or not.


sql/log_event.cc:
  Move rewrite_db to log_event.cc, the only place where it is used.
sql/slave.cc:
  Move rewrite_db to log_event.cc
sql/slave.h:
  Remove an unneeded declaration.
sql/sql_class.h:
  Fix set_db to always free the old db, even if the argument is NULL.
  Add a comment.
sql/sql_db.cc:
  Always make a deep copy of the argument in mysql_change_db, even 
  if running in a replication slave. This is necessary because 
  sp_use_new_db (stored procedures) assumes that mysql_change_db always makes
  a deep copy of the argument, and thus passes a pointer to stack into it.
  This assumption was true for all cases except the replication slave thread.
parent d127fa3b

sql/log_event.cc

+25 −7
Original line number Diff line number Diff line
@@ -1635,14 +1635,33 @@ void Query_log_event::print(FILE* file, PRINT_EVENT_INFO* print_event_info) 
*/



#if defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT)



static const char *rewrite_db(const char *db)

{

  if (replicate_rewrite_db.is_empty() || db == NULL)

    return db;

  I_List_iterator<i_string_pair> it(replicate_rewrite_db);

  i_string_pair* tmp;



  while ((tmp=it++))

  {

    if (strcmp(tmp->key, db) == 0)

      return tmp->val;

  }

  return db;

}





int Query_log_event::exec_event(struct st_relay_log_info* rli)

{

  return exec_event(rli, query, q_len);

}





int Query_log_event::exec_event(struct st_relay_log_info* rli, const char *query_arg, uint32 q_len_arg)

int Query_log_event::exec_event(struct st_relay_log_info* rli,

                                const char *query_arg, uint32 q_len_arg)

{

  const char *new_db= rewrite_db(db);

  int expected_error,actual_error= 0;

  /*

    Colleagues: please never free(thd->catalog) in MySQL. This would lead to
@@ -1651,8 +1670,7 @@ int Query_log_event::exec_event(struct st_relay_log_info* rli, const char *query 
    Thank you.

  */

  thd->catalog= catalog_len ? (char *) catalog : (char *)"";

  thd->db_length= db_len;

  thd->db= (char*) rewrite_db(db, &thd->db_length);

  thd->set_db(new_db, strlen(new_db));          /* allocates a copy of 'db' */

  thd->variables.auto_increment_increment= auto_increment_increment;

  thd->variables.auto_increment_offset=    auto_increment_offset;
@@ -1869,7 +1887,7 @@ Default database: '%s'. Query: '%s'", 
    TABLE uses the db.table syntax.

  */

  thd->catalog= 0;

  thd->reset_db(NULL, 0);               // prevent db from being freed

  thd->set_db(NULL, 0);                 /* will free the current database */

  thd->query= 0;			// just to be sure

  thd->query_length= 0;

  VOID(pthread_mutex_unlock(&LOCK_thread_count));
@@ -2817,8 +2835,8 @@ void Load_log_event::set_fields(const char* affected_db, 
int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli, 

			       bool use_rli_only_for_errors)

{

  thd->db_length= db_len;

  thd->db= (char*) rewrite_db(db, &thd->db_length);

  const char *new_db= rewrite_db(db);

  thd->set_db(new_db, strlen(new_db));

  DBUG_ASSERT(thd->query == 0);

  thd->query_length= 0;                         // Should not be needed

  thd->query_error= 0;
@@ -3018,7 +3036,7 @@ int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli, 
  const char *remember_db= thd->db;

  VOID(pthread_mutex_lock(&LOCK_thread_count));

  thd->catalog= 0;

  thd->reset_db(NULL, 0);

  thd->set_db(NULL, 0);                   /* will free the current database */

  thd->query= 0;

  thd->query_length= 0;

  VOID(pthread_mutex_unlock(&LOCK_thread_count));

sql/slave.cc

+0 −18
Original line number Diff line number Diff line
@@ -1177,24 +1177,6 @@ bool net_request_file(NET* net, const char* fname) 
}





const char *rewrite_db(const char* db, uint *new_len)

{

  if (replicate_rewrite_db.is_empty() || !db)

    return db;

  I_List_iterator<i_string_pair> it(replicate_rewrite_db);

  i_string_pair* tmp;



  while ((tmp=it++))

  {

    if (!strcmp(tmp->key, db))

    {

      *new_len= (uint32)strlen(tmp->val);

      return tmp->val;

    }

  }

  return db;

}



/*

  From other comments and tests in code, it looks like

  sometimes Query_log_event and Load_log_event can have db == 0

sql/slave.h

+0 −1
Original line number Diff line number Diff line
@@ -550,7 +550,6 @@ int add_table_rule(HASH* h, const char* table_spec); 
int add_wild_table_rule(DYNAMIC_ARRAY* a, const char* table_spec);

void init_table_rule_hash(HASH* h, bool* h_inited);

void init_table_rule_array(DYNAMIC_ARRAY* a, bool* a_inited);

const char *rewrite_db(const char* db, uint *new_db_len);

const char *print_slave_db_safe(const char *db);

int check_expected_error(THD* thd, RELAY_LOG_INFO* rli, int error_code);

void skip_load_data_infile(NET* net);

sql/sql_class.h

+13 −11
Original line number Diff line number Diff line
@@ -1574,21 +1574,23 @@ class THD :public Statement, 


  /*

    Initialize the current database from a NULL-terminated string with length

    If we run out of memory, we free the current database and return TRUE.

    This way the user will notice the error as there will be no current

    database selected (in addition to the error message set by malloc).

  */

  void set_db(const char *new_db, uint new_db_len)

  {

    if (new_db)

  bool set_db(const char *new_db, uint new_db_len)

  {

    /* Do not reallocate memory if current chunk is big enough. */

      if (db && db_length >= new_db_len)

    if (db && new_db && db_length >= new_db_len)

      memcpy(db, new_db, new_db_len+1);

    else

    {

        safeFree(db);

        db= my_strdup_with_length(new_db, new_db_len, MYF(MY_WME));

      x_free(db);

      db= new_db ? my_strdup_with_length(new_db, new_db_len, MYF(MY_WME)) :

                   NULL;

    }

    db_length= db ? new_db_len : 0;

    }

    return new_db && !db;

  }

  void reset_db(char *new_db, uint new_db_len)

  {

sql/sql_db.cc

+74 −85
Original line number Diff line number Diff line
@@ -781,32 +781,13 @@ bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent) 
exit:

  (void)sp_drop_db_routines(thd, db); /* QQ Ignore errors for now  */

  /*

    If this database was the client's selected database, we silently change the

    client's selected database to nothing (to have an empty SELECT DATABASE()

    in the future). For this we free() thd->db and set it to 0. But we don't do

    free() for the slave thread. Indeed, doing a x_free() on it leads to nasty

    problems (i.e. long painful debugging) because in this thread, thd->db is

    the same as data_buf and db of the Query_log_event which is dropping the

    database. So if you free() thd->db, you're freeing data_buf. You set

    thd->db to 0 but not data_buf (thd->db and data_buf are two distinct

    pointers which point to the same place). Then in ~Query_log_event(), we

    have 'if (data_buf) free(data_buf)' data_buf is !=0 so this makes a

    DOUBLE free().

    Side effects of this double free() are, randomly (depends on the machine),

    when the slave is replicating a DROP DATABASE:

    - garbage characters in the error message:

    "Error 'Can't drop database 'test2'; database doesn't exist' on query

    'h4zI'"

    - segfault

    - hang in "free(vio)" (yes!) in the I/O or SQL slave threads (so slave

    server hangs at shutdown etc).

    If this database was the client's selected database, we silently

    change the client's selected database to nothing (to have an empty

    SELECT DATABASE() in the future). For this we free() thd->db and set

    it to 0.

  */

  if (thd->db && !strcmp(thd->db, db))

  {

    if (!(thd->slave_thread)) /* a slave thread will free it itself */

      x_free(thd->db);

    thd->reset_db(NULL, 0);

  }

    thd->set_db(NULL, 0);

  VOID(pthread_mutex_unlock(&LOCK_mysql_create_db));

  start_waiting_global_read_lock(thd);

exit2:
@@ -1099,38 +1080,52 @@ static long mysql_rm_arc_files(THD *thd, MY_DIR *dirp, 




/*

  Change default database.

  Change the current database.



  SYNOPSIS

    mysql_change_db()

    thd			Thread handler

    name		Databasename

    no_access_check	True don't do access check. In this case name may be ""

    thd			thread handle

    name		database name

    no_access_check	if TRUE, don't do access check. In this

                        case name may be ""



  DESCRIPTION

    Becasue the database name may have been given directly from the

    communication packet (in case of 'connect' or 'COM_INIT_DB')

    we have to do end space removal in this function.

    Check that the database name corresponds to a valid and

    existent database, check access rights (unless called with

    no_access_check), and set the current database. This function

    is called to change the current database upon user request

    (COM_CHANGE_DB command) or temporarily, to execute a stored

    routine.



  NOTES

    Do as little as possible in this function, as it is not called for the

    replication slave SQL thread (for that thread, setting of thd->db is done

    in ::exec_event() methods of log_event.cc).



    This function does not send anything, including error messages to the

    client, if that should be sent to the client, call net_send_error after

    this function.

    This function is not the only way to switch the database that

    is currently employed. When the replication slave thread

    switches the database before executing a query, it calls

    thd->set_db directly. However, if the query, in turn, uses

    a stored routine, the stored routine will use this function,

    even if it's run on the slave.



    This function allocates the name of the database on the system

    heap: this is necessary to be able to uniformly change the

    database from any module of the server. Up to 5.0 different

    modules were using different memory to store the name of the

    database, and this led to memory corruption: a stack pointer

    set by Stored Procedures was used by replication after the

    stack address was long gone.



    This function does not send anything, including error

    messages, to the client. If that should be sent to the client,

    call net_send_error after this function.



  RETURN VALUES

    0	ok

    0	OK

    1	error

*/



bool mysql_change_db(THD *thd, const char *name, bool no_access_check)

{

  int length, db_length;

  char *dbname= thd->slave_thread ? (char *) name :

                                    my_strdup((char *) name, MYF(MY_WME));

  int path_length, db_length;

  char *db_name;

  char	path[FN_REFLEN];

  HA_CREATE_INFO create;

  bool system_db= 0;
@@ -1142,32 +1137,35 @@ bool mysql_change_db(THD *thd, const char *name, bool no_access_check) 
  DBUG_ENTER("mysql_change_db");

  DBUG_PRINT("enter",("name: '%s'",name));



  LINT_INIT(db_length);



  /* dbname can only be NULL if malloc failed */

  if (!dbname || !(db_length= strlen(dbname)))

  if (name == NULL || name[0] == '\0' && no_access_check == FALSE)

  {

    if (no_access_check && dbname)

    my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));

    DBUG_RETURN(1);				/* purecov: inspected */

  }

  else if (name[0] == '\0')

  {

      /* Called from SP when orignal database was not set */

    /* Called from SP to restore the original database, which was NULL */

    DBUG_ASSERT(no_access_check);

    system_db= 1;

    db_name= NULL;

    db_length= 0;

    goto end;

  }

    if (!(thd->slave_thread))

      x_free(dbname);				/* purecov: inspected */

    my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR),

               MYF(0));                         /* purecov: inspected */

    DBUG_RETURN(1);				/* purecov: inspected */

  }

  if (check_db_name(dbname))

  /*

    Now we need to make a copy because check_db_name requires a

    non-constant argument. TODO: fix check_db_name.

  */

  if ((db_name= my_strdup(name, MYF(MY_WME))) == NULL)

    DBUG_RETURN(1);                             /* the error is set */

  db_length= strlen(db_name);

  if (check_db_name(db_name))

  {

    my_error(ER_WRONG_DB_NAME, MYF(0), dbname);

    if (!(thd->slave_thread))

      my_free(dbname, MYF(0));

    my_error(ER_WRONG_DB_NAME, MYF(0), db_name);

    my_free(db_name, MYF(0));

    DBUG_RETURN(1);

  }

  DBUG_PRINT("info",("Use database: %s", dbname));

  if (!my_strcasecmp(system_charset_info, dbname, information_schema_name.str))

  DBUG_PRINT("info",("Use database: %s", db_name));

  if (!my_strcasecmp(system_charset_info, db_name, information_schema_name.str))

  {

    system_db= 1;

#ifndef NO_EMBEDDED_ACCESS_CHECKS
@@ -1182,45 +1180,36 @@ bool mysql_change_db(THD *thd, const char *name, bool no_access_check) 
    if (test_all_bits(sctx->master_access, DB_ACLS))

      db_access=DB_ACLS;

    else

      db_access= (acl_get(sctx->host, sctx->ip, sctx->priv_user, dbname, 0) |

      db_access= (acl_get(sctx->host, sctx->ip, sctx->priv_user, db_name, 0) |

                  sctx->master_access);

    if (!(db_access & DB_ACLS) && (!grant_option ||

                                   check_grant_db(thd,dbname)))

                                   check_grant_db(thd,db_name)))

    {

      my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),

               sctx->priv_user,

               sctx->priv_host,

               dbname);

               db_name);

      mysql_log.write(thd, COM_INIT_DB, ER(ER_DBACCESS_DENIED_ERROR),

                      sctx->priv_user, sctx->priv_host, dbname);

      if (!(thd->slave_thread))

        my_free(dbname,MYF(0));

                      sctx->priv_user, sctx->priv_host, db_name);

      my_free(db_name, MYF(0));

      DBUG_RETURN(1);

    }

  }

#endif

  (void) sprintf(path,"%s/%s",mysql_data_home,dbname);

  length=unpack_dirname(path,path);		// Convert if not unix

  if (length && path[length-1] == FN_LIBCHAR)

    path[length-1]=0;				// remove ending '\'

  (void) sprintf(path,"%s/%s", mysql_data_home, db_name);

  path_length= unpack_dirname(path, path);      // Convert if not UNIX

  if (path_length && path[path_length-1] == FN_LIBCHAR)

    path[path_length-1]= '\0';                  // remove ending '\'

  if (my_access(path,F_OK))

  {

    my_error(ER_BAD_DB_ERROR, MYF(0), dbname);

    if (!(thd->slave_thread))

      my_free(dbname,MYF(0));

    my_error(ER_BAD_DB_ERROR, MYF(0), db_name);

    my_free(db_name, MYF(0));

    DBUG_RETURN(1);

  }

end:

  if (!(thd->slave_thread))

  x_free(thd->db);

  if (dbname && dbname[0] == 0)

  {

    if (!(thd->slave_thread))

      my_free(dbname, MYF(0));

    thd->reset_db(NULL, 0);

  }

  else

    thd->reset_db(dbname, db_length);          // THD::~THD will free this

  DBUG_ASSERT(db_name == NULL || db_name[0] != '\0');

  thd->reset_db(db_name, db_length);            // THD::~THD will free this

#ifndef NO_EMBEDDED_ACCESS_CHECKS

  if (!no_access_check)

    sctx->db_access= db_access;