From 9349f18b4491b55d8edc4a314131bd44108278ad Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 26 Oct 2005 15:34:57 +0200 Subject: Fixed BUG#14233: Crash after tampering with the mysql.proc table Added error checking for errors when attempting to use stored procedures after the mysql.proc table has been dropped, corrupted, or tampered with. Test cases were put in a separate file (sp-destruct.test). mysql-test/t/sp.test: Added comment. sql/share/errmsg.txt: New error message for corrupted mysql.proc table. sql/sp.cc: Check and return error code when caching stored routines. In the case when no error message has been set, set one. sql/sp.h: Return error code from stored routine cache function. sql/sql_base.cc: Check for error from sp_cache_routines_* calls. sql/sql_trigger.h: Updated friend declaration for sp_cache_routines*. mysql-test/r/sp-destruct.result: New test file for destruction of the mysql.proc table. mysql-test/t/sp-destruct.test: New result file for destruction of the mysql.proc table. --- sql/share/errmsg.txt | 2 ++ sql/sp.cc | 85 ++++++++++++++++++++++++++++++++++++++-------------- sql/sp.h | 10 +++---- sql/sql_base.cc | 32 +++++++++++++++----- sql/sql_trigger.h | 2 +- 5 files changed, 95 insertions(+), 36 deletions(-) (limited to 'sql') diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt index ccf11248a1f..141fd46d464 100644 --- a/sql/share/errmsg.txt +++ b/sql/share/errmsg.txt @@ -5422,3 +5422,5 @@ ER_NO_REFERENCED_ROW_2 23000 eng "Cannot add or update a child row: a foreign key constraint fails (%.192s)" ER_SP_BAD_VAR_SHADOW 42000 eng "Variable '%-.64s' must be quoted with `...`, or renamed" +ER_SP_PROC_TABLE_CORRUPT + eng "The table mysql.proc is missing, corrupt, or contains bad data (internal code %d)" diff --git a/sql/sp.cc b/sql/sp.cc index 8386c5d58a2..6f08897b1af 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -1414,20 +1414,22 @@ static void sp_update_stmt_used_routines(THD *thd, LEX *lex, SQL_LIST *src) first_no_prelock - If true, don't add tables or cache routines used by the body of the first routine (i.e. *start) will be executed in non-prelocked mode. + tabs_changed - Set to TRUE some tables were added, FALSE otherwise NOTE If some function is missing this won't be reported here. Instead this fact will be discovered during query execution. RETURN VALUE - TRUE - some tables were added - FALSE - no tables were added. + 0 - success + -x - failure (error code, like SP_PARSE_ERROR et al) */ -static bool +static int sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, Sroutine_hash_entry *start, - bool first_no_prelock) + bool first_no_prelock, bool *tabs_changed) { + int ret= 0; bool result= FALSE; bool first= TRUE; DBUG_ENTER("sp_cache_routines_and_add_tables_aux"); @@ -1453,12 +1455,35 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, name.m_name.str+= 1; name.m_name.length= name.m_qname.length - name.m_db.length - 1; - if (db_find_routine(thd, type, &name, &sp) == SP_OK) + switch ((ret= db_find_routine(thd, type, &name, &sp))) { - if (type == TYPE_ENUM_FUNCTION) - sp_cache_insert(&thd->sp_func_cache, sp); - else - sp_cache_insert(&thd->sp_proc_cache, sp); + case SP_OK: + { + if (type == TYPE_ENUM_FUNCTION) + sp_cache_insert(&thd->sp_func_cache, sp); + else + sp_cache_insert(&thd->sp_proc_cache, sp); + } + break; + case SP_KEY_NOT_FOUND: + ret= SP_OK; + break; + case SP_OPEN_TABLE_FAILED: + /* + Force it to attempt opening it again on subsequent calls; + otherwise we will get one error message the first time, and + then ER_SP_PROC_TABLE_CORRUPT (below) on subsequent tries. + */ + mysql_proc_table_exists= 1; + /* Fall through */ + default: + /* + In some cases no error has been set (e.g. get field failed, + when the proc table has been tampered with). + */ + if (! thd->net.report_error) + my_error(ER_SP_PROC_TABLE_CORRUPT, MYF(0), ret); + break; } delete newlex; thd->lex= oldlex; @@ -1473,7 +1498,9 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, } first= FALSE; } - DBUG_RETURN(result); + if (tabs_changed) + *tabs_changed= result; + DBUG_RETURN(ret); } @@ -1488,18 +1515,20 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, lex - LEX representing statement first_no_prelock - If true, don't add tables or cache routines used by the body of the first routine (i.e. *start) + tabs_changed - Set to TRUE some tables were added, FALSE otherwise RETURN VALUE - TRUE - some tables were added - FALSE - no tables were added. + 0 - success + -x - failure (error code, like SP_PARSE_ERROR et al) */ -bool -sp_cache_routines_and_add_tables(THD *thd, LEX *lex, bool first_no_prelock) +int +sp_cache_routines_and_add_tables(THD *thd, LEX *lex, bool first_no_prelock, + bool *tabs_changed) { return sp_cache_routines_and_add_tables_aux(thd, lex, (Sroutine_hash_entry *)lex->sroutines_list.first, - first_no_prelock); + first_no_prelock, tabs_changed); } @@ -1513,16 +1542,21 @@ sp_cache_routines_and_add_tables(THD *thd, LEX *lex, bool first_no_prelock) thd - thread context lex - LEX representing statement aux_lex - LEX representing view + + RETURN VALUE + 0 - success + -x - failure (error code, like SP_PARSE_ERROR et al) */ -void +int sp_cache_routines_and_add_tables_for_view(THD *thd, LEX *lex, LEX *aux_lex) { Sroutine_hash_entry **last_cached_routine_ptr= (Sroutine_hash_entry **)lex->sroutines_list.next; sp_update_stmt_used_routines(thd, lex, &aux_lex->sroutines_list); - (void)sp_cache_routines_and_add_tables_aux(thd, lex, - *last_cached_routine_ptr, FALSE); + return sp_cache_routines_and_add_tables_aux(thd, lex, + *last_cached_routine_ptr, FALSE, + NULL); } @@ -1536,12 +1570,18 @@ sp_cache_routines_and_add_tables_for_view(THD *thd, LEX *lex, LEX *aux_lex) thd - thread context lex - LEX respresenting statement triggers - triggers of the table + + RETURN VALUE + 0 - success + -x - failure (error code, like SP_PARSE_ERROR et al) */ -void +int sp_cache_routines_and_add_tables_for_triggers(THD *thd, LEX *lex, Table_triggers_list *triggers) { + int ret= 0; + if (add_used_routine(lex, thd->stmt_arena, &triggers->sroutines_key)) { Sroutine_hash_entry **last_cached_routine_ptr= @@ -1559,10 +1599,11 @@ sp_cache_routines_and_add_tables_for_triggers(THD *thd, LEX *lex, } } } - (void)sp_cache_routines_and_add_tables_aux(thd, lex, - *last_cached_routine_ptr, - FALSE); + ret= sp_cache_routines_and_add_tables_aux(thd, lex, + *last_cached_routine_ptr, + FALSE, NULL); } + return ret; } diff --git a/sql/sp.h b/sql/sp.h index 933e5793e4c..2332e8a0cbb 100644 --- a/sql/sp.h +++ b/sql/sp.h @@ -86,11 +86,11 @@ void sp_add_used_routine(LEX *lex, Query_arena *arena, sp_name *rt, char rt_type); void sp_remove_not_own_routines(LEX *lex); void sp_update_sp_used_routines(HASH *dst, HASH *src); -bool sp_cache_routines_and_add_tables(THD *thd, LEX *lex, - bool first_no_prelock); -void sp_cache_routines_and_add_tables_for_view(THD *thd, LEX *lex, - LEX *aux_lex); -void sp_cache_routines_and_add_tables_for_triggers(THD *thd, LEX *lex, +int sp_cache_routines_and_add_tables(THD *thd, LEX *lex, + bool first_no_prelock, bool *tabs_changed); +int sp_cache_routines_and_add_tables_for_view(THD *thd, LEX *lex, + LEX *aux_lex); +int sp_cache_routines_and_add_tables_for_triggers(THD *thd, LEX *lex, Table_triggers_list *triggers); extern "C" byte* sp_sroutine_key(const byte *ptr, uint *plen, my_bool first); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 973fbca12f5..ed05dc6720a 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1978,15 +1978,20 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) if (!thd->prelocked_mode && !thd->lex->requires_prelocking() && thd->lex->sroutines_list.elements) { - bool first_no_prelocking, need_prelocking; + bool first_no_prelocking, need_prelocking, tabs_changed; TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last; DBUG_ASSERT(thd->lex->query_tables == *start); sp_get_prelocking_info(thd, &need_prelocking, &first_no_prelocking); - if ((sp_cache_routines_and_add_tables(thd, thd->lex, - first_no_prelocking) || - *start) && need_prelocking) + if (sp_cache_routines_and_add_tables(thd, thd->lex, + first_no_prelocking, + &tabs_changed) < 0) + { + result= -1; // Fatal error + goto err; + } + else if ((tabs_changed || *start) && need_prelocking) { query_tables_last_own= save_query_tables_last; *start= thd->lex->query_tables; @@ -2110,9 +2115,13 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) tables->lock_type >= TL_WRITE_ALLOW_WRITE) { if (!query_tables_last_own) - query_tables_last_own= thd->lex->query_tables_last; - sp_cache_routines_and_add_tables_for_triggers(thd, thd->lex, - tables->table->triggers); + query_tables_last_own= thd->lex->query_tables_last; + if (sp_cache_routines_and_add_tables_for_triggers(thd, thd->lex, + tables->table->triggers) < 0) + { + result= -1; // Fatal error + goto err; + } } free_root(&new_frm_mem, MYF(MY_KEEP_PREALLOC)); } @@ -2133,9 +2142,16 @@ process_view_routines: /* We have at least one table in TL here. */ if (!query_tables_last_own) query_tables_last_own= thd->lex->query_tables_last; - sp_cache_routines_and_add_tables_for_view(thd, thd->lex, tables->view); + if (sp_cache_routines_and_add_tables_for_view(thd, thd->lex, + tables->view) < 0) + { + result= -1; // Fatal error + goto err; + } } } + + err: thd->proc_info=0; free_root(&new_frm_mem, MYF(0)); // Free pre-alloced block diff --git a/sql/sql_trigger.h b/sql/sql_trigger.h index c1d1f8d0e9e..f50624767be 100644 --- a/sql/sql_trigger.h +++ b/sql/sql_trigger.h @@ -101,7 +101,7 @@ public: void set_table(TABLE *new_table); friend class Item_trigger_field; - friend void sp_cache_routines_and_add_tables_for_triggers(THD *thd, LEX *lex, + friend int sp_cache_routines_and_add_tables_for_triggers(THD *thd, LEX *lex, Table_triggers_list *triggers); private: -- cgit v1.2.1 From d4088df5e9a02e714d85f79bec3ea97cdd8128c6 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 25 Nov 2005 17:09:26 +0100 Subject: Fixed BUG#14233: Crash after tampering with the mysql.proc table Post-review version. Some minor review fixes, but also changed the way some errors are handled: Don't return specific parse errors; instead always use the more general "table corrupt" error (amended accordingly). mysql-test/r/sp-destruct.result: Updated results. mysql-test/r/sp-error.result: Updated for fully qualified name in "no return" error message. mysql-test/t/sp-destruct.test: Adopted the more consistent error handling for a corrupted mysql.proc table. (No more "parse error" et al). sql/share/errmsg.txt: Changed ER_SP_PROC_TABLE_CORRUPT to be more explicit. sql/sp.cc: Review fixes. Changed the handling of parse errors, and added the routine name to the "table corrupt" error message. sql/sql_base.cc: Review changes: Change error tests and added comments. sql/sql_parse.cc: Mored ER_SP_NORETURN test of functions to sql_yacc.yy for more general error handling. sql/sql_yacc.yy: Mored ER_SP_NORETURN test of functions from sql_parse.cc for more general error handling. --- sql/share/errmsg.txt | 2 +- sql/sp.cc | 45 ++++++++++++++++++++++++++++----------------- sql/sql_base.cc | 27 +++++++++++++++++++++------ sql/sql_parse.cc | 8 -------- sql/sql_yacc.yy | 5 +++++ 5 files changed, 55 insertions(+), 32 deletions(-) (limited to 'sql') diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt index 2e2403a21a1..eab46e308b8 100644 --- a/sql/share/errmsg.txt +++ b/sql/share/errmsg.txt @@ -5422,4 +5422,4 @@ ER_NO_REFERENCED_ROW_2 23000 ER_SP_BAD_VAR_SHADOW 42000 eng "Variable '%-.64s' must be quoted with `...`, or renamed" ER_SP_PROC_TABLE_CORRUPT - eng "The table mysql.proc is missing, corrupt, or contains bad data (internal code %d)" + eng "Failed to load routine %s. The table mysql.proc is missing, corrupt, or contains bad data (internal code %d)" diff --git a/sql/sp.cc b/sql/sp.cc index 6f08897b1af..49671a47bad 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -1420,8 +1420,8 @@ static void sp_update_stmt_used_routines(THD *thd, LEX *lex, SQL_LIST *src) Instead this fact will be discovered during query execution. RETURN VALUE - 0 - success - -x - failure (error code, like SP_PARSE_ERROR et al) + 0 - success + non-0 - failure */ static int @@ -1430,7 +1430,7 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, bool first_no_prelock, bool *tabs_changed) { int ret= 0; - bool result= FALSE; + int tabschnd= 0; /* Set if tables changed */ bool first= TRUE; DBUG_ENTER("sp_cache_routines_and_add_tables_aux"); @@ -1478,11 +1478,21 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, /* Fall through */ default: /* - In some cases no error has been set (e.g. get field failed, - when the proc table has been tampered with). - */ - if (! thd->net.report_error) - my_error(ER_SP_PROC_TABLE_CORRUPT, MYF(0), ret); + Any error when loading an existing routine is either some problem + with the mysql.proc table, or a parse error because the contents + has been tampered with (in which case we clear that error). + */ + if (ret == SP_PARSE_ERROR) + thd->clear_error(); + if (!thd->net.report_error) + { + char n[NAME_LEN*2+2]; + + /* m_qname.str is not always \0 terminated */ + memcpy(n, name.m_qname.str, name.m_qname.length); + n[name.m_qname.length]= '\0'; + my_error(ER_SP_PROC_TABLE_CORRUPT, MYF(0), n, ret); + } break; } delete newlex; @@ -1493,13 +1503,14 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, if (!(first && first_no_prelock)) { sp_update_stmt_used_routines(thd, lex, &sp->m_sroutines); - result|= sp->add_used_tables_to_table_list(thd, &lex->query_tables_last); + tabschnd|= + sp->add_used_tables_to_table_list(thd, &lex->query_tables_last); } } first= FALSE; } - if (tabs_changed) - *tabs_changed= result; + if (tabs_changed) /* it can be NULL */ + *tabs_changed= tabschnd; DBUG_RETURN(ret); } @@ -1518,8 +1529,8 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, tabs_changed - Set to TRUE some tables were added, FALSE otherwise RETURN VALUE - 0 - success - -x - failure (error code, like SP_PARSE_ERROR et al) + 0 - success + non-0 - failure */ int @@ -1544,8 +1555,8 @@ sp_cache_routines_and_add_tables(THD *thd, LEX *lex, bool first_no_prelock, aux_lex - LEX representing view RETURN VALUE - 0 - success - -x - failure (error code, like SP_PARSE_ERROR et al) + 0 - success + non-0 - failure */ int @@ -1572,8 +1583,8 @@ sp_cache_routines_and_add_tables_for_view(THD *thd, LEX *lex, LEX *aux_lex) triggers - triggers of the table RETURN VALUE - 0 - success - -x - failure (error code, like SP_PARSE_ERROR et al) + 0 - success + non-0 - failure */ int diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 273d9f7e3b4..fa3321bcb0e 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1986,9 +1986,14 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) if (sp_cache_routines_and_add_tables(thd, thd->lex, first_no_prelocking, - &tabs_changed) < 0) + &tabs_changed)) { - result= -1; // Fatal error + /* + Serious error during reading stored routines from mysql.proc table. + Something's wrong with the table or its contents, and an error has + been emitted; we must abort. + */ + result= -1; goto err; } else if ((tabs_changed || *start) && need_prelocking) @@ -2117,9 +2122,14 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) if (!query_tables_last_own) query_tables_last_own= thd->lex->query_tables_last; if (sp_cache_routines_and_add_tables_for_triggers(thd, thd->lex, - tables->table->triggers) < 0) + tables->table->triggers)) { - result= -1; // Fatal error + /* + Serious error during reading stored routines from mysql.proc table. + Something's wrong with the table or its contents, and an error has + been emitted; we must abort. + */ + result= -1; goto err; } } @@ -2143,9 +2153,14 @@ process_view_routines: if (!query_tables_last_own) query_tables_last_own= thd->lex->query_tables_last; if (sp_cache_routines_and_add_tables_for_view(thd, thd->lex, - tables->view) < 0) + tables->view)) { - result= -1; // Fatal error + /* + Serious error during reading stored routines from mysql.proc table. + Something's wrong with the table or its contents, and an error has + been emitted; we must abort. + */ + result= -1; goto err; } } diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 1882965fd1e..ce157eeb46f 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -4111,14 +4111,6 @@ end_with_restore_list: } } #endif - if (lex->sphead->m_type == TYPE_ENUM_FUNCTION && - !(lex->sphead->m_flags & sp_head::HAS_RETURN)) - { - my_error(ER_SP_NORETURN, MYF(0), name); - delete lex->sphead; - lex->sphead= 0; - goto error; - } /* We need to copy name and db in order to use them for diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 109dcd7e86a..841d4c90a70 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1475,6 +1475,11 @@ create_function_tail: YYABORT; lex->sql_command= SQLCOM_CREATE_SPFUNCTION; sp->init_strings(YYTHD, lex, lex->spname); + if (!(sp->m_flags & sp_head::HAS_RETURN)) + { + my_error(ER_SP_NORETURN, MYF(0), sp->m_qname.str); + YYABORT; + } /* Restore flag if it was cleared above */ if (sp->m_old_cmq) YYTHD->client_capabilities |= CLIENT_MULTI_QUERIES; -- cgit v1.2.1 From 29eac312cda3bebddeb7f5ab7a2af6b81ea89205 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 6 Dec 2005 14:25:12 +0100 Subject: Final review fix of #14233: Crash after tampering with the mysql.proc table. Changed variable type and added comment in sp.c. sql/sp.cc: Changed variable type and added comment. (Review fix) --- sql/sp.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/sp.cc b/sql/sp.cc index 7dc872f6225..3e5a3cb94f1 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -1462,7 +1462,7 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, bool first_no_prelock, bool *tabs_changed) { int ret= 0; - int tabschnd= 0; /* Set if tables changed */ + bool tabschnd= 0; /* Set if tables changed */ bool first= TRUE; DBUG_ENTER("sp_cache_routines_and_add_tables_aux"); @@ -1512,6 +1512,11 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, */ if (ret == SP_PARSE_ERROR) thd->clear_error(); + /* + If we cleared the parse error, or when db_find_routine() flagged + an error with it's return value without calling my_error(), we + set the generic "mysql.proc table corrupt" error here. + */ if (!thd->net.report_error) { char n[NAME_LEN*2+2]; -- cgit v1.2.1