From c75712caa81a58164019754a3690ddf434d17f61 Mon Sep 17 00:00:00 2001 From: Luis Soares Date: Sun, 6 Dec 2009 23:12:11 +0000 Subject: BUG#49119: Master crashes when executing 'REVOKE ... ON {PROCEDURE|FUNCTION} FROM ...' The master would hit an assertion when binary log was active. This was due to the fact that the thread's diagnostics area was being cleared before writing to the binlog, independently of mysql_routine_grant returning an error or not. When mysql_routine_grant was to return an error, the return value and the diagnostics area contents would mismatch. Consequently, neither my_ok would be called nor an error would be signaled in the diagnostics area, eventually triggering the assertion in net_end_statement. We fix this by not clearing the diagnostics area at binlogging time. --- sql/sql_acl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 77c72066429..3b81a85c368 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -3378,7 +3378,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, if (write_to_binlog) { - write_bin_log(thd, TRUE, thd->query(), thd->query_length()); + write_bin_log(thd, FALSE, thd->query(), thd->query_length()); } rw_unlock(&LOCK_grant); -- cgit v1.2.1 From 06a1df91813ea2c39f7312bcf8af972c7e8a926f Mon Sep 17 00:00:00 2001 From: Davi Arnaut Date: Thu, 17 Dec 2009 15:58:38 -0200 Subject: Bug#48983: Bad strmake calls (length one too long) The problem is a somewhat common misusage of the strmake function. The strmake(dst, src, len) function writes at most /len/ bytes to the string pointed to by src, not including the trailing null byte. Hence, if /len/ is the exact length of the destination buffer, a one byte buffer overflow can occur if the length of the source string is equal to or greater than /len/. client/mysqldump.c: Make room for the trailing null byte. libmysql/libmysql.c: Add comment, there is enough room in the buffer. Increase buffer length, two strings are concatenated. libmysqld/lib_sql.cc: Make room for the trailing null byte. mysys/default.c: Make room for the trailing null bytes. mysys/mf_pack.c: Make room for the trailing null byte. server-tools/instance-manager/commands.cc: Copy only if overflow isn't possible in both cases. server-tools/instance-manager/listener.cc: Make room for the trailing null byte. sql/log.cc: Make room for the trailing null byte. sql/sp_pcontext.h: Cosmetic fix. sql/sql_acl.cc: MAX_HOSTNAME already specifies space for the trailing null byte. sql/sql_parse.cc: Make room for the trailing null byte. sql/sql_table.cc: Make room for the trailing null byte. --- sql/sql_acl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index f29baad9a84..bf117874552 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -914,7 +914,7 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, *mqh= acl_user->user_resource; if (acl_user->host.hostname) - strmake(sctx->priv_host, acl_user->host.hostname, MAX_HOSTNAME); + strmake(sctx->priv_host, acl_user->host.hostname, MAX_HOSTNAME - 1); else *sctx->priv_host= 0; } @@ -1015,7 +1015,7 @@ bool acl_getroot_no_password(Security_context *sctx, char *user, char *host, sctx->priv_user= acl_user->user ? user : (char *) ""; if (acl_user->host.hostname) - strmake(sctx->priv_host, acl_user->host.hostname, MAX_HOSTNAME); + strmake(sctx->priv_host, acl_user->host.hostname, MAX_HOSTNAME - 1); else *sctx->priv_host= 0; } -- cgit v1.2.1 From f815246486444ca2dc5d408099439e1d973d3a7b Mon Sep 17 00:00:00 2001 From: Magne Mahre Date: Fri, 18 Dec 2009 11:48:34 +0100 Subject: Backport to 5.1 branch (next-mr revid: 2921) Bug#35589 SET PASSWORD caused a crash Bug#35591 FLUSH PRIVILEGES caused a crash A race condition on the privilege hash tables (proc_priv_hash and func_priv_hash) caused one thread to try to delete elements that had already been deleted by another thread. The bug was caused by reading and saving the pointers to the hash tables outside mutex protection. This led to an inconsistency where a thread copied a pointer to a hash, another thread did the same, the first thread then deleted the hash, and the second then crashed when it in turn tried to delete the deleted hash. The fix is to ensure that operations on the shared hash structures happens under mutex protection (moving the locking up a little) --- sql/sql_acl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 3b81a85c368..b30c012e633 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -3765,11 +3765,11 @@ static my_bool grant_reload_procs_priv(THD *thd) DBUG_RETURN(TRUE); } + rw_wrlock(&LOCK_grant); /* Save a copy of the current hash if we need to undo the grant load */ old_proc_priv_hash= proc_priv_hash; old_func_priv_hash= func_priv_hash; - rw_wrlock(&LOCK_grant); if ((return_val= grant_load_procs_priv(table.table))) { /* Error; Reverting to old hash */ -- cgit v1.2.1 From b3dd4d9486ca00e555823f81185fe9db83a44db5 Mon Sep 17 00:00:00 2001 From: Kristofer Pettersson Date: Wed, 13 Jan 2010 12:39:00 +0100 Subject: Bug#33982 debug assertion and crash reloading grant tables after sighup or kill In certain rare cases when a process was interrupted during a FLUSH PRIVILEGES operation the diagnostic area would be set to an error state but the function responsible for the operation would still signal success. This would lead to a debug assertion error later on when the server would attempt to reset the DA before sending the error message. This patch fixes the issue by assuring that reload_acl_and_cache() always fails if an error condition is raised. The second issue was that a KILL could cause a console error message which referred to a DA state without first making sure that such a state existed. This patch fixes this issue in two different palces by first checking DA state before fetching the error message. sql/sql_acl.cc: * Make sure that there is an error to print before attempting to do so. * Minor style change: change 1 to TRUE for clarity. sql/sql_parse.cc: * Always fail reload_acl_and_cache() if the query was killed. sql/sql_servers.cc: * Make sure that there is an error to print before attempting to do so. --- sql/sql_acl.cc | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 7d8f8ea71b3..7ba1a657578 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -310,7 +310,7 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables) { TABLE *table; READ_RECORD read_record_info; - my_bool return_val= 1; + my_bool return_val= TRUE; bool check_no_resolve= specialflag & SPECIAL_NO_RESOLVE; char tmp_name[NAME_LEN+1]; int password_length; @@ -623,7 +623,7 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables) init_check_host(); initialized=1; - return_val=0; + return_val= FALSE; end: thd->variables.sql_mode= old_sql_mode; @@ -674,7 +674,7 @@ my_bool acl_reload(THD *thd) DYNAMIC_ARRAY old_acl_hosts,old_acl_users,old_acl_dbs; MEM_ROOT old_mem; bool old_initialized; - my_bool return_val= 1; + my_bool return_val= TRUE; DBUG_ENTER("acl_reload"); if (thd->locked_tables) @@ -701,8 +701,13 @@ my_bool acl_reload(THD *thd) if (simple_open_n_lock_tables(thd, tables)) { - sql_print_error("Fatal error: Can't open and lock privilege tables: %s", - thd->main_da.message()); + /* + Execution might have been interrupted; only print the error message + if an error condition has been raised. + */ + if (thd->main_da.is_error()) + sql_print_error("Fatal error: Can't open and lock privilege tables: %s", + thd->main_da.message()); goto end; } -- cgit v1.2.1 From 3cae7d1187795a8089b6f54ac60cf2a0e579cf89 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 22 Jan 2010 17:38:21 +0800 Subject: Bug #49132 Replication failure on temporary table + DDL In RBR, DDL statement will change binlog format to non row-based format before it is binlogged, but the binlog format was not be restored, and then manipulating a temporary table can not reset binlog format to row-based format rightly. So that the manipulated statement is binlogged with statement-based format. To fix the problem, restore the state of binlog format after the DDL statement is binlogged. mysql-test/extra/rpl_tests/rpl_tmp_table_and_DDL.test: Added the test file to verify if executing DDL statement before trying to manipulate a temporary table causes row-based replication to break with error 'table does not exist'. mysql-test/suite/binlog/r/binlog_row_mix_innodb_myisam.result: Correct the test result, all the above binlog event should be row-based after the bug49132 is fixed IN RBR. mysql-test/suite/ndb/r/ndb_tmp_table_and_DDL.result: Test result for bug#49132 base on ndb engine. mysql-test/suite/ndb/t/ndb_tmp_table_and_DDL.test: Added the test file to verify if executing DDL statement before trying to manipulate a temporary table causes row-based replication to break with error 'table does not exist' base on ndb engine. mysql-test/suite/rpl/r/rpl_tmp_table_and_DDL.result: Test result for bug#49132 base on myisam engine. mysql-test/suite/rpl/t/rpl_tmp_table_and_DDL.test: Added the test file to verify if executing DDL statement before trying to manipulate a temporary table causes row-based replication to break with error 'table does not exist' base on myisam engine. sql/event_db_repository.cc: Added code to restore the state of binlog format after the DDL statement is binlogged. sql/events.cc: Added code to restore the state of binlog format after the DDL statement is binlogged. sql/sp.cc: Added code to restore the state of binlog format after the DDL statement is binlogged. sql/sql_acl.cc: Added code to restore the state of binlog format after the DDL statement is binlogged. sql/sql_udf.cc: Added code to restore the state of binlog format after the DDL statement is binlogged. --- sql/sql_acl.cc | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 7ba1a657578..e2c47957e4d 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2978,6 +2978,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, TABLE_LIST tables[3]; bool create_new_users=0; char *db_name, *table_name; + bool save_binlog_row_based; DBUG_ENTER("mysql_table_grant"); if (!initialized) @@ -3073,6 +3074,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, row-based replication. The flag will be reset at the end of the statement. */ + save_binlog_row_based= thd->current_stmt_binlog_row_based; thd->clear_current_stmt_binlog_row_based(); #ifdef HAVE_REPLICATION @@ -3088,7 +3090,11 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, */ tables[0].updating= tables[1].updating= tables[2].updating= 1; if (!(thd->spcont || rpl_filter->tables_ok(0, tables))) + { + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(FALSE); + } } #endif @@ -3101,6 +3107,8 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, if (simple_open_n_lock_tables(thd,tables)) { // Should never happen close_thread_tables(thd); /* purecov: deadcode */ + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(TRUE); /* purecov: deadcode */ } @@ -3227,6 +3235,8 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, /* Tables are automatically closed */ thd->lex->restore_backup_query_tables_list(&backup); + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result); } @@ -3255,6 +3265,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, TABLE_LIST tables[2]; bool create_new_users=0, result=0; char *db_name, *table_name; + bool save_binlog_row_based; DBUG_ENTER("mysql_routine_grant"); if (!initialized) @@ -3290,6 +3301,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, row-based replication. The flag will be reset at the end of the statement. */ + save_binlog_row_based= thd->current_stmt_binlog_row_based; thd->clear_current_stmt_binlog_row_based(); #ifdef HAVE_REPLICATION @@ -3305,13 +3317,19 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, */ tables[0].updating= tables[1].updating= 1; if (!(thd->spcont || rpl_filter->tables_ok(0, tables))) + { + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(FALSE); + } } #endif if (simple_open_n_lock_tables(thd,tables)) { // Should never happen close_thread_tables(thd); + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(TRUE); } @@ -3387,6 +3405,8 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, } rw_unlock(&LOCK_grant); + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; /* Tables are automatically closed */ DBUG_RETURN(result); @@ -3401,6 +3421,7 @@ bool mysql_grant(THD *thd, const char *db, List &list, char tmp_db[NAME_LEN+1]; bool create_new_users=0; TABLE_LIST tables[2]; + bool save_binlog_row_based; DBUG_ENTER("mysql_grant"); if (!initialized) { @@ -3429,6 +3450,7 @@ bool mysql_grant(THD *thd, const char *db, List &list, row-based replication. The flag will be reset at the end of the statement. */ + save_binlog_row_based= thd->current_stmt_binlog_row_based; thd->clear_current_stmt_binlog_row_based(); #ifdef HAVE_REPLICATION @@ -3444,13 +3466,19 @@ bool mysql_grant(THD *thd, const char *db, List &list, */ tables[0].updating= tables[1].updating= 1; if (!(thd->spcont || rpl_filter->tables_ok(0, tables))) + { + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(FALSE); + } } #endif if (simple_open_n_lock_tables(thd,tables)) { // This should never happen close_thread_tables(thd); /* purecov: deadcode */ + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(TRUE); /* purecov: deadcode */ } @@ -3510,6 +3538,8 @@ bool mysql_grant(THD *thd, const char *db, List &list, if (!result) my_ok(thd); + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result); } @@ -5666,6 +5696,7 @@ bool mysql_create_user(THD *thd, List &list) List_iterator user_list(list); TABLE_LIST tables[GRANT_TABLES]; bool some_users_created= FALSE; + bool save_binlog_row_based; DBUG_ENTER("mysql_create_user"); /* @@ -5673,11 +5704,16 @@ bool mysql_create_user(THD *thd, List &list) row-based replication. The flag will be reset at the end of the statement. */ + save_binlog_row_based= thd->current_stmt_binlog_row_based; thd->clear_current_stmt_binlog_row_based(); /* CREATE USER may be skipped on replication client. */ if ((result= open_grant_tables(thd, tables))) + { + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result != 1); + } rw_wrlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); @@ -5720,6 +5756,8 @@ bool mysql_create_user(THD *thd, List &list) rw_unlock(&LOCK_grant); close_thread_tables(thd); + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result); } @@ -5746,6 +5784,7 @@ bool mysql_drop_user(THD *thd, List &list) TABLE_LIST tables[GRANT_TABLES]; bool some_users_deleted= FALSE; ulong old_sql_mode= thd->variables.sql_mode; + bool save_binlog_row_based; DBUG_ENTER("mysql_drop_user"); /* @@ -5753,11 +5792,16 @@ bool mysql_drop_user(THD *thd, List &list) row-based replication. The flag will be reset at the end of the statement. */ + save_binlog_row_based= thd->current_stmt_binlog_row_based; thd->clear_current_stmt_binlog_row_based(); /* DROP USER may be skipped on replication client. */ if ((result= open_grant_tables(thd, tables))) + { + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result != 1); + } thd->variables.sql_mode&= ~MODE_PAD_CHAR_TO_FULL_LENGTH; @@ -5794,6 +5838,8 @@ bool mysql_drop_user(THD *thd, List &list) rw_unlock(&LOCK_grant); close_thread_tables(thd); thd->variables.sql_mode= old_sql_mode; + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result); } @@ -5820,6 +5866,7 @@ bool mysql_rename_user(THD *thd, List &list) List_iterator user_list(list); TABLE_LIST tables[GRANT_TABLES]; bool some_users_renamed= FALSE; + bool save_binlog_row_based; DBUG_ENTER("mysql_rename_user"); /* @@ -5827,11 +5874,16 @@ bool mysql_rename_user(THD *thd, List &list) row-based replication. The flag will be reset at the end of the statement. */ + save_binlog_row_based= thd->current_stmt_binlog_row_based; thd->clear_current_stmt_binlog_row_based(); /* RENAME USER may be skipped on replication client. */ if ((result= open_grant_tables(thd, tables))) + { + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result != 1); + } rw_wrlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); @@ -5878,6 +5930,8 @@ bool mysql_rename_user(THD *thd, List &list) rw_unlock(&LOCK_grant); close_thread_tables(thd); + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result); } @@ -5902,6 +5956,7 @@ bool mysql_revoke_all(THD *thd, List &list) int result; ACL_DB *acl_db; TABLE_LIST tables[GRANT_TABLES]; + bool save_binlog_row_based; DBUG_ENTER("mysql_revoke_all"); /* @@ -5909,10 +5964,15 @@ bool mysql_revoke_all(THD *thd, List &list) row-based replication. The flag will be reset at the end of the statement. */ + save_binlog_row_based= thd->current_stmt_binlog_row_based; thd->clear_current_stmt_binlog_row_based(); if ((result= open_grant_tables(thd, tables))) + { + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result != 1); + } rw_wrlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); @@ -6063,6 +6123,8 @@ bool mysql_revoke_all(THD *thd, List &list) if (result) my_message(ER_REVOKE_GRANTS, ER(ER_REVOKE_GRANTS), MYF(0)); + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(result); } @@ -6146,6 +6208,7 @@ bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name, TABLE_LIST tables[GRANT_TABLES]; HASH *hash= is_proc ? &proc_priv_hash : &func_priv_hash; Silence_routine_definer_errors error_handler; + bool save_binlog_row_based; DBUG_ENTER("sp_revoke_privileges"); if ((result= open_grant_tables(thd, tables))) @@ -6162,6 +6225,7 @@ bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name, row-based replication. The flag will be reset at the end of the statement. */ + save_binlog_row_based= thd->current_stmt_binlog_row_based; thd->clear_current_stmt_binlog_row_based(); /* Remove procedure access */ @@ -6198,6 +6262,8 @@ bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name, close_thread_tables(thd); thd->pop_internal_handler(); + /* Restore the state of binlog format */ + thd->current_stmt_binlog_row_based= save_binlog_row_based; DBUG_RETURN(error_handler.has_errors()); } -- cgit v1.2.1 From 2b16517522afad76bc94b07bdaa8af64091e713b Mon Sep 17 00:00:00 2001 From: He Zhenxing Date: Sun, 24 Jan 2010 15:03:23 +0800 Subject: Backport Bug#37148 to 5.1 --- sql/sql_acl.cc | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 7ba1a657578..1fd70b4039e 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -1659,8 +1659,8 @@ bool change_password(THD *thd, const char *host, const char *user, acl_user->host.hostname ? acl_user->host.hostname : "", new_password)); thd->clear_error(); - thd->binlog_query(THD::MYSQL_QUERY_TYPE, buff, query_length, - FALSE, FALSE, 0); + result= thd->binlog_query(THD::MYSQL_QUERY_TYPE, buff, query_length, + FALSE, FALSE, 0); } end: close_thread_tables(thd); @@ -3217,7 +3217,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, if (!result) /* success */ { - write_bin_log(thd, TRUE, thd->query(), thd->query_length()); + result= write_bin_log(thd, TRUE, thd->query(), thd->query_length()); } rw_unlock(&LOCK_grant); @@ -3383,7 +3383,8 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, if (write_to_binlog) { - write_bin_log(thd, FALSE, thd->query(), thd->query_length()); + if (write_bin_log(thd, FALSE, thd->query(), thd->query_length())) + result= TRUE; } rw_unlock(&LOCK_grant); @@ -3502,7 +3503,7 @@ bool mysql_grant(THD *thd, const char *db, List &list, if (!result) { - write_bin_log(thd, TRUE, thd->query(), thd->query_length()); + result= write_bin_log(thd, TRUE, thd->query(), thd->query_length()); } rw_unlock(&LOCK_grant); @@ -5716,7 +5717,7 @@ bool mysql_create_user(THD *thd, List &list) my_error(ER_CANNOT_USER, MYF(0), "CREATE USER", wrong_users.c_ptr_safe()); if (some_users_created) - write_bin_log(thd, FALSE, thd->query(), thd->query_length()); + result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length()); rw_unlock(&LOCK_grant); close_thread_tables(thd); @@ -5789,7 +5790,7 @@ bool mysql_drop_user(THD *thd, List &list) my_error(ER_CANNOT_USER, MYF(0), "DROP USER", wrong_users.c_ptr_safe()); if (some_users_deleted) - write_bin_log(thd, FALSE, thd->query(), thd->query_length()); + result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length()); rw_unlock(&LOCK_grant); close_thread_tables(thd); @@ -5874,7 +5875,7 @@ bool mysql_rename_user(THD *thd, List &list) my_error(ER_CANNOT_USER, MYF(0), "RENAME USER", wrong_users.c_ptr_safe()); if (some_users_renamed && mysql_bin_log.is_open()) - write_bin_log(thd, FALSE, thd->query(), thd->query_length()); + result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length()); rw_unlock(&LOCK_grant); close_thread_tables(thd); @@ -6056,15 +6057,17 @@ bool mysql_revoke_all(THD *thd, List &list) VOID(pthread_mutex_unlock(&acl_cache->lock)); - write_bin_log(thd, FALSE, thd->query(), thd->query_length()); + int binlog_error= + write_bin_log(thd, FALSE, thd->query(), thd->query_length()); rw_unlock(&LOCK_grant); close_thread_tables(thd); - if (result) + /* error for writing binary log has already been reported */ + if (result && !binlog_error) my_message(ER_REVOKE_GRANTS, ER(ER_REVOKE_GRANTS), MYF(0)); - DBUG_RETURN(result); + DBUG_RETURN(result || binlog_error); } -- cgit v1.2.1 From d9e9a73e8f1355a24b27d64d56d555d045ee0b4c Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 30 Jan 2010 20:49:25 +0800 Subject: Bug #48321 CURRENT_USER() incorrectly replicated for DROP/RENAME USER; REVOKE/GRANT; ALTER EVENT. The following statements support the CURRENT_USER() where a user is needed. DROP USER RENAME USER CURRENT_USER() ... GRANT ... TO CURRENT_USER() REVOKE ... FROM CURRENT_USER() ALTER DEFINER = CURRENT_USER() EVENT but, When these statements are binlogged, CURRENT_USER() just is binlogged as 'CURRENT_USER()', it is not expanded to the real user name. When slave executes the log event, 'CURRENT_USER()' is expand to the user of slave SQL thread, but SQL thread's user name always NULL. This breaks the replication. After this patch, All above statements are rewritten when they are binlogged. The CURRENT_USER() is expanded to the real user's name and host. --- sql/sql_acl.cc | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 112 insertions(+), 12 deletions(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index a8828d15cae..70983f69746 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -194,6 +194,7 @@ static bool compare_hostname(const acl_host_and_ip *host,const char *hostname, const char *ip); static my_bool acl_load(THD *thd, TABLE_LIST *tables); static my_bool grant_load(THD *thd, TABLE_LIST *tables); +static bool acl_write_bin_log(THD *thd, List &list, bool clear_error); /* Convert scrambled password to binary form, according to scramble type, @@ -3225,7 +3226,8 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, if (!result) /* success */ { - result= write_bin_log(thd, TRUE, thd->query(), thd->query_length()); + if (acl_write_bin_log(thd, user_list, TRUE)) + result= -1; } rw_unlock(&LOCK_grant); @@ -3401,8 +3403,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, if (write_to_binlog) { - if (write_bin_log(thd, FALSE, thd->query(), thd->query_length())) - result= TRUE; + result|= acl_write_bin_log(thd, user_list, FALSE); } rw_unlock(&LOCK_grant); @@ -3531,7 +3532,7 @@ bool mysql_grant(THD *thd, const char *db, List &list, if (!result) { - result= write_bin_log(thd, TRUE, thd->query(), thd->query_length()); + result= acl_write_bin_log(thd, list, TRUE); } rw_unlock(&LOCK_grant); @@ -5663,9 +5664,9 @@ static int handle_grant_data(TABLE_LIST *tables, bool drop, } -static void append_user(String *str, LEX_USER *user) +static void append_user(String *str, LEX_USER *user, bool comma= TRUE) { - if (str->length()) + if (comma && str->length()) str->append(','); str->append('\''); str->append(user->user.str); @@ -5674,6 +5675,65 @@ static void append_user(String *str, LEX_USER *user) str->append('\''); } +/* + The operations(DROP, RENAME, REVOKE, GRANT) will cause inconsistency between + master and slave, when CURRENT_USER() is used. To solve this problem, we + construct a new binlog statement in which CURRENT_USER() is replaced by + the real user name and host name. + */ +static bool acl_write_bin_log(THD *thd, List &list, bool clear_error) +{ + String log_query; + LEX *lex= thd->lex; + List_iterator user_list(list); + LEX_USER *user, *tmp_user; + + if (!mysql_bin_log.is_open()) + return FALSE; + + if (log_query.append(lex->stmt_begin, lex->stmt_user_begin - lex->stmt_begin)) + return TRUE; + while ((tmp_user= user_list++)) + { + if (!(user= get_current_user(thd, tmp_user))) + continue; + + /* + No User, but a password? + They did GRANT ... TO CURRENT_USER() IDENTIFIED BY ... ! + Get the current user, and shallow-copy the new password to them! + */ + if (!tmp_user->user.str && tmp_user->password.str) + user->password= tmp_user->password; + + if (log_query.append(" ", 1)) + return TRUE; + append_user(&log_query, user, FALSE); + /* Only 'GRANT' have password */ + if (user->password.str) + { + if (log_query.append(STRING_WITH_LEN(" IDENTIFIED BY ")) || + log_query.append(STRING_WITH_LEN("PASSWORD ")) || + log_query.append("'", 1) || + log_query.append(user->password.str, + user->password.length) || + log_query.append("'", 1)) + return TRUE; + } + if (log_query.append(",", 1)) + return TRUE; + } + /* It is binlogged only when at least one user is in the query */ + if (log_query.c_ptr()[log_query.length()-1] == ',') + { + log_query.length(log_query.length()-1); + if (log_query.append(lex->stmt_user_end, lex->stmt_end - lex->stmt_user_end)) + return TRUE; + return write_bin_log(thd, clear_error, log_query.c_ptr_safe(), + log_query.length()) != 0; + } + return FALSE; +} /* Create a list of users. @@ -5780,6 +5840,7 @@ bool mysql_drop_user(THD *thd, List &list) { int result; String wrong_users; + String log_query; LEX_USER *user_name, *tmp_user_name; List_iterator user_list(list); TABLE_LIST tables[GRANT_TABLES]; @@ -5809,6 +5870,7 @@ bool mysql_drop_user(THD *thd, List &list) rw_wrlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); + log_query.append(STRING_WITH_LEN("DROP USER")); while ((tmp_user_name= user_list++)) { if (!(user_name= get_current_user(thd, tmp_user_name))) @@ -5816,6 +5878,17 @@ bool mysql_drop_user(THD *thd, List &list) result= TRUE; continue; } + + /* + The operation will cause inconsistency between master and slave, when + CURRENT_USER() is used. To solve this problem, we construct a new + binlog statement in which CURRENT_USER() is replaced by the real user + name and host name. + */ + log_query.append(STRING_WITH_LEN(" ")); + append_user(&log_query, user_name, FALSE); + log_query.append(STRING_WITH_LEN(",")); + if (handle_grant_data(tables, 1, user_name, NULL) <= 0) { append_user(&wrong_users, user_name); @@ -5834,7 +5907,13 @@ bool mysql_drop_user(THD *thd, List &list) my_error(ER_CANNOT_USER, MYF(0), "DROP USER", wrong_users.c_ptr_safe()); if (some_users_deleted) - result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length()); + { + if (log_query.c_ptr()[log_query.length()-1] == ',') + { + log_query.length(log_query.length()-1); + result|= write_bin_log(thd, FALSE, log_query.c_ptr_safe(), log_query.length()); + } + } rw_unlock(&LOCK_grant); close_thread_tables(thd); @@ -5862,6 +5941,7 @@ bool mysql_rename_user(THD *thd, List &list) { int result; String wrong_users; + String log_query; LEX_USER *user_from, *tmp_user_from; LEX_USER *user_to, *tmp_user_to; List_iterator user_list(list); @@ -5889,6 +5969,7 @@ bool mysql_rename_user(THD *thd, List &list) rw_wrlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); + log_query.append(STRING_WITH_LEN("RENAME USER")); while ((tmp_user_from= user_list++)) { if (!(user_from= get_current_user(thd, tmp_user_from))) @@ -5904,6 +5985,18 @@ bool mysql_rename_user(THD *thd, List &list) } DBUG_ASSERT(user_to != 0); /* Syntax enforces pairs of users. */ + /* + The operation will cause inconsistency between master and slave, when + CURRENT_USER() is used. To solve this problem, we construct a new + binlog statement in which CURRENT_USER() is replaced by the real user + name and host name. + */ + log_query.append(STRING_WITH_LEN(" ")); + append_user(&log_query, user_from, FALSE); + log_query.append(STRING_WITH_LEN(" TO ")); + append_user(&log_query, user_to, FALSE); + log_query.append(STRING_WITH_LEN(",")); + /* Search all in-memory structures and grant tables for a mention of the new user name. @@ -5925,9 +6018,15 @@ bool mysql_rename_user(THD *thd, List &list) if (result) my_error(ER_CANNOT_USER, MYF(0), "RENAME USER", wrong_users.c_ptr_safe()); - - if (some_users_renamed && mysql_bin_log.is_open()) - result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length()); + + if (some_users_renamed) + { + if (log_query.c_ptr()[log_query.length()-1] == ',') + { + log_query.length(log_query.length()-1); + result|= write_bin_log(thd, FALSE, log_query.c_ptr_safe(), log_query.length()); + } + } rw_unlock(&LOCK_grant); close_thread_tables(thd); @@ -6117,8 +6216,9 @@ bool mysql_revoke_all(THD *thd, List &list) VOID(pthread_mutex_unlock(&acl_cache->lock)); - int binlog_error= - write_bin_log(thd, FALSE, thd->query(), thd->query_length()); + int binlog_error= 0; + if (acl_write_bin_log(thd, list, FALSE)) + binlog_error= 1; rw_unlock(&LOCK_grant); close_thread_tables(thd); -- cgit v1.2.1 From 59f1be1b636e360c410c81de1c41a8c4a254077d Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 2 Feb 2010 16:38:44 +0300 Subject: Revert a patch for Bug#48231, which introduced valgrind warnings. Original revision: ------------------------------------------------------------ revision-id: li-bing.song@sun.com-20100130124925-o6sfex42b6noyc6x parent: joro@sun.com-20100129145427-0n79l9hnk0q43ajk committer: branch nick: mysql-5.1-bugteam timestamp: Sat 2010-01-30 20:49:25 +0800 message: Bug #48321 CURRENT_USER() incorrectly replicated for DROP/RENAME USER; REVOKE/GRANT; ALTER EVENT. The following statements support the CURRENT_USER() where a user is needed. DROP USER RENAME USER CURRENT_USER() ... GRANT ... TO CURRENT_USER() REVOKE ... FROM CURRENT_USER() ALTER DEFINER = CURRENT_USER() EVENT but, When these statements are binlogged, CURRENT_USER() just is binlogged as 'CURRENT_USER()', it is not expanded to the real user name. When slave executes the log event, 'CURRENT_USER()' is expand to the user of slave SQL thread, but SQL thread's user name always NULL. This breaks the replication. After this patch, All above statements are rewritten when they are binlogged. The CURRENT_USER() is expanded to the real user's name and host. ------------------------------------------------------------ --- sql/sql_acl.cc | 124 ++++++--------------------------------------------------- 1 file changed, 12 insertions(+), 112 deletions(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 70983f69746..a8828d15cae 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -194,7 +194,6 @@ static bool compare_hostname(const acl_host_and_ip *host,const char *hostname, const char *ip); static my_bool acl_load(THD *thd, TABLE_LIST *tables); static my_bool grant_load(THD *thd, TABLE_LIST *tables); -static bool acl_write_bin_log(THD *thd, List &list, bool clear_error); /* Convert scrambled password to binary form, according to scramble type, @@ -3226,8 +3225,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, if (!result) /* success */ { - if (acl_write_bin_log(thd, user_list, TRUE)) - result= -1; + result= write_bin_log(thd, TRUE, thd->query(), thd->query_length()); } rw_unlock(&LOCK_grant); @@ -3403,7 +3401,8 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, if (write_to_binlog) { - result|= acl_write_bin_log(thd, user_list, FALSE); + if (write_bin_log(thd, FALSE, thd->query(), thd->query_length())) + result= TRUE; } rw_unlock(&LOCK_grant); @@ -3532,7 +3531,7 @@ bool mysql_grant(THD *thd, const char *db, List &list, if (!result) { - result= acl_write_bin_log(thd, list, TRUE); + result= write_bin_log(thd, TRUE, thd->query(), thd->query_length()); } rw_unlock(&LOCK_grant); @@ -5664,9 +5663,9 @@ static int handle_grant_data(TABLE_LIST *tables, bool drop, } -static void append_user(String *str, LEX_USER *user, bool comma= TRUE) +static void append_user(String *str, LEX_USER *user) { - if (comma && str->length()) + if (str->length()) str->append(','); str->append('\''); str->append(user->user.str); @@ -5675,65 +5674,6 @@ static void append_user(String *str, LEX_USER *user, bool comma= TRUE) str->append('\''); } -/* - The operations(DROP, RENAME, REVOKE, GRANT) will cause inconsistency between - master and slave, when CURRENT_USER() is used. To solve this problem, we - construct a new binlog statement in which CURRENT_USER() is replaced by - the real user name and host name. - */ -static bool acl_write_bin_log(THD *thd, List &list, bool clear_error) -{ - String log_query; - LEX *lex= thd->lex; - List_iterator user_list(list); - LEX_USER *user, *tmp_user; - - if (!mysql_bin_log.is_open()) - return FALSE; - - if (log_query.append(lex->stmt_begin, lex->stmt_user_begin - lex->stmt_begin)) - return TRUE; - while ((tmp_user= user_list++)) - { - if (!(user= get_current_user(thd, tmp_user))) - continue; - - /* - No User, but a password? - They did GRANT ... TO CURRENT_USER() IDENTIFIED BY ... ! - Get the current user, and shallow-copy the new password to them! - */ - if (!tmp_user->user.str && tmp_user->password.str) - user->password= tmp_user->password; - - if (log_query.append(" ", 1)) - return TRUE; - append_user(&log_query, user, FALSE); - /* Only 'GRANT' have password */ - if (user->password.str) - { - if (log_query.append(STRING_WITH_LEN(" IDENTIFIED BY ")) || - log_query.append(STRING_WITH_LEN("PASSWORD ")) || - log_query.append("'", 1) || - log_query.append(user->password.str, - user->password.length) || - log_query.append("'", 1)) - return TRUE; - } - if (log_query.append(",", 1)) - return TRUE; - } - /* It is binlogged only when at least one user is in the query */ - if (log_query.c_ptr()[log_query.length()-1] == ',') - { - log_query.length(log_query.length()-1); - if (log_query.append(lex->stmt_user_end, lex->stmt_end - lex->stmt_user_end)) - return TRUE; - return write_bin_log(thd, clear_error, log_query.c_ptr_safe(), - log_query.length()) != 0; - } - return FALSE; -} /* Create a list of users. @@ -5840,7 +5780,6 @@ bool mysql_drop_user(THD *thd, List &list) { int result; String wrong_users; - String log_query; LEX_USER *user_name, *tmp_user_name; List_iterator user_list(list); TABLE_LIST tables[GRANT_TABLES]; @@ -5870,7 +5809,6 @@ bool mysql_drop_user(THD *thd, List &list) rw_wrlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); - log_query.append(STRING_WITH_LEN("DROP USER")); while ((tmp_user_name= user_list++)) { if (!(user_name= get_current_user(thd, tmp_user_name))) @@ -5878,17 +5816,6 @@ bool mysql_drop_user(THD *thd, List &list) result= TRUE; continue; } - - /* - The operation will cause inconsistency between master and slave, when - CURRENT_USER() is used. To solve this problem, we construct a new - binlog statement in which CURRENT_USER() is replaced by the real user - name and host name. - */ - log_query.append(STRING_WITH_LEN(" ")); - append_user(&log_query, user_name, FALSE); - log_query.append(STRING_WITH_LEN(",")); - if (handle_grant_data(tables, 1, user_name, NULL) <= 0) { append_user(&wrong_users, user_name); @@ -5907,13 +5834,7 @@ bool mysql_drop_user(THD *thd, List &list) my_error(ER_CANNOT_USER, MYF(0), "DROP USER", wrong_users.c_ptr_safe()); if (some_users_deleted) - { - if (log_query.c_ptr()[log_query.length()-1] == ',') - { - log_query.length(log_query.length()-1); - result|= write_bin_log(thd, FALSE, log_query.c_ptr_safe(), log_query.length()); - } - } + result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length()); rw_unlock(&LOCK_grant); close_thread_tables(thd); @@ -5941,7 +5862,6 @@ bool mysql_rename_user(THD *thd, List &list) { int result; String wrong_users; - String log_query; LEX_USER *user_from, *tmp_user_from; LEX_USER *user_to, *tmp_user_to; List_iterator user_list(list); @@ -5969,7 +5889,6 @@ bool mysql_rename_user(THD *thd, List &list) rw_wrlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); - log_query.append(STRING_WITH_LEN("RENAME USER")); while ((tmp_user_from= user_list++)) { if (!(user_from= get_current_user(thd, tmp_user_from))) @@ -5985,18 +5904,6 @@ bool mysql_rename_user(THD *thd, List &list) } DBUG_ASSERT(user_to != 0); /* Syntax enforces pairs of users. */ - /* - The operation will cause inconsistency between master and slave, when - CURRENT_USER() is used. To solve this problem, we construct a new - binlog statement in which CURRENT_USER() is replaced by the real user - name and host name. - */ - log_query.append(STRING_WITH_LEN(" ")); - append_user(&log_query, user_from, FALSE); - log_query.append(STRING_WITH_LEN(" TO ")); - append_user(&log_query, user_to, FALSE); - log_query.append(STRING_WITH_LEN(",")); - /* Search all in-memory structures and grant tables for a mention of the new user name. @@ -6018,15 +5925,9 @@ bool mysql_rename_user(THD *thd, List &list) if (result) my_error(ER_CANNOT_USER, MYF(0), "RENAME USER", wrong_users.c_ptr_safe()); - - if (some_users_renamed) - { - if (log_query.c_ptr()[log_query.length()-1] == ',') - { - log_query.length(log_query.length()-1); - result|= write_bin_log(thd, FALSE, log_query.c_ptr_safe(), log_query.length()); - } - } + + if (some_users_renamed && mysql_bin_log.is_open()) + result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length()); rw_unlock(&LOCK_grant); close_thread_tables(thd); @@ -6216,9 +6117,8 @@ bool mysql_revoke_all(THD *thd, List &list) VOID(pthread_mutex_unlock(&acl_cache->lock)); - int binlog_error= 0; - if (acl_write_bin_log(thd, list, FALSE)) - binlog_error= 1; + int binlog_error= + write_bin_log(thd, FALSE, thd->query(), thd->query_length()); rw_unlock(&LOCK_grant); close_thread_tables(thd); -- cgit v1.2.1