MDEV-36871 mariadb-backup incremental segfault querying mariadb_backup_history

Problem:
=========
(1) Mariabackup tries to read the history data from
mysql.mariadb_backup_history and fails with segfault. Reason is that
mariabackup does force innodb_log_checkpoint_now from commit 652f33e(MDEV-30000).
Mariabackup sends the "innodb_log_checkpoint_now=1" query to server and
reads the result set for the query later in the code because the query
may trigger the page thread to flush the pages. But before reading the
query result for innodb_log_checkpoint_now=1, mariabackup does execute
the select query for the history table (mysql.mariadb_backup_history)
and wrongly reads the query result of innodb_log_checkpoint_now. This leads
to assertion in mariabackup.

(2) The recording of incremental backups has the format as "tar"
when mbstream was used. The xb_stream_fmt_t only had XB_STREAM_FMT_NONE
and XB_STREAM_FMT_XBSTREAM and hence in the mysql.mariadb_backup_history
table the format was recorded as "tar" for the "mbstream" due to the
offset in the xb_stream_name array within mariadb-backup.

(3) Also under Windows the full path of mariabackup was recorded in the the
history.

(4) select_incremental_lsn_from_history(): Name of the backup and UUID
of the history record variable could lead to buffer overflow while
copying the variable value from global variable.

Solution:
=========
(1) Move the reading of history data from mysql.mariadb_backup_history
after reading the result of innodb_log_checkpoint_now=1 query

(2) We've removed the "tar" element from the xb_stream_name. As the
"xbstream" was never used, the format name is changed to mbstream.
As the table needs alteration the "mbstream" appended instead of
the unused xbstream in the table. "tar" is left in the enum as
the previous recordings are still possible.

(3) The Windows path separator is used to store just the executable
name as the tool in the mariadb_backup_history table.

(4) select_incremental_lsn_from_history(): Check and validate
the length of incremental history name and incremental history uuid
before copying into temporary buffer

Thanks to Daniel black for contributing the code for solution (2) and (3)

Author: Thirunarayanan

extra/mariabackup/backup_mysql.cc

@@ -561,12 +561,36 @@ select_incremental_lsn_from_history(lsn_t *incremental_lsn)
 {
 	MYSQL_RES *mysql_result;
 	char query[1000];
-	char buf[100];
+	char buf[NAME_LEN*2+3];
+
+	size_t opt_incremental_history_name_len= 0;
+	size_t opt_incremental_history_uuid_len= 0;
+
+	if (opt_incremental_history_name)
+		opt_incremental_history_name_len=
+                  strlen(opt_incremental_history_name);
+
+	if (opt_incremental_history_uuid)
+		opt_incremental_history_uuid_len=
+                  strlen(opt_incremental_history_uuid);
+
+	if (opt_incremental_history_name_len*2 > sizeof(buf))
+		die("Incremental history table name '%s' is too long.",
+		    opt_incremental_history_name);
+
+	if (opt_incremental_history_uuid_len*2 > sizeof(buf))
+		die("Incremental history uuid '%s' is too long.",
+		    opt_incremental_history_uuid);
+
+	if (opt_incremental_history_name && opt_incremental_history_name[0]
+	    && opt_incremental_history_uuid && opt_incremental_history_uuid[0])
+		die("It is allowed to use either --incremental-history-name "
+		    "or --incremental-history-uuid, but not both.");
 
 	if (opt_incremental_history_name) {
 		mysql_real_escape_string(mysql_connection, buf,
 				opt_incremental_history_name,
-				(unsigned long)strlen(opt_incremental_history_name));
+				(unsigned long) opt_incremental_history_name_len);
 		snprintf(query, sizeof(query),
 			"SELECT innodb_to_lsn "
 			"FROM " XB_HISTORY_TABLE " "
@@ -575,11 +599,10 @@ select_incremental_lsn_from_history(lsn_t *incremental_lsn)
 			"ORDER BY innodb_to_lsn DESC LIMIT 1",
 			buf);
 	}
