Don't allow BACKUP TABLE to overwrite files

Fixed memory leak when replication restarts in debug mode


include/my_sys.h:
  Added option to not overwrite files to my_copy()
mysql-test/mysql-test-run.sh:
  Fixed --ddd option
  Fixed that mysqld is restarted if there is a testname-master.sh file
mysql-test/r/backup.result:
  Update for security fix in BACKUP TABLE
mysql-test/t/backup.test:
  Update for security fix in BACKUP TABLE
mysys/my_copy.c:
  Added option to not overwrite files to my_copy()
sql/ha_myisam.cc:
  Don't allow BACKUP TABLE to overwrite files
sql/slave.cc:
  Fixed problem with --debug output from 'handle_slave'
  Fixed memory leak when replication restarts in debug mode

include/my_sys.h

@@ -73,6 +73,7 @@ extern int NEAR my_errno;		/* Last error in mysys */
 #define MY_FREE_ON_ERROR 128	/* my_realloc() ; Free old ptr on error */
 #define MY_HOLD_ON_ERROR 256	/* my_realloc() ; Return old ptr on error */
 #define MY_THREADSAFE	128	/* pread/pwrite:  Don't allow interrupts */
+#define MY_DONT_OVERWRITE_FILE 1024	/* my_copy; Don't overwrite file */
 
 #define MY_CHECK_ERROR	1	/* Params to my_end; Check open-close */
 #define MY_GIVE_INFO	2	/* Give time info about process*/

mysql-test/mysql-test-run.sh

@@ -604,7 +604,7 @@ start_slave()
              $EXTRA_SLAVE_OPT $EXTRA_SLAVE_MYSQLD_OPT"
     if [ x$DO_DDD = x1 ]
     then
-      $ECHO "set args $master_args" > $GDB_SLAVE_INIT
+      $ECHO "set args $slave_args" > $GDB_SLAVE_INIT
       ddd --debugger "gdb -x $GDB_SLAVE_INIT" $SLAVE_MYSQLD &
       prompt_user "Hit enter to continue after you've started the slave"
     elif [ x$DO_GDB = x1 ]
@@ -746,7 +746,7 @@ run_testcase ()
      stop_master
      start_master
    else
-     if [ ! -z "$EXTRA_MASTER_OPT" ] || [ x$MASTER_RUNNING != x1 ] ;
+     if [ ! -z "$EXTRA_MASTER_OPT" ] || [ x$MASTER_RUNNING != x1 ] || [ -f $master_init_script ]
      then
        EXTRA_MASTER_OPT=""
        stop_master

mysql-test/r/backup.result

 Table	Op	Msg_type	Msg_text
-test.t1	backup	error	Failed copying .frm file: errno = X
-test.t1	backup	status	Operation failed
+test.t4	backup	error	Failed copying .frm file (errno: X)
+test.t4	backup	status	Operation failed
 Table	Op	Msg_type	Msg_text
-test.t1	backup	status	OK
+test.t4	backup	status	OK
 Table	Op	Msg_type	Msg_text
-test.t1	restore	status	OK
+test.t4	backup	error	Failed copying .frm file (errno: X)
+test.t4	backup	status	Operation failed
+Table	Op	Msg_type	Msg_text
+test.t4	restore	status	OK
 count(*)
 0
 Table	Op	Msg_type	Msg_text
@@ -18,7 +21,6 @@ n
 45
 67
 Table	Op	Msg_type	Msg_text
-test.t1	backup	status	OK
 test.t2	backup	status	OK
 test.t3	backup	status	OK
 Table	Op	Msg_type	Msg_text
@@ -40,4 +42,4 @@ k
 Table	Op	Msg_type	Msg_text
 test.t1	restore	status	OK
 Table	Op	Msg_type	Msg_text
-test.t1	backup	status	OK
+test.t5	backup	status	OK

mysql-test/t/backup-master.sh

