List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:December 11 2012 6:40pm
Subject:bzr push into mysql-5.1 branch (Dmitry.Lenev:3882 to 3883) Bug#15954872
View as plain text  
 3883 Dmitry Lenev	2012-12-11
      Bug #15954872 "MAKE MDL SUBSYSTEM AND TABLE DEFINITION CACHE 
      ROBUST AGAINST BUGS IN CALLERS".
      
      Both MDL subsystems and Table Definition Cache code assume 
      that callers ensure that names of objects passed to them are 
      not longer than NAME_LEN bytes. Unfortunately due to bugs in 
      callers this assumption might be broken in some cases. As
      result we get nasty bugs causing buffer overruns when we
      construct MDL key or TDC key from object names.
      
      This patch makes TDC code more robust against such bugs by 
      ensuring that we always checking size of result buffer when
      constructing TDC keys. This doesn't free its callers from 
      ensuring that both db and table names are shorter than 
      NAME_LEN bytes. But at least this steps prevents buffer 
      overruns in case of bug in caller, replacing them with less 
      harmful behavior.
      
      This is 5.1-only version of patch.
      
      This patch introduces new version of create_table_def_key()
      helper function which constructs TDC key without risk of
      result buffer overrun. Places in code that construct TDC keys 
      were changed to use this function.
      
      Also changed rm_temporary_table() and open_new_frm() functions
      to avoid use of "unsafe" strmov() and strxmov() functions and 
      use safer strnxmov() instead.

    modified:
      sql/mysql_priv.h
      sql/sql_base.cc
      sql/sql_cache.cc
      sql/sql_trigger.cc
 3882 sayantan.dutta@stripped	2012-12-11
      Bug #14737171: MTR DOES NOT PRESERVE TEST CASE LOGS ON RETRY-FAIL

    modified:
      mysql-test/mysql-test-run.pl
=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2012-10-21 19:28:19 +0000
+++ b/sql/mysql_priv.h	2012-12-11 18:00:51 +0000
@@ -1296,6 +1296,31 @@ bool mysql_truncate(THD *thd, TABLE_LIST
 bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create);
 uint create_table_def_key(THD *thd, char *key, TABLE_LIST *table_list,
                           bool tmp_table);
+
+/**
+  Create a table cache key for non-temporary table.
+
+  @param key         Buffer for key (must be at least NAME_LEN*2+2 bytes).
+  @param db          Database name.
+  @param table_name  Table name.
+
+  @return Length of key.
+
+  @sa create_table_def_key(thd, char *, table_list, bool)
+*/
+
+inline uint
+create_table_def_key(char *key, const char *db, const char *table_name)
+{
+  /*
+    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.
+  */
+  return (uint)(strmake(strmake(key, db, NAME_LEN) + 1, table_name,
+                        NAME_LEN) - key + 1);
+}
+
 TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key,
                              uint key_length, uint db_flags, int *error);
 void release_table_share(TABLE_SHARE *share, enum release_type type);
