From: Dmitry Lenev Date: December 11 2012 6:37pm Subject: bzr push into mysql-trunk branch (Dmitry.Lenev:5215 to 5216) Bug#15954872 List-Archive: http://lists.mysql.com/commits/145484 X-Bug: 15954872 Message-Id: <20121211183719.6894.72864.5216@jubjub> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 5216 Dmitry Lenev 2012-12-11 [merge] Merged fix for Bug #15954872 "MAKE MDL SUBSYSTEM AND TABLE DEFINITION CACHE ROBUST AGAINST BUGS IN CALLERS" into trunk. modified: sql/mdl.h sql/sql_base.cc sql/sql_cache.cc sql/sql_cache.h sql/table.h unittest/gunit/mdl-t.cc 5215 Gleb Shchepa 2012-12-11 [merge] Auto up-merge (15892875) modified: mysql-test/r/innodb_explain_json_non_select_all.result mysql-test/r/innodb_explain_json_non_select_none.result mysql-test/r/innodb_explain_non_select_all.result mysql-test/r/innodb_explain_non_select_none.result mysql-test/r/myisam_explain_json_non_select_all.result mysql-test/r/myisam_explain_json_non_select_none.result mysql-test/r/myisam_explain_non_select_all.result mysql-test/r/myisam_explain_non_select_none.result mysql-test/r/partition_explicit_prune.result mysql-test/r/partition_locking.result sql/opt_explain.cc === modified file 'sql/mdl.h' --- a/sql/mdl.h 2012-12-05 17:06:53 +0000 +++ b/sql/mdl.h 2012-12-11 18:12:13 +0000 @@ -335,8 +335,16 @@ public: const char *db, const char *name) { m_ptr[0]= (char) mdl_namespace; - m_db_name_length= (uint16) (strmov(m_ptr + 1, db) - m_ptr - 1); - m_length= (uint16) (strmov(m_ptr + m_db_name_length + 2, name) - m_ptr + 1); + /* + It is responsibility of caller to ensure that db and object names + are not longer than NAME_LEN. Still we play safe and try to avoid + buffer overruns. + */ + DBUG_ASSERT(strlen(db) <= NAME_LEN && strlen(name) <= NAME_LEN); + m_db_name_length= static_cast(strmake(m_ptr + 1, db, NAME_LEN) - + m_ptr - 1); + m_length= static_cast(strmake(m_ptr + m_db_name_length + 2, name, + NAME_LEN) - m_ptr + 1); } void mdl_key_init(const MDL_key *rhs) { === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2012-12-11 11:58:31 +0000 +++ b/sql/sql_base.cc 2012-12-11 18:12:13 +0000 @@ -271,8 +271,16 @@ static uint create_table_def_key(THD *th const char *db_name, const char *table_name, bool tmp_table) { - uint key_length= (uint) (strmov(strmov(key, db_name) + 1, table_name) - - key) + 1; + /* + In theory caller should ensure that both db and table_name are + not longer than NAME_LEN bytes. In practice we play safe to avoid + buffer overruns. + */ + DBUG_ASSERT(strlen(db_name) <= NAME_LEN && strlen(table_name) <= NAME_LEN); + uint key_length= static_cast(strmake(strmake(key, db_name, NAME_LEN) + + 1, table_name, NAME_LEN) - key + + 1); + if (tmp_table) { int4store(key + key_length, thd->server_id); @@ -310,8 +318,10 @@ uint get_table_def_key(const TABLE_LIST is properly initialized, so table definition cache can be produced from key used by MDL subsystem. */ - DBUG_ASSERT(!strcmp(table_list->db, table_list->mdl_request.key.db_name()) && - !strcmp(table_list->table_name, table_list->mdl_request.key.name())); + DBUG_ASSERT(!strcmp(table_list->get_db_name(), + table_list->mdl_request.key.db_name()) && + !strcmp(table_list->get_table_name(), + table_list->mdl_request.key.name())); *key= (const char*)table_list->mdl_request.key.ptr() + 1; return table_list->mdl_request.key.length() - 1; @@ -3090,7 +3100,8 @@ err_unlock: TABLE *find_locked_table(TABLE *list, const char *db, const char *table_name) { char key[MAX_DBKEY_LENGTH]; - uint key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1; + uint key_length= create_table_def_key((THD*)NULL, key, db, table_name, + false); for (TABLE *table= list; table ; table=table->next) { @@ -9337,7 +9348,7 @@ void tdc_remove_table(THD *thd, enum_tdc thd->mdl_context.is_lock_owner(MDL_key::TABLE, db, table_name, MDL_EXCLUSIVE)); - key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1; + key_length= create_table_def_key(thd, key, db, table_name, false); if ((share= (TABLE_SHARE*) my_hash_search(&table_def_cache,(uchar*) key, key_length))) === modified file 'sql/sql_cache.cc' --- a/sql/sql_cache.cc 2012-11-21 12:44:48 +0000 +++ b/sql/sql_cache.cc 2012-12-11 18:12:13 +0000 @@ -2824,11 +2824,9 @@ void Query_cache::invalidate_table(THD * invalidate_table(thd, table_list->table); // Table is open else { - char key[MAX_DBKEY_LENGTH]; + const char *key; uint key_length; - - key_length=(uint) (strmov(strmov(key,table_list->db)+1, - table_list->table_name) -key)+ 1; + key_length= get_table_def_key(table_list, &key); // We don't store temporary tables => no key_length+=4 ... invalidate_table(thd, (uchar *)key, key_length); @@ -2940,13 +2938,12 @@ Query_cache::register_tables_from_list(T block_table->n= n; if (tables_used->view) { - char key[MAX_DBKEY_LENGTH]; + const char *key; uint key_length; DBUG_PRINT("qcache", ("view: %s db: %s", tables_used->view_name.str, tables_used->view_db.str)); - key_length= (uint) (strmov(strmov(key, tables_used->view_db.str) + 1, - tables_used->view_name.str) - key) + 1; + key_length= get_table_def_key(tables_used, &key); /* There are not callback function for for VIEWs */ @@ -3056,7 +3053,7 @@ my_bool Query_cache::register_all_tables */ my_bool -Query_cache::insert_table(uint key_len, char *key, +Query_cache::insert_table(uint key_len, const char *key, Query_cache_block_table *node, uint32 db_length, uint8 cache_type, qc_engine_callback callback, @@ -4205,8 +4202,10 @@ uint Query_cache::filename_2_table_key ( *db_length= (filename - dbname) - 1; DBUG_PRINT("qcache", ("table '%-.*s.%s'", *db_length, dbname, filename)); - DBUG_RETURN((uint) (strmov(strmake(key, dbname, *db_length) + 1, - filename) -key) + 1); + DBUG_RETURN(static_cast(strmake(strmake(key, dbname, + min(*db_length, + NAME_LEN)) + 1, + filename, NAME_LEN) - key) + 1); } /**************************************************************************** === modified file 'sql/sql_cache.h' --- a/sql/sql_cache.h 2012-05-31 23:23:37 +0000 +++ b/sql/sql_cache.h 2012-12-11 18:07:34 +0000 @@ -370,7 +370,7 @@ protected: my_bool register_all_tables(Query_cache_block *block, TABLE_LIST *tables_used, TABLE_COUNTER_TYPE tables); - my_bool insert_table(uint key_len, char *key, + my_bool insert_table(uint key_len, const char *key, Query_cache_block_table *node, uint32 db_length, uint8 cache_type, qc_engine_callback callback, === modified file 'sql/table.h' --- a/sql/table.h 2012-12-11 11:58:31 +0000 +++ b/sql/table.h 2012-12-11 18:12:13 +0000 @@ -1992,7 +1992,7 @@ public: @brief Returns the name of the database that the referenced table belongs to. */ - char *get_db_name() { return view != NULL ? view_db.str : db; } + char *get_db_name() const { return view != NULL ? view_db.str : db; } /** @brief Returns the name of the table that this TABLE_LIST represents. @@ -2000,7 +2000,7 @@ public: @details The unqualified table name or view name for a table or view, respectively. */ - char *get_table_name() { return view != NULL ? view_name.str : table_name; } + char *get_table_name() const { return view != NULL ? view_name.str : table_name; } int fetch_number_of_rows(); bool update_derived_keys(Field*, Item**, uint); bool generate_keys(); === modified file 'unittest/gunit/mdl-t.cc' --- a/unittest/gunit/mdl-t.cc 2012-12-04 12:57:18 +0000 +++ b/unittest/gunit/mdl-t.cc 2012-12-11 18:12:13 +0000 @@ -1612,4 +1612,83 @@ TEST_F(MDLTest, HogLockTest5) max_write_lock_count= org_max_write_lock_count; } + +/** Test class for MDL_key class testing. Doesn't require MDL initialization. */ + +class MDLKeyTest : public ::testing::Test +{ +protected: + MDLKeyTest() + { } +private: + GTEST_DISALLOW_COPY_AND_ASSIGN_(MDLKeyTest); +}; + + +// Google Test recommends DeathTest suffix for classes use in death tests. +typedef MDLKeyTest MDLKeyDeathTest; + + +/* + Verifies that debug build dies with a DBUG_ASSERT if we try to construct + MDL_key with too long database or object names. +*/ + +#if GTEST_HAS_DEATH_TEST && !defined(DBUG_OFF) +TEST_F(MDLKeyDeathTest, DieWhenNamesAreTooLong) +{ + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + + /* We need a name which is longer than NAME_LEN = 64*3 = 192.*/ + const char *too_long_name= + "0123456789012345678901234567890123456789012345678901234567890123" + "0123456789012345678901234567890123456789012345678901234567890123" + "0123456789012345678901234567890123456789012345678901234567890123" + "0123456789"; + + EXPECT_DEATH(MDL_key key0(MDL_key::TABLE, too_long_name, ""), + ".*Assertion.*strlen.*failed.*"); + EXPECT_DEATH(MDL_key key1(MDL_key::TABLE, "", too_long_name), + ".*Assertion.*strlen.*failed.*"); + + MDL_key key2; + + EXPECT_DEATH(key2.mdl_key_init(MDL_key::TABLE, too_long_name, ""), + ".*Assertion.*strlen.*failed.*"); + EXPECT_DEATH(key2.mdl_key_init(MDL_key::TABLE, "", too_long_name), + ".*Assertion.*strlen.*failed.*"); + +} +#endif // GTEST_HAS_DEATH_TEST && !defined(DBUG_OFF) + + +/* + Verifies that for production build we allow construction of + MDL_key with too long database or object names, but they are + truncated. +*/ + +#if defined(DBUG_OFF) +TEST_F(MDLKeyTest, TruncateTooLongNames) +{ + /* We need a name which is longer than NAME_LEN = 64*3 = 192.*/ + const char *too_long_name= + "0123456789012345678901234567890123456789012345678901234567890123" + "0123456789012345678901234567890123456789012345678901234567890123" + "0123456789012345678901234567890123456789012345678901234567890123" + "0123456789"; + + MDL_key key(MDL_key::TABLE, too_long_name, too_long_name); + + const char *db_name= key.db_name(); + const char *name= key.name(); + + EXPECT_LE(strlen(db_name), (uint)NAME_LEN); + EXPECT_TRUE(strncmp(db_name, too_long_name, NAME_LEN) == 0); + EXPECT_LE(strlen(name), (uint)NAME_LEN); + EXPECT_TRUE(strncmp(name, too_long_name, NAME_LEN) == 0); +} +#endif // defined(DBUG_OFF) + + } // namespace No bundle (reason: useless for push emails).