+#!/bin/sh
+if [ "$MYSQL_TEST_DIR" ]
+then
+  rm -f $MYSQL_TEST_DIR/var/tmp/*.frm $MYSQL_TEST_DIR/var/tmp/*.MY?
+fi

mysql-test/t/backup.test

+#
+# This test is a bit tricky as we can't use backup table to overwrite an old
+# table
+#
 connect (con1,localhost,root,,);
 connect (con2,localhost,root,,);
 connection con1;
 set SQL_LOG_BIN=0;
-drop table if exists t1;
+drop table if exists t1,t2,t3,t4;
+create table t4(n int);
+--replace_result "errno: 2" "errno: X" "errno: 22" "errno: X" "errno: 23" "errno: X"
+backup table t4 to '../bogus';
+backup table t4 to '../tmp';
+--replace_result "errno: 17" "errno: X"
+backup table t4 to '../tmp';
+drop table t4;
+restore table t4 from '../tmp';
+select count(*) from t4;
+
 create table t1(n int);
---replace_result "errno = 2" "errno = X" "errno = 22" "errno = X" "errno = 23" "errno = X"
-backup table t1 to '../bogus';
-backup table t1 to '../tmp';
-drop table t1;
-restore table t1 from '../tmp';
-select count(*) from t1;
 insert into t1 values (23),(45),(67);
 backup table t1 to '../tmp';
 drop table t1;
@@ -20,29 +28,24 @@ create table t2(m int not null primary key);
 create table t3(k int not null primary key);
 insert into t2 values (123),(145),(167);
 insert into t3 values (223),(245),(267);
-backup table t1,t2,t3 to '../tmp';
+backup table t2,t3 to '../tmp';
 drop table t1,t2,t3;
 restore table t1,t2,t3 from '../tmp';
 select n from t1;
 select m from t2;
 select k from t3;
-drop table t1,t2,t3;
+drop table t1,t2,t3,t4;
 restore table t1 from '../tmp';
 connection con2;
+rename table t1 to t5;
 --send
-lock tables t1 write;
+lock tables t5 write;
 connection con1;
 --send
-backup table t1 to '../tmp';
+backup table t5 to '../tmp';
 connection con2;
 reap;
 unlock tables;
 connection con1;
 reap;
-drop table t1;
-
-
-
-
-
-
+drop table t5;

mysys/my_copy.c

@@ -32,17 +32,29 @@ struct utimbuf {
 #endif
 
 
-	/*
-	  Ordinary ownership and accesstimes are copied from 'from-file'
-	  if MyFlags & MY_HOLD_ORIGINAL_MODES is set and to-file exists then
-	  the modes of to-file isn't changed
-	  Dont set MY_FNABP or MY_NABP bits on when calling this function !
-	  */
+/*
+  int my_copy(const char *from, const char *to, myf MyFlags)
+
+  NOTES
+    Ordinary ownership and accesstimes are copied from 'from-file'
+    If MyFlags & MY_HOLD_ORIGINAL_MODES is set and to-file exists then
+    the modes of to-file isn't changed
+    If MyFlags & MY_DONT_OVERWRITE_FILE is set, we will give an error
+    if the file existed.
+
+  WARNING
+    Don't set MY_FNABP or MY_NABP bits on when calling this function !
+
+  RETURN
+    0	ok
+    #	Error
+
+*/
 
 int my_copy(const char *from, const char *to, myf MyFlags)
 {
   uint Count;
-  int new_file_stat;
+  int new_file_stat, create_flag;
   File from_file,to_file;
   char buff[IO_SIZE];
   struct stat stat_buff,new_stat_buff;
@@ -63,8 +75,10 @@ int my_copy(const char *from, const char *to, myf MyFlags)
     }
     if (MyFlags & MY_HOLD_ORIGINAL_MODES && !new_file_stat)
       stat_buff=new_stat_buff;
+    create_flag= (MyFlags & MY_DONT_OVERWRITE_FILE) ? O_EXCL : O_TRUNC;
+
     if ((to_file=  my_create(to,(int) stat_buff.st_mode,
-			     O_WRONLY | O_TRUNC | O_BINARY | O_SHARE,
+			     O_WRONLY | create_flag | O_BINARY | O_SHARE,
 			     MyFlags)) < 0)
       goto err;
 

sql/ha_myisam.cc

@@ -419,38 +419,38 @@ int ha_myisam::backup(THD* thd, HA_CHECK_OPT *check_opt)
   const char* errmsg = "";
   
   if (!fn_format(dst_path, table_name, backup_dir, reg_ext, 4 + 64))
-    {
-      errmsg = "Failed in fn_format() for .frm file: errno = %d";
-      error = HA_ADMIN_INVALID;
-      goto err;
-    }
+  {
+    errmsg = "Failed in fn_format() for .frm file (errno: %d)";
+    error = HA_ADMIN_INVALID;
+    goto err;
+  }
   
   if (my_copy(fn_format(src_path, table->path,"", reg_ext, 4),
-	     dst_path,
-	     MYF(MY_WME | MY_HOLD_ORIGINAL_MODES )))
+	      dst_path,
+	      MYF(MY_WME | MY_HOLD_ORIGINAL_MODES | MY_DONT_OVERWRITE_FILE)))
   {
     error = HA_ADMIN_FAILED;
-    errmsg = "Failed copying .frm file: errno = %d";
+    errmsg = "Failed copying .frm file (errno: %d)";
     goto err;
   }
 
   if (!fn_format(dst_path, table_name, backup_dir, MI_NAME_DEXT, 4 + 64))