@@ -1593,7 +1618,7 @@ int lock_tables(THD *thd, TABLE_LIST *ta
 int decide_logging_format(THD *thd, TABLE_LIST *tables);
 TABLE *open_temporary_table(THD *thd, const char *path, const char *db,
 			    const char *table_name, bool link_in_list);
-bool rm_temporary_table(handlerton *base, char *path);
+bool rm_temporary_table(handlerton *base, const char *path);
 void free_io_cache(TABLE *entry);
 void intern_close_table(TABLE *entry);
 bool close_thread_table(THD *thd, TABLE **table_ptr);

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2012-09-22 12:20:51 +0000
+++ b/sql/sql_base.cc	2012-12-11 18:00:51 +0000
@@ -242,8 +242,9 @@ static void check_unused(void)
 uint create_table_def_key(THD *thd, char *key, TABLE_LIST *table_list,
                           bool tmp_table)
 {
-  uint key_length= (uint) (strmov(strmov(key, table_list->db)+1,
-                                  table_list->table_name)-key)+1;
+  uint key_length= create_table_def_key(key, table_list->db,
+                                        table_list->table_name);
+
   if (tmp_table)
   {
     int4store(key + key_length, thd->server_id);
@@ -619,13 +620,10 @@ void release_table_share(TABLE_SHARE *sh
 TABLE_SHARE *get_cached_table_share(const char *db, const char *table_name)
 {
   char key[NAME_LEN*2+2];
-  TABLE_LIST table_list;
   uint key_length;
   safe_mutex_assert_owner(&LOCK_open);
 
-  table_list.db= (char*) db;
-  table_list.table_name= (char*) table_name;
-  key_length= create_table_def_key((THD*) 0, key, &table_list, 0);
+  key_length= create_table_def_key(key, db, table_name);
   return (TABLE_SHARE*) hash_search(&table_def_cache,(uchar*) key, key_length);
 }  
 
@@ -2419,7 +2417,7 @@ bool lock_table_name_if_not_cached(THD *
   uint key_length;
   DBUG_ENTER("lock_table_name_if_not_cached");
 
-  key_length= (uint)(strmov(strmov(key, db) + 1, table_name) - key) + 1;
+  key_length= create_table_def_key(key, db, table_name);
   VOID(pthread_mutex_lock(&LOCK_open));
 
   if (hash_search(&open_cache, (uchar *)key, key_length))
@@ -3025,7 +3023,7 @@ TABLE *open_table(THD *thd, TABLE_LIST *
 TABLE *find_locked_table(THD *thd, 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(key, db, table_name);
 
   for (TABLE *table=thd->open_tables; table ; table=table->next)
   {
@@ -5737,17 +5735,27 @@ TABLE *open_temporary_table(THD *thd, co
 }
 
 
-bool rm_temporary_table(handlerton *base, char *path)
+/**
+  Delete a temporary table.
+
+  @param base  Handlerton for table to be deleted.
+  @param path  Path to the table to be deleted (i.e. path
+               to its .frm without an extension).
+
+  @retval false - success.
+  @retval true  - failure.
+*/
+
+bool rm_temporary_table(handlerton *base, const char *path)
 {
   bool error=0;
   handler *file;
-  char *ext;
+  char frm_path[FN_REFLEN + 1];
   DBUG_ENTER("rm_temporary_table");
 
-  strmov(ext= strend(path), reg_ext);
-  if (my_delete(path,MYF(0)))
+  strxnmov(frm_path, sizeof(frm_path) - 1, path, reg_ext, NullS);
+  if (my_delete(frm_path, MYF(0)))
     error=1; /* purecov: inspected */
-  *ext= 0;				// remove extension
   file= get_new_handler((TABLE_SHARE*) 0, current_thd->mem_root, base);
   if (file && file->ha_delete_table(path))
   {
@@ -8633,7 +8641,7 @@ bool remove_table_from_cache(THD *thd, c
   DBUG_ENTER("remove_table_from_cache");
   DBUG_PRINT("enter", ("table: '%s'.'%s'  flags: %u", db, table_name, flags));
 
-  key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
+  key_length= create_table_def_key(key, db, table_name);
   for (;;)
   {
     HASH_SEARCH_STATE state;
@@ -8831,12 +8839,14 @@ open_new_frm(THD *thd, TABLE_SHARE *shar
 {
   LEX_STRING pathstr;
   File_parser *parser;
-  char path[FN_REFLEN];
+  char path[FN_REFLEN+1];
   DBUG_ENTER("open_new_frm");
 
   /* Create path with extension */
-  pathstr.length= (uint) (strxmov(path, share->normalized_path.str, reg_ext,
-                                  NullS)- path);
+  pathstr.length= (uint) (strxnmov(path, sizeof(path) - 1,
+                                   share->normalized_path.str,
+                                   reg_ext,
+                                   NullS) - path);
   pathstr.str=    path;
 
   if ((parser= sql_parse_prepare(&pathstr, mem_root, 1)))
@@ -8977,7 +8987,7 @@ void mysql_wait_completed_table(ALTER_PA
   TABLE *table;
   DBUG_ENTER("mysql_wait_completed_table");
 
-  key_length=(uint) (strmov(strmov(key,lpt->db)+1,lpt->table_name)-key)+1;
+  key_length= create_table_def_key(key, lpt->db, lpt->table_name);
   VOID(pthread_mutex_lock(&LOCK_open));
   HASH_SEARCH_STATE state;
   for (table= (TABLE*) hash_first(&open_cache,(uchar*) key,key_length,

=== modified file 'sql/sql_cache.cc'
--- a/sql/sql_cache.cc	2012-02-15 16:21:38 +0000
+++ b/sql/sql_cache.cc	2012-12-11 18:00:51 +0000
@@ -2708,8 +2708,8 @@ void Query_cache::invalidate_table(THD *
     char key[MAX_DBKEY_LENGTH];
     uint key_length;
 
-    key_length=(uint) (strmov(strmov(key,table_list->db)+1,
-			      table_list->table_name) -key)+ 1;
+    key_length= create_table_def_key(key, table_list->db,
+                                     table_list->table_name);
 
     // We don't store temporary tables => no key_length+=4 ...
     invalidate_table(thd, (uchar *)key, key_length);
@@ -2830,8 +2830,8 @@ Query_cache::register_tables_from_list(T
       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= create_table_def_key(key, tables_used->view_db.str,
+                                       tables_used->view_name.str);
       /*
         There are not callback function for for VIEWs
       */
@@ -4062,8 +4062,9 @@ 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((uint) (strmake(strmake(key, dbname,
+                                      min(*db_length, NAME_LEN)) + 1,
+                              filename, NAME_LEN) - key) + 1);
 }
 
 /****************************************************************************

=== modified file 'sql/sql_trigger.cc'
--- a/sql/sql_trigger.cc	2011-07-03 15:47:37 +0000
+++ b/sql/sql_trigger.cc	2012-12-11 18:00:51 +0000
@@ -1988,9 +1988,7 @@ bool Table_triggers_list::change_table_n
   */
 #ifndef DBUG_OFF
   uchar key[MAX_DBKEY_LENGTH];
-  uint key_length= (uint) (strmov(strmov((char*)&key[0], db)+1,
-                    old_table)-(char*)&key[0])+1;
-
+  uint key_length= create_table_def_key((char *)key, db, old_table);
   if (!is_table_name_exclusively_locked_by_this_thread(thd, key, key_length))
     safe_mutex_assert_owner(&LOCK_open);
 #endif

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.1 branch (Dmitry.Lenev:3882 to 3883) Bug#15954872Dmitry Lenev12 Dec