From: Dmitry Lenev Date: December 1 2010 10:12am Subject: bzr push into mysql-trunk-bugfixing branch (Dmitry.Lenev:3392 to 3393) Bug#27480 List-Archive: http://lists.mysql.com/commits/125631 X-Bug: 27480 Message-Id: <20101201101242.807A91E520D@mockturtle> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3393 Dmitry Lenev 2010-12-01 Prerequisite patch for Bug#27480 (Extend CREATE TEMPORARY TABLES privilege to allow temp table operations). Review fixes in progress. Fixed handling of administrative commands, extended test-coverage. modified: mysql-test/r/grant4.result mysql-test/t/grant4.test sql/sql_admin.cc sql/sql_parse.cc 3392 Dmitry Lenev 2010-11-30 Prerequisite patch for Bug#27480 (Extend CREATE TEMPORARY TABLES privilege to allow temp table operations). Review fixes in progress. modified: sql/sp_head.cc sql/sql_view.cc === modified file 'mysql-test/r/grant4.result' --- a/mysql-test/r/grant4.result 2010-02-20 10:07:32 +0000 +++ b/mysql-test/r/grant4.result 2010-12-01 10:07:22 +0000 @@ -121,3 +121,60 @@ View Create View character_set_client co v3 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v3` AS select `t_select_priv`.`a` AS `a`,`t_select_priv`.`b` AS `b` from `t_select_priv` latin1 latin1_swedish_ci drop database mysqltest_db1; drop user mysqltest_u1@localhost; +# +# Additional coverage for refactoring which is made as part +# of fix for bug #27480 "Extend CREATE TEMPORARY TABLES privilege +# to allow temp table operations". +# +# Check that for statements like CHECK/REPAIR and OPTIMIZE TABLE +# privileges for all tables involved are checked before processing +# any tables. Doing otherwise, i.e. checking privileges for table +# right before processing it might result in lost results for tables +# which were processed by the time when table for which privileges +# are insufficient are discovered. +# +call mtr.add_suppression("Got an error from thread_id=.*ha_myisam.cc:"); +call mtr.add_suppression("MySQL thread id .*, query id .* localhost.*mysqltest_u1 Checking table"); +drop database if exists mysqltest_db1; +create database mysqltest_db1; +# Create tables which we are going to CHECK/REPAIR. +create table mysqltest_db1.t1 (a int, key(a)) engine=myisam; +create table mysqltest_db1.t2 (b int); +insert into mysqltest_db1.t1 values (1), (2); +insert into mysqltest_db1.t2 values (1); +# Create user which will try to do this. +create user mysqltest_u1@localhost; +grant insert, select on mysqltest_db1.t1 to mysqltest_u1@localhost; +# Corrupt t1 by replacing t1.MYI with a corrupt + unclosed one created +# by doing: 'create table t1 (a int key(a))' +# head -c1024 t1.MYI > corrupt_t1.MYI +flush table mysqltest_db1.t1; +# Switching to connection 'con1'. +check table mysqltest_db1.t1; +Table Op Msg_type Msg_text +mysqltest_db1.t1 check warning 1 client is using or hasn't closed the table properly +mysqltest_db1.t1 check error Size of indexfile is: 1024 Should be: 2048 +mysqltest_db1.t1 check warning Size of datafile is: 14 Should be: 7 +mysqltest_db1.t1 check error Corrupt +# The below statement should fail before repairing t1. +# Otherwise info about such repair will be missing from its result-set. +repair table mysqltest_db1.t1, mysqltest_db1.t2; +ERROR 42000: SELECT, INSERT command denied to user 'mysqltest_u1'@'localhost' for table 't2' +# The same is true for CHECK TABLE statement. +check table mysqltest_db1.t1, mysqltest_db1.t2; +ERROR 42000: SELECT command denied to user 'mysqltest_u1'@'localhost' for table 't2' +check table mysqltest_db1.t1; +Table Op Msg_type Msg_text +mysqltest_db1.t1 check warning Table is marked as crashed +mysqltest_db1.t1 check warning 1 client is using or hasn't closed the table properly +mysqltest_db1.t1 check error Size of indexfile is: 1024 Should be: 2048 +mysqltest_db1.t1 check warning Size of datafile is: 14 Should be: 7 +mysqltest_db1.t1 check error Corrupt +repair table mysqltest_db1.t1; +Table Op Msg_type Msg_text +mysqltest_db1.t1 repair warning Number of rows changed from 1 to 2 +mysqltest_db1.t1 repair status OK +# Clean-up. +# Switching to connection 'default'. +drop database mysqltest_db1; +drop user mysqltest_u1@localhost; === modified file 'mysql-test/t/grant4.test' --- a/mysql-test/t/grant4.test 2009-10-19 12:58:13 +0000 +++ b/mysql-test/t/grant4.test 2010-12-01 10:07:22 +0000 @@ -144,3 +144,62 @@ connection default; disconnect con1; drop database mysqltest_db1; drop user mysqltest_u1@localhost; + + +--echo # +--echo # Additional coverage for refactoring which is made as part +--echo # of fix for bug #27480 "Extend CREATE TEMPORARY TABLES privilege +--echo # to allow temp table operations". +--echo # +--echo # Check that for statements like CHECK/REPAIR and OPTIMIZE TABLE +--echo # privileges for all tables involved are checked before processing +--echo # any tables. Doing otherwise, i.e. checking privileges for table +--echo # right before processing it might result in lost results for tables +--echo # which were processed by the time when table for which privileges +--echo # are insufficient are discovered. +--echo # +call mtr.add_suppression("Got an error from thread_id=.*ha_myisam.cc:"); +call mtr.add_suppression("MySQL thread id .*, query id .* localhost.*mysqltest_u1 Checking table"); +--disable_warnings +drop database if exists mysqltest_db1; +--enable_warnings +let $MYSQLD_DATADIR = `SELECT @@datadir`; +create database mysqltest_db1; +--echo # Create tables which we are going to CHECK/REPAIR. +create table mysqltest_db1.t1 (a int, key(a)) engine=myisam; +create table mysqltest_db1.t2 (b int); +insert into mysqltest_db1.t1 values (1), (2); +insert into mysqltest_db1.t2 values (1); +--echo # Create user which will try to do this. +create user mysqltest_u1@localhost; +grant insert, select on mysqltest_db1.t1 to mysqltest_u1@localhost; +connect (con1,localhost,mysqltest_u1,,); +connection default; + +--echo # Corrupt t1 by replacing t1.MYI with a corrupt + unclosed one created +--echo # by doing: 'create table t1 (a int key(a))' +--echo # head -c1024 t1.MYI > corrupt_t1.MYI +flush table mysqltest_db1.t1; +--remove_file $MYSQLD_DATADIR/mysqltest_db1/t1.MYI +--copy_file std_data/corrupt_t1.MYI $MYSQLD_DATADIR/mysqltest_db1/t1.MYI + +--echo # Switching to connection 'con1'. +connection con1; +check table mysqltest_db1.t1; +--echo # The below statement should fail before repairing t1. +--echo # Otherwise info about such repair will be missing from its result-set. +--error ER_TABLEACCESS_DENIED_ERROR +repair table mysqltest_db1.t1, mysqltest_db1.t2; +--echo # The same is true for CHECK TABLE statement. +--error ER_TABLEACCESS_DENIED_ERROR +check table mysqltest_db1.t1, mysqltest_db1.t2; +check table mysqltest_db1.t1; +repair table mysqltest_db1.t1; + +--echo # Clean-up. +disconnect con1; +--source include/wait_until_disconnected.inc +--echo # Switching to connection 'default'. +connection default; +drop database mysqltest_db1; +drop user mysqltest_u1@localhost; === modified file 'sql/sql_admin.cc' --- a/sql/sql_admin.cc 2010-11-29 14:13:07 +0000 +++ b/sql/sql_admin.cc 2010-12-01 10:07:22 +0000 @@ -261,8 +261,6 @@ static bool mysql_admin_table(THD* thd, HA_CHECK_OPT* check_opt, const char *operator_name, thr_lock_type lock_type, - bool any_priv_will_do, - ulong op_privs, bool open_for_modify, bool no_warnings_for_error, uint extra_open_options, @@ -293,6 +291,12 @@ static bool mysql_admin_table(THD* thd, Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)) DBUG_RETURN(TRUE); + /* + Close all temporary tables which were pre-open to simplify + privilege checking. + */ + close_thread_tables(thd); + for (table= tables; table; table= table->next_local) { char table_name[NAME_LEN*2+2]; @@ -300,29 +304,6 @@ static bool mysql_admin_table(THD* thd, bool fatal_error=0; bool open_error; - /* - Here we need to open list of tables (one table is not enough) because - of merge tables: children of merge tables must be opened here. - */ - - TABLE_LIST *next_global_tl= table->next_global; - - if (open_and_process_temporary_table_list(thd, table)) - DBUG_RETURN(TRUE); - - TABLE_LIST *last_added_tl; - - for (last_added_tl= table; - last_added_tl && last_added_tl->next_global != next_global_tl; - last_added_tl= last_added_tl->next_global) - ; - - if (op_privs) - { - if (check_table_access(thd, op_privs, table, any_priv_will_do, 1, FALSE)) - DBUG_RETURN(TRUE); - } - DBUG_PRINT("admin", ("table: '%s'.'%s'", table->db, table->table_name)); DBUG_PRINT("admin", ("extra_open_options: %u", extra_open_options)); strxmov(table_name, db, ".", table->table_name, NullS); @@ -338,10 +319,11 @@ static bool mysql_admin_table(THD* thd, MDL_SHARED_NO_READ_WRITE : MDL_SHARED_READ); /* open only one table from local list of command */ { - TABLE_LIST *next_global_saved= last_added_tl->next_global; - TABLE_LIST *next_local_saved= last_added_tl->next_local; - last_added_tl->next_global= NULL; - last_added_tl->next_local= NULL; + TABLE_LIST *save_next_global, *save_next_local; + save_next_global= table->next_global; + table->next_global= 0; + save_next_local= table->next_local; + table->next_local= 0; select->table_list.first= table; /* Time zone tables and SP tables can be add to lex->query_tables list, @@ -362,10 +344,14 @@ static bool mysql_admin_table(THD* thd, if (view_operator_func == NULL) table->required_type=FRMTYPE_TABLE; - open_error= open_and_lock_tables(thd, table, TRUE, 0); + open_error= open_and_process_temporary_table_list(thd, table); + + if (! open_error) + open_error= open_and_lock_tables(thd, table, TRUE, 0); + thd->no_warnings_for_error= 0; - last_added_tl->next_global= next_global_saved; - last_added_tl->next_local= next_local_saved; + table->next_global= save_next_global; + table->next_local= save_next_local; thd->open_options&= ~extra_open_options; } @@ -926,10 +912,8 @@ bool mysql_assign_to_keycache(THD* thd, mysql_mutex_unlock(&LOCK_global_system_variables); check_opt.key_cache= key_cache; DBUG_RETURN(mysql_admin_table(thd, tables, &check_opt, - "assign_to_keycache", TL_READ_NO_INSERT, - FALSE, 0, - FALSE, FALSE, 0, NULL, - &handler::assign_to_keycache, NULL)); + "assign_to_keycache", TL_READ_NO_INSERT, 0, 0, + 0, 0, &handler::assign_to_keycache, 0)); } @@ -955,10 +939,8 @@ bool mysql_preload_keys(THD* thd, TABLE_ outdated information if parallel inserts into cache blocks happen. */ DBUG_RETURN(mysql_admin_table(thd, tables, 0, - "preload_keys", TL_READ_NO_INSERT, - FALSE, 0, - FALSE, FALSE, 0, NULL, - &handler::preload_keys, NULL)); + "preload_keys", TL_READ_NO_INSERT, 0, 0, 0, 0, + &handler::preload_keys, 0)); } @@ -969,12 +951,13 @@ bool Sql_cmd_analyze_table::execute(THD thr_lock_type lock_type = TL_READ_NO_INSERT; DBUG_ENTER("Sql_cmd_analyze_table::execute"); + if (check_table_access(thd, SELECT_ACL | INSERT_ACL, first_table, + FALSE, UINT_MAX, FALSE)) + goto error; thd->enable_slow_log= opt_log_slow_admin_statements; res= mysql_admin_table(thd, first_table, &thd->lex->check_opt, - "analyze", lock_type, - FALSE, SELECT_ACL | INSERT_ACL, - TRUE, FALSE, 0, NULL, &handler::ha_analyze, NULL); - + "analyze", lock_type, 1, 0, 0, 0, + &handler::ha_analyze, 0); /* ! we write after unlocking the table */ if (!res && !thd->lex->no_write_to_binlog) { @@ -986,6 +969,7 @@ bool Sql_cmd_analyze_table::execute(THD thd->lex->select_lex.table_list.first= first_table; thd->lex->query_tables= first_table; +error: DBUG_RETURN(res); } @@ -1003,9 +987,7 @@ bool Sql_cmd_check_table::execute(THD *t thd->enable_slow_log= opt_log_slow_admin_statements; res= mysql_admin_table(thd, first_table, &thd->lex->check_opt, "check", - lock_type, - TRUE, SELECT_ACL, - FALSE, FALSE, HA_OPEN_FOR_REPAIR, NULL, + lock_type, 0, 0, HA_OPEN_FOR_REPAIR, 0, &handler::ha_check, &view_checksum); thd->lex->select_lex.table_list.first= first_table; @@ -1029,9 +1011,8 @@ bool Sql_cmd_optimize_table::execute(THD res= (specialflag & (SPECIAL_SAFE_MODE | SPECIAL_NO_NEW_FUNC)) ? mysql_recreate_table(thd, first_table) : mysql_admin_table(thd, first_table, &thd->lex->check_opt, - "optimize", TL_WRITE, - FALSE, SELECT_ACL | INSERT_ACL, - TRUE, FALSE, 0, NULL, &handler::ha_optimize, 0); + "optimize", TL_WRITE, 1, 0, 0, 0, + &handler::ha_optimize, 0); /* ! we write after unlocking the table */ if (!res && !thd->lex->no_write_to_binlog) { @@ -1059,12 +1040,10 @@ bool Sql_cmd_repair_table::execute(THD * goto error; /* purecov: inspected */ thd->enable_slow_log= opt_log_slow_admin_statements; res= mysql_admin_table(thd, first_table, &thd->lex->check_opt, "repair", - TL_WRITE, - FALSE, SELECT_ACL| INSERT_ACL, - TRUE, + TL_WRITE, 1, test(thd->lex->check_opt.sql_flags & TT_USEFRM), HA_OPEN_FOR_REPAIR, &prepare_for_repair, - &handler::ha_repair, NULL); + &handler::ha_repair, 0); /* ! we write after unlocking the table */ if (!res && !thd->lex->no_write_to_binlog) === modified file 'sql/sql_parse.cc' --- a/sql/sql_parse.cc 2010-11-29 14:13:07 +0000 +++ b/sql/sql_parse.cc 2010-12-01 10:07:22 +0000 @@ -476,6 +476,12 @@ void init_update_queries(void) sql_command_flags[SQLCOM_DO]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_CALL]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_CHECKSUM]|= CF_PREOPEN_TMP_TABLES; + sql_command_flags[SQLCOM_ANALYZE]|= CF_PREOPEN_TMP_TABLES; + sql_command_flags[SQLCOM_CHECK]|= CF_PREOPEN_TMP_TABLES; + sql_command_flags[SQLCOM_OPTIMIZE]|= CF_PREOPEN_TMP_TABLES; + sql_command_flags[SQLCOM_REPAIR]|= CF_PREOPEN_TMP_TABLES; + sql_command_flags[SQLCOM_PRELOAD_KEYS]|= CF_PREOPEN_TMP_TABLES; + sql_command_flags[SQLCOM_ASSIGN_TO_KEYCACHE]|= CF_PREOPEN_TMP_TABLES; /* DDL statements should start with closing opened handlers. No bundle (reason: useless for push emails).