From 095e8271b768ca50df05557e9833b7c8f9d5636a Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Wed, 5 Dec 2012 19:26:56 +0400 Subject: [PATCH] Bug #15954896 "SP, MULTI-TABLE DELETE AND LONG ALIAS". Using too long table aliases in stored routines might have caused server crashes. Code in sp_head::merge_table_list() which is responsible for collecting information about tables used in stored routine was not aware of the fact that table alias might have arbitrary length. I.e. it assumed that table alias can't be longer than NAME_LEN bytes and allocated buffer for a key identifying table accordingly. This patch fixes the issue by ensuring that we use dynamically allocated buffer for table key when table alias is too long. By default stack based buffer is used in which NAME_LEN bytes are reserved for table alias. --- sql/sp_head.cc | 48 +++++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 7eef9f5ab28..054fc5e223e 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -3839,8 +3839,6 @@ typedef struct st_sp_table Multi-set key: db_name\0table_name\0alias\0 - for normal tables db_name\0table_name\0 - for temporary tables - Note that in both cases we don't take last '\0' into account when - we count length of key. */ LEX_STRING qname; uint db_length, table_name_length; @@ -3897,19 +3895,26 @@ sp_head::merge_table_list(THD *thd, TABLE_LIST *table, LEX *lex_for_tmp_check) for (; table ; table= table->next_global) if (!table->derived && !table->schema_table) { - char tname[(NAME_LEN + 1) * 3]; // db\0table\0alias\0 - uint tlen, alen; + /* + Structure of key for the multi-set is "db\0table\0alias\0". + Since "alias" part can have arbitrary length we use String + object to construct the key. By default String will use + buffer allocated on stack with NAME_LEN bytes reserved for + alias, since in most cases it is going to be smaller than + NAME_LEN bytes. + */ + char tname_buff[(NAME_LEN + 1) * 3]; + String tname(tname_buff, sizeof(tname_buff), &my_charset_bin); + uint temp_table_key_length; - tlen= table->db_length; - memcpy(tname, table->db, tlen); - tname[tlen++]= '\0'; - memcpy(tname+tlen, table->table_name, table->table_name_length); - tlen+= table->table_name_length; - tname[tlen++]= '\0'; - alen= strlen(table->alias); - memcpy(tname+tlen, table->alias, alen); - tlen+= alen; - tname[tlen]= '\0'; + tname.length(0); + tname.append(table->db, table->db_length); + tname.append('\0'); + tname.append(table->table_name, table->table_name_length); + tname.append('\0'); + temp_table_key_length= tname.length(); + tname.append(table->alias); + tname.append('\0'); /* Upgrade the lock type because this table list will be used @@ -3924,9 +3929,10 @@ sp_head::merge_table_list(THD *thd, TABLE_LIST *table, LEX *lex_for_tmp_check) (and therefore should not be prelocked). Otherwise we will erroneously treat table with same name but with different alias as non-temporary. */ - if ((tab= (SP_TABLE *)hash_search(&m_sptabs, (uchar *)tname, tlen)) || - ((tab= (SP_TABLE *)hash_search(&m_sptabs, (uchar *)tname, - tlen - alen - 1)) && + if ((tab= (SP_TABLE *)hash_search(&m_sptabs, (uchar *)tname.ptr(), + tname.length())) || + ((tab= (SP_TABLE *)hash_search(&m_sptabs, (uchar *)tname.ptr(), + temp_table_key_length)) && tab->temp)) { if (tab->lock_type < table->lock_type) @@ -3945,11 +3951,11 @@ sp_head::merge_table_list(THD *thd, TABLE_LIST *table, LEX *lex_for_tmp_check) lex_for_tmp_check->create_info.options & HA_LEX_CREATE_TMP_TABLE) { tab->temp= TRUE; - tab->qname.length= tlen - alen - 1; + tab->qname.length= temp_table_key_length; } else - tab->qname.length= tlen; - tab->qname.str= (char*) thd->memdup(tname, tab->qname.length + 1); + tab->qname.length= tname.length(); + tab->qname.str= (char*) thd->memdup(tname.ptr(), tab->qname.length); if (!tab->qname.str) return FALSE; tab->table_name_length= table->table_name_length; @@ -4018,7 +4024,7 @@ sp_head::add_used_tables_to_table_list(THD *thd, if (!(tab_buff= (char *)thd->calloc(ALIGN_SIZE(sizeof(TABLE_LIST)) * stab->lock_count)) || !(key_buff= (char*)thd->memdup(stab->qname.str, - stab->qname.length + 1))) + stab->qname.length))) DBUG_RETURN(FALSE); for (uint j= 0; j < stab->lock_count; j++)