-
-	if (opt_incremental_history_uuid) {
+	else if (opt_incremental_history_uuid) {
 		mysql_real_escape_string(mysql_connection, buf,
 				opt_incremental_history_uuid,
-				(unsigned long)strlen(opt_incremental_history_uuid));
+				(unsigned long) opt_incremental_history_uuid_len);
 		snprintf(query, sizeof(query),
 			"SELECT innodb_to_lsn "
 			"FROM " XB_HISTORY_TABLE " "
@@ -589,6 +612,8 @@ select_incremental_lsn_from_history(lsn_t *incremental_lsn)
 			buf);
 	}
 
+	/* xb_mysql_query(..,.., true) will die on error, so
+	mysql_result can't be nullptr */
 	mysql_result = xb_mysql_query(mysql_connection, query, true);
 
 	ut_ad(mysql_num_fields(mysql_result) == 1);
@@ -1689,7 +1714,7 @@ write_xtrabackup_info(ds_ctxt *datasink,
 	char buf_end_time[100];
 	tm tm;
 	std::ostringstream oss;
-	const char *xb_stream_name[] = {"file", "tar", "xbstream"};
+	const char *xb_stream_name[] = {"file", "mbstream"};
 
 	uuid = read_mysql_one_value(connection, "SELECT UUID()");
 	server_version = read_mysql_one_value(connection, "SELECT VERSION()");
@@ -1772,6 +1797,10 @@ write_xtrabackup_info(ds_ctxt *datasink,
 		goto cleanup;
 	}
 
+	xb_mysql_query(connection,
+		"ALTER TABLE IF EXISTS " XB_HISTORY_TABLE
+		" MODIFY format ENUM('file', 'tar', 'mbstream') DEFAULT NULL", false);
+
 	xb_mysql_query(connection,
 		"CREATE TABLE IF NOT EXISTS " XB_HISTORY_TABLE "("
 		"uuid VARCHAR(40) NOT NULL PRIMARY KEY,"
@@ -1789,7 +1818,7 @@ write_xtrabackup_info(ds_ctxt *datasink,
 		"innodb_to_lsn BIGINT UNSIGNED DEFAULT NULL,"
 		"partial ENUM('Y', 'N') DEFAULT NULL,"
 		"incremental ENUM('Y', 'N') DEFAULT NULL,"
-		"format ENUM('file', 'tar', 'xbstream') DEFAULT NULL,"
+		"format ENUM('file', 'tar', 'mbstream') DEFAULT NULL,"
 		"compressed ENUM('Y', 'N') DEFAULT NULL"
 		") CHARACTER SET utf8 ENGINE=innodb", false);
 
@@ -1940,7 +1969,7 @@ void
 capture_tool_command(int argc, char **argv)
 {
 	/* capture tool name tool args */
-	tool_name = strrchr(argv[0], '/');
+	tool_name = strrchr(argv[0], IF_WIN('\\', '/'));
 	tool_name = tool_name ? tool_name + 1 : argv[0];
 
 	make_argv(tool_args, sizeof(tool_args), argc, argv);

extra/mariabackup/xtrabackup.cc

@@ -5520,10 +5520,6 @@ static bool xtrabackup_backup_func()
 
 	backup_datasinks.init();
 
-	if (!select_history()) {
-		goto fail;
-	}
-
 	/* open the log file */
 	memset(&stat_info, 0, sizeof(MY_STAT));
 	dst_log_file = ds_open(backup_datasinks.m_redo, LOG_FILE_NAME, &stat_info);
@@ -5538,6 +5534,11 @@ static bool xtrabackup_backup_func()
 	if (innodb_log_checkpoint_now != false) {
 		mysql_read_query_result(mysql_connection);
 	}
+
+	if (!select_history()) {
+		goto fail;
+	}
+
 	/* label it */
 	recv_sys.file_checkpoint = log_sys.next_checkpoint_lsn;
 	log_hdr_init();

mysql-test/suite/mariabackup/xb_history.result

@@ -17,7 +17,7 @@ mariadb_backup_history	CREATE TABLE `mariadb_backup_history` (
   `innodb_to_lsn` bigint(20) unsigned DEFAULT NULL,
   `partial` enum('Y','N') DEFAULT NULL,
   `incremental` enum('Y','N') DEFAULT NULL,
-  `format` enum('file','tar','xbstream') DEFAULT NULL,
+  `format` enum('file','tar','mbstream') DEFAULT NULL,
   `compressed` enum('Y','N') DEFAULT NULL,
   PRIMARY KEY (`uuid`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_general_ci

mysql-test/suite/mariabackup/xbstream.result

@@ -1,13 +1,37 @@
 CREATE TABLE t(i INT) ENGINE INNODB;
 INSERT INTO t VALUES(1);
-# xtrabackup backup to stream
+# xtrabackup full backup to stream
+INSERT INTO t VALUES(2), (3), (4);
+# xtrabackup incremental backup to stream
+# checking recording of history
+SELECT tool_name, name, partial, incremental, format, compressed
+FROM mysql.mariadb_backup_history
+ORDER BY innodb_from_lsn;
+tool_name	mariabackup
+name	fullback
+partial	Y
+incremental	N
+format	mbstream
+compressed	N
+tool_name	mariabackup
+name	incr_1
+partial	N
+incremental	Y
+format	mbstream
+compressed	N
 # xbstream extract
 # xtrabackup prepare
+# xbstream extract for incremental backup
+# xtrabackup incremental prepare
 # shutdown server
 # remove datadir
 # xtrabackup move back
 # restart
 SELECT * FROM t;
 i
 1
+2
+3
+4
 DROP TABLE t;
+DROP TABLE mysql.mariadb_backup_history;

mysql-test/suite/mariabackup/xbstream.test

@@ -4,21 +4,48 @@ CREATE TABLE t(i INT) ENGINE INNODB;
 INSERT INTO t VALUES(1);
 
 let $targetdir=$MYSQLTEST_VARDIR/tmp/backup;
+let $incr_dir=$MYSQLTEST_VARDIR/tmp/backup_incr;
 mkdir $targetdir;
+mkdir $incr_dir;
+
 let $streamfile=$MYSQLTEST_VARDIR/tmp/backup.xb;
+let $stream_incr_file=$MYSQLTEST_VARDIR/tmp/backup_incr.xb;
+
+echo # xtrabackup full backup to stream;
+exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf  --backup --parallel=10  --databases-exclude=foobar --history=fullback --stream=xbstream > $streamfile 2>$targetdir/backup_stream.log;
+
+INSERT INTO t VALUES(2), (3), (4);
+
+echo # xtrabackup incremental backup to stream;
+exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf --backup  --incremental-history-name=fullback --history=incr_1  --stream=xbstream > $stream_incr_file 2>$targetdir/backup_incr.log;
+
+echo # checking recording of history;
+--replace_result mariabackup.exe mariabackup
+--vertical_results
+SELECT tool_name, name, partial, incremental, format, compressed
+FROM mysql.mariadb_backup_history
+ORDER BY innodb_from_lsn;
+
+--horizontal_results
 
-echo # xtrabackup backup to stream;
-exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf  --backup --parallel=10  --databases-exclude=foobar --stream=xbstream > $streamfile 2>$targetdir/backup_stream.log;
 echo # xbstream extract;
 --disable_result_log
 exec $XBSTREAM -x -C $targetdir  < $streamfile;
 
 echo # xtrabackup prepare;
 exec $XTRABACKUP --prepare --target-dir=$targetdir;
 
+echo # xbstream extract for incremental backup;
+exec $XBSTREAM -x -C $incr_dir  < $stream_incr_file;
+
+echo # xtrabackup incremental prepare;
+exec $XTRABACKUP --prepare --target-dir=$targetdir --incremental-dir=$incr_dir;
+
 -- source include/restart_and_restore.inc
 --enable_result_log
 SELECT * FROM t;
 DROP TABLE t;
+DROP TABLE mysql.mariadb_backup_history;
 rmdir $targetdir;
-
+remove_file $streamfile;
+remove_file $stream_incr_file;