-    {
-      errmsg = "Failed in fn_format() for .MYD file: errno = %d";
-      error = HA_ADMIN_INVALID;
-      goto err;
-    }
+  {
+    errmsg = "Failed in fn_format() for .MYD file (errno: %d)";
+    error = HA_ADMIN_INVALID;
+    goto err;
+  }
 
   if (my_copy(fn_format(src_path, table->path,"", MI_NAME_DEXT, 4),
 	      dst_path,
-	      MYF(MY_WME | MY_HOLD_ORIGINAL_MODES ))  )
-    {
-      errmsg = "Failed copying .MYD file: errno = %d";
-      error= HA_ADMIN_FAILED;
-      goto err;
-    }
+	      MYF(MY_WME | MY_HOLD_ORIGINAL_MODES | MY_DONT_OVERWRITE_FILE)))
+  {
+    errmsg = "Failed copying .MYD file (errno: %d)";
+    error= HA_ADMIN_FAILED;
+    goto err;
+  }
   return HA_ADMIN_OK;
- err:
+err:
   {
     MI_CHECK param;
     myisamchk_init(&param);
@@ -459,7 +459,7 @@ int ha_myisam::backup(THD* thd, HA_CHECK_OPT *check_opt)
     param.db_name    = table->table_cache_key;
     param.table_name = table->table_name;
     param.testflag = 0;
-    mi_check_print_error(&param,errmsg, errno );
+    mi_check_print_error(&param, errmsg, my_errno);
     return error;
   }
 }

sql/slave.cc

@@ -1270,6 +1270,17 @@ This may also be a network problem, or just a bug in the master or slave code.\
 
 pthread_handler_decl(handle_slave,arg __attribute__((unused)))
 {
+  // needs to call my_thread_init(), otherwise we get a coredump in DBUG_ stuff
+  my_thread_init();
+  if (!server_id)
+  {
+    pthread_cond_broadcast(&COND_slave_start);
+    sql_print_error("Server id not set, will not start slave");
+    my_thread_end();
+    pthread_exit((void*)1);
+  }
+  DBUG_ENTER("handle_slave");
+
 #ifndef DBUG_OFF
  slave_begin:  
 #endif  
@@ -1278,20 +1289,14 @@ pthread_handler_decl(handle_slave,arg __attribute__((unused)))
   char llbuff[22];
 
   pthread_mutex_lock(&LOCK_slave);
-  if (!server_id)
+  
+  if (slave_running)
   {
     pthread_cond_broadcast(&COND_slave_start);
     pthread_mutex_unlock(&LOCK_slave);
-    sql_print_error("Server id not set, will not start slave");
-    pthread_exit((void*)1);
+    my_thread_end();
+    pthread_exit((void*)1);  // safety just in case
   }
-  
-  if(slave_running)
-    {
-      pthread_cond_broadcast(&COND_slave_start);
-      pthread_mutex_unlock(&LOCK_slave);
-      pthread_exit((void*)1);  // safety just in case
-    }
   slave_running = 1;
   abort_slave = 0;
 #ifndef DBUG_OFF  
@@ -1304,11 +1309,8 @@ pthread_handler_decl(handle_slave,arg __attribute__((unused)))
   bool retried_once = 0;
   ulonglong last_failed_pos = 0;
   
-  // needs to call my_thread_init(), otherwise we get a coredump in DBUG_ stuff
-  my_thread_init();
   slave_thd = thd = new THD; // note that contructor of THD uses DBUG_ !
   thd->set_time();
-  DBUG_ENTER("handle_slave");
 
   pthread_detach_this_thread();
   if (init_slave_thread(thd) || init_master_info(&glob_mi))
@@ -1518,18 +1520,18 @@ position %s",
   abort_slave = 0;
   save_temporary_tables = thd->temporary_tables;
   thd->temporary_tables = 0; // remove tempation from destructor to close them
-  pthread_cond_broadcast(&COND_slave_stopped); // tell the world we are done
-  pthread_mutex_unlock(&LOCK_slave);
   net_end(&thd->net); // destructor will not free it, because we are weird
   slave_thd = 0;
   (void) pthread_mutex_lock(&LOCK_thread_count);
   delete thd;
   (void) pthread_mutex_unlock(&LOCK_thread_count);
-  my_thread_end();
+  pthread_mutex_unlock(&LOCK_slave);
+  pthread_cond_broadcast(&COND_slave_stopped); // tell the world we are done
 #ifndef DBUG_OFF
   if(abort_slave_event_count && !events_till_abort)
     goto slave_begin;
 #endif  
+  my_thread_end();
   pthread_exit(0);
   DBUG_RETURN(0);				// Can't return anything here
 }